fix: not being able to specifying variable names in toolbox (google#5408

) * fix: not being able to specifying variable names in toolbox * fix: id -> ID
alschmiedt · Sep 20, 2021 · 6d87b85 · 6d87b85
1 parent 4f890d7
commit 6d87b85
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 11 deletions.
diff --git a/core/field_dropdown.js b/core/field_dropdown.js
@@ -172,12 +172,12 @@ FieldDropdown.prototype.fromXml = function(fieldElement) {
 
 /**
  * Saves this field's value.
- * @return {string} The dropdown value held by this field.
+ * @return {*} The dropdown value held by this field.
  * @override
  * @package
  */
 FieldDropdown.prototype.saveState = function() {
-  return /** @type {string} */ (this.getValue());
+  return this.getValue();
 };
 
 /**

diff --git a/core/field_variable.js b/core/field_variable.js
@@ -199,24 +199,32 @@ FieldVariable.prototype.toXml = function(fieldElement) {
 
 /**
  * Saves this field's value.
- * @return {string} The id of the variable referenced by this field.
+ * @return {{id: string}} The ID of the variable referenced by this field.
  * @override
  * @package
  */
 FieldVariable.prototype.saveState = function() {
   // Make sure the variable is initialized.
   this.initModel();
-  return this.variable_.getId();
+  return {
+    'id': this.variable_.getId()
+  };
 };
 
 /**
  * Sets the field's value based on the given state.
- * @param {*} id The id of the variable to assign to this variable field.
+ * @param {*} state The state of the variable to assign to this variable field.
  * @override
  * @package
  */
-FieldVariable.prototype.loadState = function(id) {
-  this.setValue(id);
+FieldVariable.prototype.loadState = function(state) {
+  // This is necessary so that blocks in the flyout can have custom var names.
+  const variable = Variables.getOrCreateVariablePackage(
+      this.sourceBlock_.workspace,
+      state['id'] || null,
+      state['name'],
+      state['type'] || '');
+  this.setValue(variable.getId());
 };
 
 /**

diff --git a/tests/mocha/field_variable_test.js b/tests/mocha/field_variable_test.js
@@ -399,7 +399,7 @@ suite('Variable Fields', function() {
       const field = new Blockly.FieldVariable('x');
       block.getInput('INPUT').appendField(field, 'VAR');
       const jso = Blockly.serialization.blocks.save(block);
-      chai.assert.deepEqual(jso['fields'], {'VAR': 'id2'});
+      chai.assert.deepEqual(jso['fields'], {'VAR': {'id': 'id2'}});
     });
 
     test('Typed', function() {
@@ -408,7 +408,69 @@ suite('Variable Fields', function() {
           new Blockly.FieldVariable('x', undefined, undefined, ['String']);
       block.getInput('INPUT').appendField(field, 'VAR');
       const jso = Blockly.serialization.blocks.save(block);
-      chai.assert.deepEqual(jso['fields'], {'VAR': 'id2'});
+      chai.assert.deepEqual(jso['fields'], {'VAR': {'id': 'id2'}});
+    });
+  });
+
+  suite('Deserialization', function() {
+    setup(function() {
+      this.workspace = new Blockly.Workspace();
+      defineRowBlock();
+      createGenUidStubWithReturns(new Array(10).fill().map((_, i) => 'id' + i));
+    });
+
+    teardown(function() {
+      workspaceTeardown.call(this, this.workspace);
+    });
+
+    test('ID', function() {
+      this.workspace.createVariable('test', '', 'id1');
+      var block = Blockly.serialization.blocks.load({
+        'type': 'variables_get',
+        'fields': {
+          'VAR': {
+            'id': 'id1'
+          }
+        }
+      },
+      this.workspace);
+      var variable = block.getField('VAR').getVariable();
+      chai.assert.equal(variable.name, 'test');
+      chai.assert.equal(variable.type, '');
+      chai.assert.equal(variable.getId(), 'id1');
+    });
+
+    test('Name, untyped', function() {
+      var block = Blockly.serialization.blocks.load({
+        'type': 'variables_get',
+        'fields': {
+          'VAR': {
+            'name': 'test',
+          }
+        }
+      },
+      this.workspace);
+      var variable = block.getField('VAR').getVariable();
+      chai.assert.equal(variable.name, 'test');
+      chai.assert.equal(variable.type, '');
+      chai.assert.equal(variable.getId(), 'id2');
+    });
+
+    test('Name, typed', function() {
+      var block = Blockly.serialization.blocks.load({
+        'type': 'variables_get',
+        'fields': {
+          'VAR': {
+            'name': 'test',
+            'type': 'string',
+          }
+        }
+      },
+      this.workspace);
+      var variable = block.getField('VAR').getVariable();
+      chai.assert.equal(variable.name, 'test');
+      chai.assert.equal(variable.type, 'string');
+      chai.assert.equal(variable.getId(), 'id2');
     });
   });
 });
diff --git a/tests/mocha/jso_deserialization_test.js b/tests/mocha/jso_deserialization_test.js
@@ -80,7 +80,9 @@ suite('JSO Deserialization', function() {
                 'x': 42,
                 'y': 42,
                 'fields': {
-                  'VAR': 'testId'
+                  'VAR': {
+                    'id': 'testId'
+                  }
                 }
               },
             ]
@@ -196,7 +198,9 @@ suite('JSO Deserialization', function() {
                 'x': 42,
                 'y': 42,
                 'fields': {
-                  'VAR': 'testId'
+                  'VAR': {
+                    'id': 'testId'
+                  }
                 }
               },
             ]