Remove this.text_ (#2910)

* Remove this.text_ and have every field handle its own properties.

core/field.js

@@ -54,6 +54,7 @@ Blockly.Field = function(value, opt_validator, opt_config) {
   /**
    * The size of the area rendered by the field.
    * @type {Blockly.utils.Size}
+   * @protected
    */
   this.size_ = new Blockly.utils.Size(0, 0);
   this.setValue(value);
@@ -117,15 +118,6 @@ Blockly.Field.prototype.maxDisplayLength = 50;
  */
 Blockly.Field.prototype.value_ = null;
 
-/**
- * Text representation of the field's value. Maintained for backwards
- * compatibility reasons.
- * @type {string}
- * @protected
- * @deprecated Use or override getText instead.
- */
-Blockly.Field.prototype.text_ = '';
-
 /**
  * Used to cache the field's tooltip value if setTooltip is called when the
  * field is not yet initialized. Is *not* guaranteed to be accurate.
@@ -169,6 +161,15 @@ Blockly.Field.prototype.validator_ = null;
  */
 Blockly.Field.prototype.clickTarget_ = null;
 
+/**
+ * A developer hook to override the returned text of this field.
+ * Override if the text representation of the value of this field
+ * is not just a string cast of its value.
+ * @return {?string} Current text. Return null to resort to a string cast.
+ * @protected
+ */
+Blockly.Field.prototype.getText_;
+
 /**
  * Non-breaking space.
  * @const
@@ -619,13 +620,13 @@ Blockly.Field.prototype.getScaledBBox_ = function() {
 };
 
 /**
- * Get the text from this field as displayed on screen.  May differ from getText
- * due to ellipsis, and other formatting.
- * @return {string} Currently displayed text.
+ * Get the text from this field to display on the block. May differ from
+ * ``getText`` due to ellipsis, and other formatting.
+ * @return {string} Text to display.
  * @protected
  */
 Blockly.Field.prototype.getDisplayText_ = function() {
-  var text = this.text_;
+  var text = this.getText();
   if (!text) {
     // Prevent the field from disappearing if empty.
     return Blockly.Field.NBSP;
@@ -636,7 +637,7 @@ Blockly.Field.prototype.getDisplayText_ = function() {
   }
   // Replace whitespace with non-breaking spaces so the text doesn't collapse.
   text = text.replace(/\s/g, Blockly.Field.NBSP);
-  if (this.sourceBlock_.RTL) {
+  if (this.sourceBlock_ && this.sourceBlock_.RTL) {
     // The SVG is LTR, force text to be RTL.
     text += '\u200F';
   }
@@ -648,26 +649,22 @@ Blockly.Field.prototype.getDisplayText_ = function() {
  * @return {string} Current text.
  */
 Blockly.Field.prototype.getText = function() {
-  return this.text_;
+  if (this.getText_) {
+    var text = this.getText_.call(this);
+    if (text !== null) {
+      return String(text);
+    }
+  }
+  return String(this.getValue());
 };
 
 /**
  * Set the text in this field.  Trigger a rerender of the source block.
- * @param {*} newText New text.
+ * @param {*} _newText New text.
  * @deprecated 2019 setText should not be used directly. Use setValue instead.
  */
-Blockly.Field.prototype.setText = function(newText) {
-  if (newText === null) {
-    // No change if null.
-    return;
-  }
-  newText = String(newText);
-  if (newText === this.text_) {
-    // No change.
-    return;
-  }
-  this.text_ = newText;
-  this.forceRerender();
+Blockly.Field.prototype.setText = function(_newText) {
+  throw new Error('setText method is deprecated');
 };
 
 /**
@@ -787,8 +784,6 @@ Blockly.Field.prototype.doClassValidation_ = function(newValue) {
 Blockly.Field.prototype.doValueUpdate_ = function(newValue) {
   this.value_ = newValue;
   this.isDirty_ = true;
-  // For backwards compatibility.
-  this.text_ = String(newValue);
 };
 
 /**

core/field_angle.js

@@ -294,13 +294,8 @@ Blockly.FieldAngle.prototype.setAngle = function(angle) {
   }
 
   // Update value.
-  var angleString = String(angle);
-  if (angleString != this.text_) {
-    this.htmlInput_.value = angle;
-    this.setValue(angle);
-    // Always render the input angle.
-    this.text_ = angleString;
-    this.forceRerender();
+  if (angle != this.value_) {
+    this.setEditorValue_(angle);
   }
 };
 

core/field_dropdown.js

@@ -57,25 +57,44 @@ Blockly.FieldDropdown = function(menuGenerator, opt_validator) {
   if (typeof menuGenerator != 'function') {
     Blockly.FieldDropdown.validateOptions_(menuGenerator);
   }
-  
-  /**
-   * A reference to the currently selected menu item.
-   * @type {Blockly.MenuItem}
-   * @private
-   */
-  this.selectedMenuItem_ = null;
 
   this.menuGenerator_ = menuGenerator;
 
   this.trimOptions_();
   var firstTuple = this.getOptions()[0];
 
+  /**
+   * The currently selected index. A value of -1 indicates no option
+   * has been selected.
+   * @type {number}
+   * @private
+   */
+  this.selectedIndex_ = -1;
+
   // Call parent's constructor.
   Blockly.FieldDropdown.superClass_.constructor.call(this, firstTuple[1],
       opt_validator);
+
+  /**
+   * A reference to the currently selected menu item.
+   * @type {Blockly.MenuItem}
+   * @private
+   */
+  this.selectedMenuItem_ = null;
 };
 goog.inherits(Blockly.FieldDropdown, Blockly.Field);
 
+/**
+ * Dropdown image properties.
+ * @typedef {{
+  *            src:string,
+  *            alt:string,
+  *            width:number,
+  *            height:number
+  *          }}
+  */
+Blockly.FieldDropdown.ImageProperties;
+
 /**
  * Construct a FieldDropdown from a JSON arg object.
  * @param {!Object} options A JSON object with options (options).
@@ -120,7 +139,8 @@ Blockly.FieldDropdown.IMAGE_Y_OFFSET = 5;
  * @const
  * @private
  */
-Blockly.FieldDropdown.IMAGE_Y_PADDING = Blockly.FieldDropdown.IMAGE_Y_OFFSET * 2;
+Blockly.FieldDropdown.IMAGE_Y_PADDING =
+    Blockly.FieldDropdown.IMAGE_Y_OFFSET * 2;
 
 /**
  * Android can't (in 2014) display "▾", so use "▼" instead.
@@ -140,14 +160,6 @@ Blockly.FieldDropdown.prototype.CURSOR = 'default';
  */
 Blockly.FieldDropdown.prototype.imageElement_ = null;
 
-/**
- * Object with src, height, width, and alt attributes if currently selected
- * option is an image, or null.
- * @type {Object}
- * @private
- */
-Blockly.FieldDropdown.prototype.imageJson_ = null;
-
 /**
  * Create the block UI for this dropdown.
  * @package
@@ -187,15 +199,18 @@ Blockly.FieldDropdown.prototype.showEditor_ = function() {
       /** @type {!Element} */ (this.menu_.getElement()), 'blocklyDropdownMenu');
 
   this.positionMenu_(this.menu_);
-  // Scroll the dropdown to show the selected menu item.
-  if (this.selectedMenuItem_) {
-    Blockly.utils.style.scrollIntoContainerView(
-        this.selectedMenuItem_.getElement(), this.menu_.getElement());
-  }
+
   // Focusing needs to be handled after the menu is rendered and positioned.
   // Otherwise it will cause a page scroll to get the misplaced menu in
   // view. See issue #1329.
   this.menu_.focus();
+
+  // Scroll the dropdown to show the selected menu item.
+  if (this.selectedMenuItem_) {
+    Blockly.utils.style.scrollIntoContainerView(
+        /** @type {!Element} */ (this.selectedMenuItem_.getElement()),
+        /** @type {!Element} */ (this.menu_.getElement()));
+  }
 };
 
 /**
@@ -449,14 +464,7 @@ Blockly.FieldDropdown.prototype.doValueUpdate_ = function(newValue) {
   var options = this.getOptions();
   for (var i = 0, option; option = options[i]; i++) {
     if (option[1] == this.value_) {
-      var content = option[0];
-      if (typeof content == 'object') {
-        this.imageJson_ = content;
-        this.text_ = content.alt;
-      } else {
-        this.imageJson_ = null;
-        this.text_ = content;
-      }
+      this.selectedIndex_ = i;
     }
   }
 };
@@ -486,8 +494,11 @@ Blockly.FieldDropdown.prototype.render_ = function() {
   this.imageElement_.style.display = 'none';
 
   // Show correct element.
-  if (this.imageJson_) {
-    this.renderSelectedImage_();
+  var options = this.getOptions();
+  var selectedOption = this.selectedIndex_ >= 0 &&
+      options[this.selectedIndex_][0];
+  if (selectedOption && typeof selectedOption == 'object') {
+    this.renderSelectedImage_(selectedOption);
   } else {
     this.renderSelectedText_();
   }
@@ -497,19 +508,21 @@ Blockly.FieldDropdown.prototype.render_ = function() {
 
 /**
  * Renders the selected option, which must be an image.
+ * @param {!Blockly.FieldDropdown.ImageProperties} imageJson Selected
+ *   option that must be an image.
  * @private
  */
-Blockly.FieldDropdown.prototype.renderSelectedImage_ = function() {
+Blockly.FieldDropdown.prototype.renderSelectedImage_ = function(imageJson) {
   this.imageElement_.style.display = '';
   this.imageElement_.setAttributeNS(
-      Blockly.utils.dom.XLINK_NS, 'xlink:href', this.imageJson_.src);
-  this.imageElement_.setAttribute('height', this.imageJson_.height);
-  this.imageElement_.setAttribute('width', this.imageJson_.width);
+      Blockly.utils.dom.XLINK_NS, 'xlink:href', imageJson.src);
+  this.imageElement_.setAttribute('height', imageJson.height);
+  this.imageElement_.setAttribute('width', imageJson.width);
 
   var arrowWidth = Blockly.utils.dom.getTextWidth(this.arrow_);
 
-  var imageHeight = Number(this.imageJson_.height);
-  var imageWidth = Number(this.imageJson_.width);
+  var imageHeight = Number(imageJson.height);
+  var imageWidth = Number(imageJson.width);
 
   // Height and width include the border rect.
   this.size_.height = imageHeight + Blockly.FieldDropdown.IMAGE_Y_PADDING;
@@ -521,7 +534,8 @@ Blockly.FieldDropdown.prototype.renderSelectedImage_ = function() {
     this.imageElement_.setAttribute('x', imageX);
     this.textElement_.setAttribute('x', arrowX);
   } else {
-    var arrowX = imageWidth + arrowWidth + Blockly.Field.DEFAULT_TEXT_OFFSET + 1;
+    var arrowX =
+        imageWidth + arrowWidth + Blockly.Field.DEFAULT_TEXT_OFFSET + 1;
     this.textElement_.setAttribute('text-anchor', 'end');
     this.textElement_.setAttribute('x', arrowX);
     this.imageElement_.setAttribute('x', Blockly.Field.DEFAULT_TEXT_OFFSET);
@@ -533,6 +547,7 @@ Blockly.FieldDropdown.prototype.renderSelectedImage_ = function() {
  * @private
  */
 Blockly.FieldDropdown.prototype.renderSelectedText_ = function() {
+  // Retrieves the selected option to display through getText_.
   this.textContent_.nodeValue = this.getDisplayText_();
   this.textElement_.setAttribute('text-anchor', 'start');
   this.textElement_.setAttribute('x', Blockly.Field.DEFAULT_TEXT_OFFSET);
@@ -542,6 +557,26 @@ Blockly.FieldDropdown.prototype.renderSelectedText_ = function() {
       Blockly.Field.X_PADDING;
 };
 
+/**
+ * Use the `getText_` developer hook to override the field's text represenation.
+ * Get the selected option text. If the selected option is an image
+ * we return the image alt text.
+ * @return {?string} Selected option text.
+ * @protected
+ * @override
+ */
+Blockly.FieldDropdown.prototype.getText_ = function() {
+  if (this.selectedIndex_ < 0) {
+    return null;
+  }
+  var options = this.getOptions();
+  var selectedOption = options[this.selectedIndex_][0];
+  if (typeof selectedOption == 'object') {
+    return selectedOption['alt'];
+  }
+  return selectedOption;
+};
+
 /**
  * Validates the data structure to be processed as an options list.
  * @param {?} options The proposed dropdown options.
@@ -565,8 +600,9 @@ Blockly.FieldDropdown.validateOptions_ = function(options) {
       console.error(
           'Invalid option[' + i + ']: Each FieldDropdown option id must be ' +
           'a string. Found ' + tuple[1] + ' in: ', tuple);
-    } else if ((typeof tuple[0] != 'string') &&
-               (typeof tuple[0].src != 'string')) {
+    } else if (tuple[0] &&
+              (typeof tuple[0] != 'string') &&
+              (typeof tuple[0].src != 'string')) {
       foundError = true;
       console.error(
           'Invalid option[' + i + ']: Each FieldDropdown option must have a ' +

core/field_image.js

@@ -70,8 +70,20 @@ Blockly.FieldImage = function(src, width, height,
   this.size_ = new Blockly.utils.Size(imageWidth,
       imageHeight + Blockly.FieldImage.Y_PADDING);
 
-  this.flipRtl_ = opt_flipRtl;
-  this.text_ = opt_alt || '';
+  /**
+   * Whether to flip this image in RTL.
+   * @type {boolean}
+   * @private
+   */
+  this.flipRtl_ = opt_flipRtl || false;
+
+  /**
+   * Alt text of this image.
+   * @type {string}
+   * @private
+   */
+  this.altText_ = opt_alt || '';
+
   this.setValue(src || '');
 
   if (typeof opt_onClick == 'function') {
@@ -134,7 +146,7 @@ Blockly.FieldImage.prototype.initView = function() {
       {
         'height': this.imageHeight_ + 'px',
         'width': this.size_.width + 'px',
-        'alt': this.text_
+        'alt': this.altText_
       },
       this.fieldGroup_);
   this.imageElement_.setAttributeNS(Blockly.utils.dom.XLINK_NS,
@@ -175,30 +187,19 @@ Blockly.FieldImage.prototype.getFlipRtl = function() {
   return this.flipRtl_;
 };
 
-/**
- * Set the alt text of this image.
- * @param {?string} alt New alt text.
- * @override
- * @deprecated 2019 setText has been deprecated for all fields. Instead use
- *    setAlt to set the alt text of the field.
- */
-Blockly.FieldImage.prototype.setText = function(alt) {
-  this.setAlt(alt);
-};
-
 /**
  * Set the alt text of this image.
  * @param {?string} alt New alt text.
  * @public
  */
 Blockly.FieldImage.prototype.setAlt = function(alt) {
-  if (alt === null) {
-    // No change if null.
+  if (alt === this.altText_) {
+    // No change.
     return;
   }
-  this.text_ = alt;
+  this.altText_ = alt || '';
   if (this.imageElement_) {
-    this.imageElement_.setAttribute('alt', alt || '');
+    this.imageElement_.setAttribute('alt', this.altText_);
   }
 };
 
@@ -222,4 +223,15 @@ Blockly.FieldImage.prototype.setOnClickHandler = function(func) {
   this.clickHandler_ = func;
 };
 
+/**
+ * Use the `getText_` developer hook to override the field's text represenation.
+ * Return the image alt text instead.
+ * @return {?string} The image alt text.
+ * @protected
+ * @override
+ */
+Blockly.FieldImage.prototype.getText_ = function() {
+  return this.altText_;
+};
+
 Blockly.fieldRegistry.register('field_image', Blockly.FieldImage);

core/field_textinput.js

@@ -56,6 +56,12 @@ Blockly.FieldTextInput = function(opt_value, opt_validator) {
   }
   Blockly.FieldTextInput.superClass_.constructor.call(this, opt_value,
       opt_validator);
+  /**
+   * A cache of the last value in the html input.
+   * @type {*}
+   * @private
+   */
+  this.htmlInputValue_ = null;
 };
 goog.inherits(Blockly.FieldTextInput, Blockly.Field);
 
@@ -153,7 +159,6 @@ Blockly.FieldTextInput.prototype.doValueUpdate_ = function(newValue) {
   this.value_ = newValue;
   if (!this.isBeingEdited_) {
     // This should only occur if setValue is triggered programmatically.
-    this.text_ = String(newValue);
     this.isDirty_ = true;
   }
 };
@@ -219,7 +224,7 @@ Blockly.FieldTextInput.prototype.showEditor_ = function(opt_quietInput) {
  */
 Blockly.FieldTextInput.prototype.showPromptEditor_ = function() {
   var fieldText = this;
-  Blockly.prompt(Blockly.Msg['CHANGE_VALUE_TITLE'], this.text_,
+  Blockly.prompt(Blockly.Msg['CHANGE_VALUE_TITLE'], this.getText(),
       function(newValue) {
         fieldText.setValue(newValue);
       });
@@ -251,7 +256,7 @@ Blockly.FieldTextInput.prototype.showInlineEditor_ = function(quietInput) {
 Blockly.FieldTextInput.prototype.widgetCreate_ = function() {
   var div = Blockly.WidgetDiv.DIV;
 
-  var htmlInput = document.createElement('input');
+  var htmlInput = /** @type {HTMLInputElement} */ (document.createElement('input'));
   htmlInput.className = 'blocklyHtmlInput';
   htmlInput.setAttribute('spellcheck', this.spellcheck_);
   var fontSize =
@@ -263,7 +268,7 @@ Blockly.FieldTextInput.prototype.widgetCreate_ = function() {
   htmlInput.style.borderRadius = borderRadius;
   div.appendChild(htmlInput);
 
-  htmlInput.value = htmlInput.defaultValue = this.value_;
+  htmlInput.value = htmlInput.defaultValue = String(this.value_);
   htmlInput.untypedDefaultValue_ = this.value_;
   htmlInput.oldValue_ = null;
   if (Blockly.utils.userAgent.GECKO) {
@@ -286,14 +291,17 @@ Blockly.FieldTextInput.prototype.widgetCreate_ = function() {
 Blockly.FieldTextInput.prototype.widgetDispose_ = function() {
   // Finalize value.
   this.isBeingEdited_ = false;
+  this.isTextValid_ = true;
   // No need to call setValue because if the widget is being closed the
   // latest input text has already been validated.
-  if (this.value_ !== this.text_) {
+  if (this.value_ != this.htmlInputValue_) {
     // At the end of an edit the text should be the same as the value. It
     // may not be if the input text is different than the validated text.
-    // We should fix that.
-    this.text_ = String(this.value_);
-    this.isTextValid_ = true;
+    // There are two scenarios where that is the case:
+    // - The text in the input was invalid.
+    // - The text in the input is different to that returned by a validator.
+    // Re-render to fix that.
+    this.htmlInputValue_ = null;
     this.forceRerender();
   }
   // Otherwise don't rerender.
@@ -349,11 +357,14 @@ Blockly.FieldTextInput.prototype.onHtmlInputKeyDown_ = function(e) {
   var tabKey = 9, enterKey = 13, escKey = 27;
   if (e.keyCode == enterKey) {
     Blockly.WidgetDiv.hide();
+    Blockly.DropDownDiv.hideWithoutAnimation();
   } else if (e.keyCode == escKey) {
     this.htmlInput_.value = this.htmlInput_.defaultValue;
     Blockly.WidgetDiv.hide();
+    Blockly.DropDownDiv.hideWithoutAnimation();
   } else if (e.keyCode == tabKey) {
     Blockly.WidgetDiv.hide();
+    Blockly.DropDownDiv.hideWithoutAnimation();
     this.sourceBlock_.tab(this, !e.shiftKey);
     e.preventDefault();
   }
@@ -373,14 +384,29 @@ Blockly.FieldTextInput.prototype.onHtmlInputChange_ = function(_e) {
     //              moved up to the Field setValue method. This would create a
     //              broader fix for all field types.
     Blockly.Events.setGroup(true);
-    this.setValue(text);
-    // Always render the input text.
-    this.text_ = this.htmlInput_.value;
-    this.forceRerender();
+    this.setEditorValue_(text);
     Blockly.Events.setGroup(false);
   }
 };
 
+/**
+ * Set the html input value and the field's internal value. The difference
+ * between this and ``setValue`` is that this also updates the html input
+ * value whilst editing.
+ * @param {*} newValue New value.
+ * @protected
+ */
+Blockly.FieldTextInput.prototype.setEditorValue_ = function(newValue) {
+  this.setValue(newValue);
+  if (this.isBeingEdited_) {
+    this.htmlInput_.value = newValue;
+    // This cache is stored in order to determine if we must re-render when
+    // disposing of the widget div.
+    this.htmlInputValue_ = newValue;
+    this.forceRerender();
+  }
+};
+
 /**
  * Resize the editor to fit the text.
  * @protected
@@ -447,4 +473,21 @@ Blockly.FieldTextInput.nonnegativeIntegerValidator = function(text) {
   return n;
 };
 
+/**
+ * Use the `getText_` developer hook to override the field's text represenation.
+ * When we're currently editing, return the current html value instead.
+ * Otherwise, return null which tells the field to use the default behaviour
+ * (which is a string cast of the field's value).
+ * @return {?string} The html value if we're editing, otherwise null.
+ * @protected
+ * @override
+ */
+Blockly.FieldTextInput.prototype.getText_ = function() {
+  if (this.isBeingEdited_ && this.htmlInput_) {
+    // We are currently editing, return the html input value instead.
+    return this.htmlInput_.value;
+  }
+  return null;
+};
+
 Blockly.fieldRegistry.register('field_input', Blockly.FieldTextInput);

core/field_variable.js

@@ -253,9 +253,7 @@ Blockly.FieldVariable.prototype.doClassValidation_ = function(newId) {
  */
 Blockly.FieldVariable.prototype.doValueUpdate_ = function(newId) {
   this.variable_ = Blockly.Variables.getVariable(this.workspace_, newId);
-  this.value_ = newId;
-  this.text_ = this.variable_.name;
-  this.isDirty_ = true;
+  Blockly.FieldVariable.superClass_.doValueUpdate_.call(this, newId);
 };
 
 /**
@@ -349,14 +347,13 @@ Blockly.FieldVariable.prototype.setTypes_ = function(opt_variableTypes,
  * @package
  */
 Blockly.FieldVariable.prototype.refreshVariableName = function() {
-  this.text_ = this.variable_.name;
   this.forceRerender();
 };
 
 /**
  * Return a sorted list of variable names for variable dropdown menus.
  * Include a special option at the end for creating a new variable name.
- * @return {!Array.<string>} Array of variable names.
+ * @return {!Array.<!Array>} Array of variable names/id tuples.
  * @this {Blockly.FieldVariable}
  */
 Blockly.FieldVariable.dropdownCreate = function() {

tests/mocha/field_image_test.js

@@ -132,41 +132,25 @@ suite('Image Fields', function() {
     });
   });
   suite('setAlt', function() {
-    suite('No Alt -> New Alt', function() {
-      setup(function() {
-        this.imageField = new Blockly.FieldImage('src', 1, 1);
-      });
-      test('Backwards Compat - setText', function() {
-        this.imageField.setText('newAlt');
-        assertValue(this.imageField, 'src', 'newAlt');
-      });
-      test('Null', function() {
-        this.imageField.setText(null);
-        assertValue(this.imageField, 'src', '');
-      });
-      test('Good Alt', function() {
-        this.imageField.setText('newAlt');
-        assertValue(this.imageField, 'src', 'newAlt');
-      });
-    });
-    suite('Alt -> New Alt', function() {
+    suite('Alt', function() {
       setup(function() {
         this.imageField = new Blockly.FieldImage('src', 1, 1, 'alt');
       });
-      test('Backwards Compat - setText', function() {
-        this.imageField.setText('newAlt');
-        assertValue(this.imageField, 'src', 'newAlt');
+      test('Deprecated - setText', function() {
+        chai.assert.throws(function() {
+          this.imageField.setText('newAlt');
+        });
       });
       test('Null', function() {
-        this.imageField.setText(null);
-        assertValue(this.imageField, 'src', 'alt');
+        this.imageField.setAlt(null);
+        assertValue(this.imageField, 'src', '');
       });
       test('Empty String', function() {
-        this.imageField.setText('');
+        this.imageField.setAlt('');
         assertValue(this.imageField, 'src', '');
       });
       test('Good Alt', function() {
-        this.imageField.setText('newAlt');
+        this.imageField.setAlt('newAlt');
         assertValue(this.imageField, 'src', 'newAlt');
       });
     });