From 0e0c5fea0edef253f33624bcb57b5542172b2be3 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 10 Sep 2021 16:11:20 +0000 Subject: [PATCH] fix: dragging variables from flyout (#5434) * fix: dragging variables from flyout * fix: rename positionBlock_ to positionNewBlock_ * fix: type * fix: try alternative method for handling variables in flyout --- core/block.js | 2 +- core/field.js | 5 +- core/field_variable.js | 14 ++++-- core/flyout_base.js | 15 +++++- core/serialization/blocks.js | 44 ++++++++++------ tests/mocha/field_variable_test.js | 55 +++++++++++++++----- tests/mocha/jso_serialization_test.js | 72 +++++++++++++++++++++++++++ 7 files changed, 173 insertions(+), 34 deletions(-) diff --git a/core/block.js b/core/block.js index eb072da342f..c73649a4ceb 100644 --- a/core/block.js +++ b/core/block.js @@ -1074,7 +1074,7 @@ Block.prototype.getField = function(name) { /** * Return all variables referenced by this block. - * @return {!Array} List of variable names. + * @return {!Array} List of variable ids. */ Block.prototype.getVars = function() { const vars = []; diff --git a/core/field.js b/core/field.js index 647a366c8c7..a82a3116494 100644 --- a/core/field.js +++ b/core/field.js @@ -437,10 +437,13 @@ Field.prototype.toXml = function(fieldElement) { /** * Saves this fields value as something which can be serialized to JSON. Should * only be called by the serialization system. + * @param {boolean=} _doFullSerialization If true, this signals to the field that + * if it normally just saves a reference to some state (eg variable fields) + * it should instead serialize the full state of the thing being referenced. * @return {*} JSON serializable state. * @package */ -Field.prototype.saveState = function() { +Field.prototype.saveState = function(_doFullSerialization) { const legacyState = this.saveLegacyState(Field); if (legacyState !== null) { return legacyState; diff --git a/core/field_variable.js b/core/field_variable.js index 7e76f7e3539..331b06dc0d9 100644 --- a/core/field_variable.js +++ b/core/field_variable.js @@ -199,20 +199,28 @@ FieldVariable.prototype.toXml = function(fieldElement) { /** * Saves this field's value. - * @return {*} The ID of the variable referenced by this field. + * @param {boolean=} doFullSerialization If true, the variable field will + * serialize the full state of the field being referenced (ie ID, name, + * and type) rather than just a reference to it (ie ID). + * @return {*} The state of the variable field. * @override * @package */ -FieldVariable.prototype.saveState = function() { +FieldVariable.prototype.saveState = function(doFullSerialization) { const legacyState = this.saveLegacyState(FieldVariable); if (legacyState !== null) { return legacyState; } // Make sure the variable is initialized. this.initModel(); - return { + const state = { 'id': this.variable_.getId() }; + if (doFullSerialization) { + state['name'] = this.variable_.name; + state['type'] = this.variable_.type; + } + return state; }; /** diff --git a/core/flyout_base.js b/core/flyout_base.js index 4256485745e..49b291639ff 100644 --- a/core/flyout_base.js +++ b/core/flyout_base.js @@ -1079,6 +1079,20 @@ Flyout.prototype.placeNewBlock_ = function(oldBlock) { targetWorkspace.setResizesEnabled(false); const block = /** @type {!BlockSvg} */ (blocks.load(json, targetWorkspace)); + this.positionNewBlock_(oldBlock, block); + + return block; +}; + +/** + * Positions a block on the target workspace. + * @param {!BlockSvg} oldBlock The flyout block being copied. + * @param {!BlockSvg} block The block to posiiton. + * @private + */ +Flyout.prototype.positionNewBlock_ = function(oldBlock, block) { + const targetWorkspace = this.targetWorkspace; + // The offset in pixels between the main workspace's origin and the upper left // corner of the injection div. const mainOffsetPixels = targetWorkspace.getOriginOffsetInPixels(); @@ -1105,7 +1119,6 @@ Flyout.prototype.placeNewBlock_ = function(oldBlock) { finalOffset.scale(1 / targetWorkspace.scale); block.moveBy(finalOffset.x, finalOffset.y); - return block; }; /** diff --git a/core/serialization/blocks.js b/core/serialization/blocks.js index a3ee95acf9c..5d09190fb54 100644 --- a/core/serialization/blocks.js +++ b/core/serialization/blocks.js @@ -71,7 +71,8 @@ exports.State = State; * Returns the state of the given block as a plain JavaScript object. * @param {!Block} block The block to serialize. * @param {{addCoordinates: (boolean|undefined), addInputBlocks: - * (boolean|undefined), addNextBlocks: (boolean|undefined)}=} param1 + * (boolean|undefined), addNextBlocks: (boolean|undefined), + * doFullSerialization: (boolean|undefined)}=} param1 * addCoordinates: If true, the coordinates of the block are added to the * serialized state. False by default. * addinputBlocks: If true, children of the block which are connected to @@ -79,6 +80,10 @@ exports.State = State; * addNextBlocks: If true, children of the block which are connected to the * block's next connection (if it exists) will be serialized. * True by default. + * doFullSerialization: If true, fields that normally just save a reference + * to some external state (eg variables) will instead serialize all of the + * info about that state. This supports deserializing the block into a + * workspace where that state doesn't yet exist. True by default. * @return {?State} The serialized state of the block, or null if the block * could not be serialied (eg it was an insertion marker). */ @@ -87,7 +92,8 @@ const save = function( { addCoordinates = false, addInputBlocks = true, - addNextBlocks = true + addNextBlocks = true, + doFullSerialization = true, } = {} ) { if (block.isInsertionMarker()) { @@ -105,12 +111,12 @@ const save = function( saveAttributes(block, state); saveExtraState(block, state); saveIcons(block, state); - saveFields(block, state); + saveFields(block, state, doFullSerialization); if (addInputBlocks) { - saveInputBlocks(block, state); + saveInputBlocks(block, state, doFullSerialization); } if (addNextBlocks) { - saveNextBlocks(block, state); + saveNextBlocks(block, state, doFullSerialization); } return state; @@ -195,8 +201,11 @@ const saveIcons = function(block, state) { * Adds the state of all of the fields on the block to the given state object. * @param {!Block} block The block to serialize the field state of. * @param {!State} state The state object to append to. + * @param {boolean} doFullSerialization Whether or not to serialize the full + * state of the field (rather than possibly saving a reference to some + * state). */ -const saveFields = function(block, state) { +const saveFields = function(block, state, doFullSerialization) { let hasFieldState = false; const fields = Object.create(null); for (let i = 0; i < block.inputList.length; i++) { @@ -205,7 +214,7 @@ const saveFields = function(block, state) { const field = input.fieldRow[j]; if (field.isSerializable()) { hasFieldState = true; - fields[field.name] = field.saveState(); + fields[field.name] = field.saveState(doFullSerialization); } } } @@ -219,16 +228,17 @@ const saveFields = function(block, state) { * connected to inputs) to the given state object. * @param {!Block} block The block to serialize the input blocks of. * @param {!State} state The state object to append to. + * @param {boolean} doFullSerialization Whether or not to do full serialization. */ -const saveInputBlocks = function(block, state) { +const saveInputBlocks = function(block, state, doFullSerialization) { const inputs = Object.create(null); for (let i = 0; i < block.inputList.length; i++) { const input = block.inputList[i]; if (input.type === inputTypes.DUMMY) { continue; } - const connectionState = - saveConnection(/** @type {!Connection} */ (input.connection)); + const connectionState = saveConnection( + /** @type {!Connection} */ (input.connection), doFullSerialization); if (connectionState) { inputs[input.name] = connectionState; } @@ -244,12 +254,14 @@ const saveInputBlocks = function(block, state) { * state object. * @param {!Block} block The block to serialize the next blocks of. * @param {!State} state The state object to append to. + * @param {boolean} doFullSerialization Whether or not to do full serialization. */ -const saveNextBlocks = function(block, state) { +const saveNextBlocks = function(block, state, doFullSerialization) { if (!block.nextConnection) { return; } - const connectionState = saveConnection(block.nextConnection); + const connectionState = saveConnection( + block.nextConnection, doFullSerialization); if (connectionState) { state['next'] = connectionState; } @@ -262,8 +274,9 @@ const saveNextBlocks = function(block, state) { * blocks of. * @return {?ConnectionState} An object containing the state of any connected * shadow block, or any connected real block. + * @param {boolean} doFullSerialization Whether or not to do full serialization. */ -const saveConnection = function(connection) { +const saveConnection = function(connection, doFullSerialization) { const shadow = connection.getShadowState(true); const child = connection.targetBlock(); if (!shadow && !child) { @@ -274,7 +287,7 @@ const saveConnection = function(connection) { state['shadow'] = shadow; } if (child && !child.isShadow()) { - state['block'] = save(child); + state['block'] = save(child, {doFullSerialization}); } return state; }; @@ -640,7 +653,8 @@ class BlockSerializer { save(workspace) { const blockState = []; for (const block of workspace.getTopBlocks(false)) { - const state = saveBlock(block, {addCoordinates: true}); + const state = saveBlock( + block, {addCoordinates: true, doFullSerialization: false}); if (state) { blockState.push(state); } diff --git a/tests/mocha/field_variable_test.js b/tests/mocha/field_variable_test.js index 5eb2a9ce033..0bee6acdac2 100644 --- a/tests/mocha/field_variable_test.js +++ b/tests/mocha/field_variable_test.js @@ -394,21 +394,50 @@ suite('Variable Fields', function() { workspaceTeardown.call(this, this.workspace); }); - test('Untyped', function() { - const block = this.workspace.newBlock('row_block'); - 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': {'id': 'id2'}}); + suite('Full', function() { + test('Untyped', function() { + const block = this.workspace.newBlock('row_block'); + 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': {'id': 'id2', 'name': 'x', 'type': ''}}); + }); + + test('Typed', function() { + const block = this.workspace.newBlock('row_block'); + const field = + 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': {'id': 'id2', 'name': 'x', 'type': 'String'}}); + }); }); - test('Typed', function() { - const block = this.workspace.newBlock('row_block'); - const field = - 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': {'id': 'id2'}}); + suite('Not full', function() { + test('Untyped', function() { + const block = this.workspace.newBlock('row_block'); + const field = new Blockly.FieldVariable('x'); + block.getInput('INPUT').appendField(field, 'VAR'); + const jso = Blockly.serialization.blocks.save( + block, {doFullSerialization: false}); + chai.assert.deepEqual(jso['fields'], {'VAR': {'id': 'id2'}}); + chai.assert.isUndefined(jso['fields']['VAR']['name']); + chai.assert.isUndefined(jso['fields']['VAR']['type']); + }); + + test('Typed', function() { + const block = this.workspace.newBlock('row_block'); + const field = + new Blockly.FieldVariable('x', undefined, undefined, 'String'); + block.getInput('INPUT').appendField(field, 'VAR'); + const jso = Blockly.serialization.blocks.save( + block, {doFullSerialization: false}); + chai.assert.deepEqual(jso['fields'], {'VAR': {'id': 'id2'}}); + chai.assert.isUndefined(jso['fields']['VAR']['name']); + chai.assert.isUndefined(jso['fields']['VAR']['type']); + }); }); }); diff --git a/tests/mocha/jso_serialization_test.js b/tests/mocha/jso_serialization_test.js index cfd931d3eb6..1b40a4b976d 100644 --- a/tests/mocha/jso_serialization_test.js +++ b/tests/mocha/jso_serialization_test.js @@ -699,6 +699,78 @@ suite('JSO Serialization', function() { }); }); }); + + suite('Do full serialization', function() { + suite('True', function() { + test('Single block', function() { + var block = this.workspace.newBlock('variables_get'); + var jso = Blockly.serialization.blocks.save(block); + chai.assert.deepEqual( + jso['fields']['VAR'], {'id': 'id2', 'name': 'item', 'type': ''}); + }); + + test('Input block', function() { + var block = this.workspace.newBlock('row_block'); + var childBlock = this.workspace.newBlock('variables_get'); + block.getInput('INPUT').connection.connect( + childBlock.outputConnection); + var jso = Blockly.serialization.blocks.save(block); + chai.assert.deepEqual( + jso['inputs']['INPUT']['block']['fields']['VAR'], + {'id': 'id4', 'name': 'item', 'type': ''}); + }); + + test('Next block', function() { + var block = this.workspace.newBlock('stack_block'); + var childBlock = this.workspace.newBlock('variables_set'); + block.nextConnection.connect(childBlock.previousConnection); + var jso = Blockly.serialization.blocks.save(block); + chai.assert.deepEqual( + jso['next']['block']['fields']['VAR'], + {'id': 'id4', 'name': 'item', 'type': ''}); + }); + }); + + suite('False', function() { + test('Single block', function() { + var block = this.workspace.newBlock('variables_get'); + var jso = Blockly.serialization.blocks.save( + block, {doFullSerialization: false}); + chai.assert.deepEqual(jso['fields']['VAR'], {'id': 'id2'}); + chai.assert.isUndefined(jso['fields']['VAR']['name']); + chai.assert.isUndefined(jso['fields']['VAR']['type']); + }); + + test('Input block', function() { + var block = this.workspace.newBlock('row_block'); + var childBlock = this.workspace.newBlock('variables_get'); + block.getInput('INPUT').connection.connect( + childBlock.outputConnection); + var jso = Blockly.serialization.blocks.save( + block, {doFullSerialization: false}); + chai.assert.deepEqual( + jso['inputs']['INPUT']['block']['fields']['VAR'], {'id': 'id4'}); + chai.assert.isUndefined( + jso['inputs']['INPUT']['block']['fields']['VAR']['name']); + chai.assert.isUndefined( + jso['inputs']['INPUT']['block']['fields']['VAR']['type']); + }); + + test('Next block', function() { + var block = this.workspace.newBlock('stack_block'); + var childBlock = this.workspace.newBlock('variables_set'); + block.nextConnection.connect(childBlock.previousConnection); + var jso = Blockly.serialization.blocks.save( + block, {doFullSerialization: false}); + chai.assert.deepEqual( + jso['next']['block']['fields']['VAR'], {'id': 'id4'}); + chai.assert.isUndefined( + jso['next']['block']['fields']['VAR']['name']); + chai.assert.isUndefined( + jso['next']['block']['fields']['VAR']['type']); + }); + }); + }); }); suite('Variables', function() {