From 46fad66ab183799d19bf81ab2aa29593a149ebba Mon Sep 17 00:00:00 2001
From: Rachel Fenichel <fenichel@google.com>
Date: Wed, 30 Oct 2019 15:58:53 -0700
Subject: [PATCH] Review feedback.

---
 core/block_svg.js                    |  2 --
 core/renderers/common/path_object.js |  4 ++--
 core/renderers/geras/path_object.js  |  7 ++++---
 core/theme.js                        | 20 +++++++++-----------
 core/utils/colour.js                 |  2 +-
 5 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/core/block_svg.js b/core/block_svg.js
index 510220a89..fe2b0b705 100644
--- a/core/block_svg.js
+++ b/core/block_svg.js
@@ -1043,8 +1043,6 @@ Blockly.BlockSvg.prototype.applyColour = function() {
 
   var icons = this.getIcons();
   for (var i = 0; i < icons.length; i++) {
-    // TODO: Decide whether to make icon and field applyColour take in a
-    // colourer object.
     icons[i].applyColour();
   }
 
diff --git a/core/renderers/common/path_object.js b/core/renderers/common/path_object.js
index e24f60a14..493245289 100644
--- a/core/renderers/common/path_object.js
+++ b/core/renderers/common/path_object.js
@@ -63,9 +63,9 @@ Blockly.blockRendering.PathObject = function(root) {
   /**
    * The style object to use when colouring block paths.
    * @type {!Blockly.Theme.BlockStyle}
-   * @public
+   * @package
    */
-  this.style = Blockly.Theme.createBlockStyle('#0000000');
+  this.style = Blockly.Theme.createBlockStyle('#000000');
 };
 
 /**
diff --git a/core/renderers/geras/path_object.js b/core/renderers/geras/path_object.js
index 8c63b4eb0..42e995b85 100644
--- a/core/renderers/geras/path_object.js
+++ b/core/renderers/geras/path_object.js
@@ -25,6 +25,7 @@
 goog.provide('Blockly.geras.PathObject');
 
 goog.require('Blockly.blockRendering.IPathObject');
+goog.require('Blockly.Theme');
 goog.require('Blockly.utils.dom');
 goog.require('Blockly.utils.object');
 
@@ -73,14 +74,14 @@ Blockly.geras.PathObject = function(root) {
    * @type {string}
    * @package
    */
-  this.darkColour = '#000000';
+  this.colourDark = '#000000';
 
   /**
    * The style object to use when colouring block paths.
    * @type {!Blockly.Theme.BlockStyle}
-   * @public
+   * @package
    */
-  this.style = Blockly.Theme.createBlockStyle('#0000000');
+  this.style = Blockly.Theme.createBlockStyle('#000000');
 };
 
 /**
diff --git a/core/theme.js b/core/theme.js
index dbae4f453..a46f1af15 100644
--- a/core/theme.js
+++ b/core/theme.js
@@ -38,12 +38,12 @@ goog.require('Blockly.utils.colour');
 Blockly.Theme = function(blockStyles, categoryStyles, opt_componentStyles) {
   /**
    * The block styles map.
-   * @type {!Object.<string, Blockly.Theme.BlockStyle>}
+   * @type {!Object.<string, !Blockly.Theme.BlockStyle>}
    * @private
    */
   this.blockStyles_ = {};
 
-  // Make sure all styles are valid before insterting them into the map.
+  // Make sure all styles are valid before inserting them into the map.
   this.setAllBlockStyles(blockStyles);
 
   /**
@@ -106,18 +106,14 @@ Blockly.Theme.prototype.getAllBlockStyles = function() {
  *     if no style with the given name was found.
  */
 Blockly.Theme.prototype.getBlockStyle = function(blockStyleName) {
-  var defaultStyle = Blockly.Theme.createBlockStyle('#000000');
-  if (blockStyleName == null) {
-    return defaultStyle;
-  }
-  return this.blockStyles_[blockStyleName] || defaultStyle;
+  return this.blockStyles_[blockStyleName || ''] ||
+      Blockly.Theme.createBlockStyle('#000000');
 };
 
 /**
  * Overrides or adds a style to the blockStyles map.
  * @param {string} blockStyleName The name of the block style.
  * @param {Blockly.Theme.BlockStyle} blockStyle The block style.
- * @package
  */
 Blockly.Theme.prototype.setBlockStyle = function(blockStyleName, blockStyle) {
   blockStyle = Blockly.Theme.validatedBlockStyle(blockStyle);
@@ -159,7 +155,7 @@ Blockly.Theme.createBlockStyle = function(colour) {
 /**
  * Get a full block style object based on the input style object.  Populate
  * any missing values.
- * @param {!Blockly.Theme.BlockStyle} blockStyle A full or partial block
+ * @param {Blockly.Theme.BlockStyle} blockStyle A full or partial block
  *     style object.
  * @return {!Blockly.Theme.BlockStyle} A full block style object, with all
  *     required properties populated.
@@ -168,11 +164,13 @@ Blockly.Theme.createBlockStyle = function(colour) {
 Blockly.Theme.validatedBlockStyle = function(blockStyle) {
   // Make a new object with all of the same properties.
   var valid = {};
-  Blockly.utils.object.mixin(valid, blockStyle);
+  if (blockStyle) {
+    Blockly.utils.object.mixin(valid, blockStyle);
+  }
 
   // Validate required properties.
   var parsedColour = Blockly.utils.colour.parseBlockColour(
-      valid.colourPrimary || '#000000');
+      valid.colourPrimary || '#000');
   valid.colourPrimary = parsedColour.hex;
   valid.colourSecondary = valid.colourSecondary ||
       Blockly.utils.colour.blend('#fff', valid.colourPrimary, 0.6) ||
diff --git a/core/utils/colour.js b/core/utils/colour.js
index 48f56576f..810bd5637 100644
--- a/core/utils/colour.js
+++ b/core/utils/colour.js
@@ -219,7 +219,7 @@ Blockly.utils.colour.names = {
  *     or a message reference string pointing to one of those two values.
  * @return {{hue: ?number, hex: string}} An object containing the colour as
  *     a #RRGGBB string, and the hue if the input was an HSV hue value.
- * @throws {Error} If If the colour cannot be parsed.
+ * @throws {Error} If the colour cannot be parsed.
  */
 Blockly.utils.colour.parseBlockColour = function(colour) {
   var dereferenced = (typeof colour == 'string') ?