Clean up TODOs and move potential variable map to the flyout workspace

core/block.js

@@ -704,7 +704,7 @@ Blockly.Block.prototype.getVarModels = function() {
 
 /**
  * Notification that a variable is renaming.
- * TODO (fenichel): consider deleting this.
+ * TODO (#1498): consider deleting this.
  * If the name matches one of this block's variables, rename it.
  * @param {string} oldName Previous name of variable.
  * @param {string} newName Renamed variable.

core/field_variable.js

@@ -52,7 +52,7 @@ Blockly.FieldVariable = function(varname, opt_validator, opt_variableTypes) {
   this.menuGenerator_ = Blockly.FieldVariable.dropdownCreate;
   this.size_ = new goog.math.Size(0, Blockly.BlockSvg.MIN_BLOCK_Y);
   this.setValidator(opt_validator);
-  // TODO: Add opt_default_type to match default value.  If not set, ''.
+  // TODO (#1499): Add opt_default_type to match default value.  If not set, ''.
   this.defaultVariableName = (varname || '');
   this.defaultType_ = '';
   this.variableTypes = opt_variableTypes;
@@ -135,8 +135,6 @@ Blockly.FieldVariable.prototype.getText = function() {
  *     variable.
  */
 Blockly.FieldVariable.prototype.setValue = function(id) {
-  // TODO: Handle undo a change to go back to the default value.
-  // TODO: Handle null ID, which means "use default".
   var workspace = this.sourceBlock_.workspace;
   var variable = Blockly.Variables.getVariable(workspace, id);
 

core/flyout_base.js

@@ -544,7 +544,7 @@ Blockly.Flyout.prototype.clearOldBlocks_ = function() {
   this.buttons_.length = 0;
 
   // Clear potential variables from the previous showing.
-  this.targetWorkspace_.potentialVariableMap_.clear();
+  this.workspace_.getPotentialVariableMap().clear();
 };
 
 /**

core/variables.js

@@ -376,8 +376,7 @@ Blockly.Variables.getOrCreateVariable = function(workspace, id, name, type) {
  * @private
  */
 Blockly.Variables.getVariable = function(workspace, id, opt_name, opt_type) {
-  var potentialVariableMap =
-      Blockly.Variables.getPotentialVariableMap_(workspace);
+  var potentialVariableMap = workspace.getPotentialVariableMap();
   // Try to just get the variable, by ID if possible.
   if (id) {
     // Look in the real variable map before checking the potential variable map.
@@ -407,8 +406,7 @@ Blockly.Variables.getVariable = function(workspace, id, opt_name, opt_type) {
  * @private
  */
 Blockly.Variables.createVariable_ = function(workspace, id, name, type) {
-  var potentialVariableMap =
-      Blockly.Variables.getPotentialVariableMap_(workspace);
+  var potentialVariableMap = workspace.getPotentialVariableMap();
   // Variables without names get uniquely named for this workspace.
   if (!name) {
     var ws = workspace.isFlyout ? workspace.targetWorkspace : workspace;
@@ -424,24 +422,6 @@ Blockly.Variables.createVariable_ = function(workspace, id, name, type) {
   return variable;
 };
 
-/**
- * Blocks in the flyout can refer to variables that don't exist in the
- * workspace.  For instance, the "get item in list" block refers to an "item"
- * variable regardless of whether the variable has been created yet.
- * A FieldVariable must always refer to a Blockly.VariableModel.  We reconcile
- * these by tracking "potential" variables in the flyout.  These variables
- * become real when references to them are dragged into the main workspace.
- * @param {!Blockly.Workspace} workspace The workspace to get the potential
- *     variable map from.
- * @return {?Blockly.VariableMap} The potential variable map for the given
- *     workspace, or null if it was not a flyout workspace.
- * @private
- */
-Blockly.Variables.getPotentialVariableMap_ = function(workspace) {
-  return workspace.isFlyout && workspace.targetWorkspace ?
-    workspace.targetWorkspace.getPotentialVariableMap() : null;
-};
-
 /**
  * Helper function to get the list of variables that have been added to the
  * workspace after adding a new block, using the given list of variables that

core/workspace.js

@@ -532,13 +532,11 @@ Blockly.Workspace.prototype.getAllVariables = function() {
 /**
  * Return the variable map that contains "potential" variables.  These exist in
  * the flyout but not in the workspace.
- * TODO: Decide if this can be stored on the flyout workspace instead of the
- * main workspace.
  * @return {?Blockly.VariableMap} The potential variable map.
  * @package
  */
 Blockly.Workspace.prototype.getPotentialVariableMap = function() {
-  return this.potentialVariableMap_;
+  return this.isFlyout ? this.potentialVariableMap_ : null;
 };
 
 /**

core/xml.js

@@ -91,8 +91,7 @@ Blockly.Xml.fieldToDomVariable_ = function(field, workspace) {
   var variable = workspace.getVariableById(id);
   if (!variable) {
     if (workspace.isFlyout && workspace.targetWorkspace) {
-      var potentialVariableMap =
-          workspace.targetWorkspace.potentialVariableMap_;
+      var potentialVariableMap = workspace.getPotentialVariableMap();
       if (potentialVariableMap) {
         variable = potentialVariableMap.getVariableById(id);
       }