From 38ba86c060c865aace1d2566fbfc8f8fbcd465ff Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 16 Aug 2021 18:45:07 +0000 Subject: [PATCH 1/5] fix: add tests for fixing change events --- tests/mocha/block_change_event_test.js | 69 +++++++++++++++++++ tests/mocha/index.html | 2 + tests/mocha/mutator_test.js | 66 ++++++++++++++++++ tests/mocha/test_helpers.js | 94 ++++++++++++++++++++++++++ 4 files changed, 231 insertions(+) create mode 100644 tests/mocha/block_change_event_test.js create mode 100644 tests/mocha/mutator_test.js diff --git a/tests/mocha/block_change_event_test.js b/tests/mocha/block_change_event_test.js new file mode 100644 index 00000000000..242546f2ede --- /dev/null +++ b/tests/mocha/block_change_event_test.js @@ -0,0 +1,69 @@ + +/** + * @license + * Copyright 2021 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +suite('Block Change Event', function() { + setup(function() { + sharedTestSetup.call(this); + this.workspace = new Blockly.Workspace(); + }); + + teardown(function() { + sharedTestTeardown.call(this); + }); + + suite('Undo and Redo', function() { + suite('Mutation', function() { + setup(function() { + defineMutatorBlocks(); + }); + + teardown(function() { + Blockly.Extensions.unregister('xml_mutator'); + Blockly.Extensions.unregister('jso_mutator'); + }); + + suite('XML', function() { + test('Undo', function() { + const block = this.workspace.newBlock('xml_block', 'block_id'); + block.domToMutation( + Blockly.Xml.textToDom('')); + const blockChange = new Blockly.Events.BlockChange( + block, 'mutation', null, '', ''); + blockChange.run(false); + chai.assert.isFalse(block.hasInput); + }); + + test('Redo', function() { + const block = this.workspace.newBlock('xml_block', 'block_id'); + const blockChange = new Blockly.Events.BlockChange( + block, 'mutation', null, '', ''); + blockChange.run(true); + chai.assert.isTrue(block.hasInput); + }); + }); + + suite('JSO', function() { + test('Undo', function() { + const block = this.workspace.newBlock('jso_block', 'block_id'); + block.loadExtraState({hasInput: true}); + const blockChange = new Blockly.Events.BlockChange( + block, 'mutation', null, '', '{"hasInput":true}'); + blockChange.run(false); + chai.assert.isFalse(block.hasInput); + }); + + test('Redo', function() { + const block = this.workspace.newBlock('jso_block', 'block_id'); + const blockChange = new Blockly.Events.BlockChange( + block, 'mutation', null, '', '{"hasInput":true}'); + blockChange.run(true); + chai.assert.isTrue(block.hasInput); + }); + }); + }); + }); +}); diff --git a/tests/mocha/index.html b/tests/mocha/index.html index 42c2e3ca4f0..4f67cf44149 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -54,6 +54,7 @@ + @@ -90,6 +91,7 @@ + diff --git a/tests/mocha/mutator_test.js b/tests/mocha/mutator_test.js new file mode 100644 index 00000000000..ffaf49b6177 --- /dev/null +++ b/tests/mocha/mutator_test.js @@ -0,0 +1,66 @@ + +/** + * @license + * Copyright 2021 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +suite('Mutator', function() { + setup(function() { + sharedTestSetup.call(this); + }); + + suite('Firing change event', function() { + setup(function() { + this.workspace = Blockly.inject('blocklyDiv', {}); + defineMutatorBlocks(); + }); + + teardown(function() { + Blockly.Extensions.unregister('xml_mutator'); + Blockly.Extensions.unregister('jso_mutator'); + sharedTestTeardown.call(this); + }); + + test('No change', function() { + var block = createRenderedBlock(this.workspace, 'xml_block'); + block.mutator.setVisible(true); + var mutatorWorkspace = block.mutator.getWorkspace(); + // Trigger mutator change listener. + createRenderedBlock(mutatorWorkspace, 'checkbox_block'); + chai.assert.isTrue( + this.eventsFireStub.getCalls().every( + ({args}) => + args[0].type !== Blockly.Events.BLOCK_CHANGE || + args[0].element !== 'mutation')); + }); + + test('XML', function() { + var block = createRenderedBlock(this.workspace, 'xml_block'); + block.mutator.setVisible(true); + var mutatorWorkspace = block.mutator.getWorkspace(); + mutatorWorkspace.getBlockById('check_block') + .setFieldValue('TRUE', 'CHECK'); + chai.assert.isTrue( + this.eventsFireStub.getCalls().some( + ({args}) => + args[0].type === Blockly.Events.BLOCK_CHANGE && + args[0].element === 'mutation' && + /<\/mutation>/.test(args[0].newValue))); + }); + + test('JSO', function() { + var block = createRenderedBlock(this.workspace, 'jso_block'); + block.mutator.setVisible(true); + var mutatorWorkspace = block.mutator.getWorkspace(); + mutatorWorkspace.getBlockById('check_block') + .setFieldValue('TRUE', 'CHECK'); + chai.assert.isTrue( + this.eventsFireStub.getCalls().some( + ({args}) => + args[0].type === Blockly.Events.BLOCK_CHANGE && + args[0].element === 'mutation' && + args[0].newValue === '{"hasInput":true}')); + }); + }); +}); diff --git a/tests/mocha/test_helpers.js b/tests/mocha/test_helpers.js index 10699195583..c5c65945321 100644 --- a/tests/mocha/test_helpers.js +++ b/tests/mocha/test_helpers.js @@ -496,6 +496,7 @@ function defineStatementBlock() { "helpUrl": "" }]); } + function defineBasicBlockWithField() { Blockly.defineBlocksWithJsonArray([{ "type": "test_field_block", @@ -510,6 +511,99 @@ function defineBasicBlockWithField() { }]); } +function defineMutatorBlocks() { + Blockly.defineBlocksWithJsonArray([ + { + 'type': 'xml_block', + 'mutator': 'xml_mutator' + }, + { + 'type': 'jso_block', + 'mutator': 'jso_mutator' + }, + { + 'type': 'checkbox_block', + 'message0': '%1', + 'args0': [ + { + 'type': 'field_checkbox', + 'name': 'CHECK' + } + ] + } + ]); + + const xmlMutator = { + hasInput: false, + + mutationToDom: function() { + var mutation = Blockly.utils.xml.createElement('mutation'); + mutation.setAttribute('hasInput', this.hasInput); + return mutation; + }, + + domToMutation: function(mutation) { + this.hasInput = mutation.getAttribute('hasInput') == 'true'; + this.updateShape(); + }, + + decompose: function(workspace) { + var topBlock = workspace.newBlock('checkbox_block', 'check_block'); + topBlock.initSvg(); + topBlock.render(); + return topBlock; + }, + + compose: function(topBlock) { + this.hasInput = topBlock.getFieldValue('CHECK') == 'TRUE'; + this.updateShape(); + }, + + updateShape: function() { + if (this.hasInput && !this.getInput('INPUT')) { + this.appendValueInput('INPUT'); + } else if (!this.hasInput && this.getInput('INPUT')) { + this.removeInput('INPUT'); + } + } + }; + Blockly.Extensions.registerMutator('xml_mutator', xmlMutator); + + const jsoMutator = { + hasInput: false, + + saveExtraState: function() { + return {hasInput: this.hasInput}; + }, + + loadExtraState: function(state) { + this.hasInput = state.hasInput || false; + this.updateShape(); + }, + + decompose: function(workspace) { + var topBlock = workspace.newBlock('checkbox_block', 'check_block'); + topBlock.initSvg(); + topBlock.render(); + return topBlock; + }, + + compose: function(topBlock) { + this.hasInput = topBlock.getFieldValue('CHECK') == 'TRUE'; + this.updateShape(); + }, + + updateShape: function() { + if (this.hasInput && !this.getInput('INPUT')) { + this.appendValueInput('INPUT'); + } else if (!this.hasInput && this.getInput('INPUT')) { + this.removeInput('INPUT'); + } + } + }; + Blockly.Extensions.registerMutator('jso_mutator', jsoMutator); +} + function createTestBlock() { return { id: 'test', From b0431e4a5f2dbcfadc2ba99c0e2f1b8c51913a62 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 16 Aug 2021 18:45:18 +0000 Subject: [PATCH 2/5] fix: change events and insertion markers --- core/events/block_events.js | 34 +++++++++++++++++++++++--------- core/field.js | 2 +- core/insertion_marker_manager.js | 7 ++++++- core/mutator.js | 27 +++++++++++++++++++------ 4 files changed, 53 insertions(+), 17 deletions(-) diff --git a/core/events/block_events.js b/core/events/block_events.js index 8be1dae302e..349c468f41e 100644 --- a/core/events/block_events.js +++ b/core/events/block_events.js @@ -193,23 +193,39 @@ Blockly.Events.BlockChange.prototype.run = function(forward) { block.setInputsInline(!!value); break; case 'mutation': - var oldMutation = ''; - if (block.mutationToDom) { - var oldMutationDom = block.mutationToDom(); - oldMutation = oldMutationDom && Blockly.Xml.domToText(oldMutationDom); - } - if (block.domToMutation) { - var dom = Blockly.Xml.textToDom(/** @type {string} */ (value) || ''); - block.domToMutation(dom); + var oldState = this.getExtraBlockState_(block); + if (block.loadExtraState) { + block.loadExtraState(JSON.parse(/** @type {string} */ (value) || '{}')); + } else if (block.domToMutation) { + block.domToMutation( + Blockly.Xml.textToDom( + /** @type {string} */ (value) || '')); } Blockly.Events.fire(new Blockly.Events.BlockChange( - block, 'mutation', null, oldMutation, value)); + block, 'mutation', null, oldState, value)); break; default: console.warn('Unknown change type: ' + this.element); } }; +/** + * Returns the extra state of the given block (either as XML or a JSO, depending + * on the block's definition). + * @param {!Blockly.BlockSvg} block The block to get the extra state of. + * @return {string} A strigified version of the extra state of the given block. + */ +Blockly.Events.BlockChange.prototype.getExtraBlockState_ = function(block) { + if (block.saveExtraState) { + var state = block.saveExtraState(); + return state ? JSON.stringify(state) : ''; + } else if (block.mutationToDom) { + var state = block.mutationToDom(); + return state ? Blockly.Xml.domToText(state) : ''; + } + return ''; +}; + /** * Class for a block creation event. * @param {!Blockly.Block=} opt_block The created block. Undefined for a blank diff --git a/core/field.js b/core/field.js index 1ed750d6ba3..cb6f0e89071 100644 --- a/core/field.js +++ b/core/field.js @@ -856,11 +856,11 @@ Blockly.Field.prototype.setValue = function(newValue) { return; } + this.doValueUpdate_(newValue); if (source && Blockly.Events.isEnabled()) { Blockly.Events.fire(new (Blockly.Events.get(Blockly.Events.BLOCK_CHANGE))( source, 'field', this.name || null, oldValue, newValue)); } - this.doValueUpdate_(newValue); if (this.isDirty_) { this.forceRerender(); } diff --git a/core/insertion_marker_manager.js b/core/insertion_marker_manager.js index d0fea4502e7..a3dffb1a8fa 100644 --- a/core/insertion_marker_manager.js +++ b/core/insertion_marker_manager.js @@ -277,7 +277,12 @@ Blockly.InsertionMarkerManager.prototype.createMarkerBlock_ = function(sourceBlo try { var result = this.workspace_.newBlock(imType); result.setInsertionMarker(true); - if (sourceBlock.mutationToDom) { + if (sourceBlock.saveExtraState) { + var state = sourceBlock.saveExtraState(); + if (state) { + result.loadExtraState(state); + } + } else if (sourceBlock.mutationToDom) { var oldMutationDom = sourceBlock.mutationToDom(); if (oldMutationDom) { result.domToMutation(oldMutationDom); diff --git a/core/mutator.js b/core/mutator.js index 1bd6d098800..913ebf48c6a 100644 --- a/core/mutator.js +++ b/core/mutator.js @@ -414,8 +414,7 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) { if (this.rootBlock_.workspace == this.workspace_) { Blockly.Events.setGroup(true); var block = this.block_; - var oldMutationDom = block.mutationToDom(); - var oldMutation = oldMutationDom && Blockly.Xml.domToText(oldMutationDom); + var oldExtraState = this.getExtraBlockState_(block); // Switch off rendering while the source block is rebuilt. var savedRendered = block.rendered; @@ -433,11 +432,10 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) { block.render(); } - var newMutationDom = block.mutationToDom(); - var newMutation = newMutationDom && Blockly.Xml.domToText(newMutationDom); - if (oldMutation != newMutation) { + var newExtraState = this.getExtraBlockState_(block); + if (oldExtraState != newExtraState) { Blockly.Events.fire(new (Blockly.Events.get(Blockly.Events.BLOCK_CHANGE))( - block, 'mutation', null, oldMutation, newMutation)); + block, 'mutation', null, oldExtraState, newExtraState)); // Ensure that any bump is part of this mutation's event group. var group = Blockly.Events.getGroup(); setTimeout(function() { @@ -456,6 +454,23 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) { } }; +/** + * Returns the extra state of the given block (either as XML or a JSO, depending + * on the block's definition). + * @param {!Blockly.BlockSvg} block The block to get the extra state of. + * @return {string} A strigified version of the extra state of the given block. + */ +Blockly.Mutator.prototype.getExtraBlockState_ = function(block) { + if (block.saveExtraState) { + var state = block.saveExtraState(); + return state ? JSON.stringify(state) : ''; + } else if (block.mutationToDom) { + var state = block.mutationToDom(); + return state ? Blockly.Xml.domToText(state) : ''; + } + return ''; +}; + /** * Dispose of this mutator. */ From e37040c52160dbfef8719b021923d4a47577f306 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 16 Aug 2021 19:00:55 +0000 Subject: [PATCH 3/5] fix: build: --- core/events/block_events.js | 3 ++- core/mutator.js | 2 +- tests/mocha/.eslintrc.json | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/events/block_events.js b/core/events/block_events.js index 349c468f41e..0eafa94c0d0 100644 --- a/core/events/block_events.js +++ b/core/events/block_events.js @@ -193,7 +193,8 @@ Blockly.Events.BlockChange.prototype.run = function(forward) { block.setInputsInline(!!value); break; case 'mutation': - var oldState = this.getExtraBlockState_(block); + var oldState = this.getExtraBlockState_( + /** @type {!Blockly.BlockSvg} */ (block)); if (block.loadExtraState) { block.loadExtraState(JSON.parse(/** @type {string} */ (value) || '{}')); } else if (block.domToMutation) { diff --git a/core/mutator.js b/core/mutator.js index 913ebf48c6a..4a3bd900bfc 100644 --- a/core/mutator.js +++ b/core/mutator.js @@ -413,7 +413,7 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) { // When the mutator's workspace changes, update the source block. if (this.rootBlock_.workspace == this.workspace_) { Blockly.Events.setGroup(true); - var block = this.block_; + var block = /** @type {!Blockly.BlockSvg} */ (this.block_); var oldExtraState = this.getExtraBlockState_(block); // Switch off rendering while the source block is rebuilt. diff --git a/tests/mocha/.eslintrc.json b/tests/mocha/.eslintrc.json index 47ffaefb5b3..6ee7cbf44f3 100644 --- a/tests/mocha/.eslintrc.json +++ b/tests/mocha/.eslintrc.json @@ -30,6 +30,7 @@ "defineRowBlock": true, "defineStackBlock": true, "defineStatementBlock": true, + "defineMutatorBlocks": true, "dispatchPointerEvent": true, "createFireChangeListenerSpy": true, "createGenUidStubWithReturns": true, From 2165fc772c43eae2b5760f3e4c76648d46c6b1f4 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 19 Aug 2021 20:07:14 +0000 Subject: [PATCH 4/5] fix: remove duplicate code --- core/events/block_events.js | 9 ++++++--- core/mutator.js | 21 ++------------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/core/events/block_events.js b/core/events/block_events.js index 0eafa94c0d0..439603a8615 100644 --- a/core/events/block_events.js +++ b/core/events/block_events.js @@ -193,7 +193,7 @@ Blockly.Events.BlockChange.prototype.run = function(forward) { block.setInputsInline(!!value); break; case 'mutation': - var oldState = this.getExtraBlockState_( + var oldState = Blockly.Events.BlockChange.getExtraBlockState_( /** @type {!Blockly.BlockSvg} */ (block)); if (block.loadExtraState) { block.loadExtraState(JSON.parse(/** @type {string} */ (value) || '{}')); @@ -210,13 +210,16 @@ Blockly.Events.BlockChange.prototype.run = function(forward) { } }; +// TODO (#5397): Encapsulate this in the BlocklyMutationChange event when +// refactoring change events. /** * Returns the extra state of the given block (either as XML or a JSO, depending * on the block's definition). * @param {!Blockly.BlockSvg} block The block to get the extra state of. - * @return {string} A strigified version of the extra state of the given block. + * @return {string} A stringified version of the extra state of the given block. + * @package */ -Blockly.Events.BlockChange.prototype.getExtraBlockState_ = function(block) { +Blockly.Events.BlockChange.getExtraBlockState_ = function(block) { if (block.saveExtraState) { var state = block.saveExtraState(); return state ? JSON.stringify(state) : ''; diff --git a/core/mutator.js b/core/mutator.js index 4a3bd900bfc..5e9c845284b 100644 --- a/core/mutator.js +++ b/core/mutator.js @@ -414,7 +414,7 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) { if (this.rootBlock_.workspace == this.workspace_) { Blockly.Events.setGroup(true); var block = /** @type {!Blockly.BlockSvg} */ (this.block_); - var oldExtraState = this.getExtraBlockState_(block); + var oldExtraState = Blockly.Events.BlockChange.getExtraBlockState_(block); // Switch off rendering while the source block is rebuilt. var savedRendered = block.rendered; @@ -432,7 +432,7 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) { block.render(); } - var newExtraState = this.getExtraBlockState_(block); + var newExtraState = Blockly.Events.BlockChange.getExtraBlockState_(block); if (oldExtraState != newExtraState) { Blockly.Events.fire(new (Blockly.Events.get(Blockly.Events.BLOCK_CHANGE))( block, 'mutation', null, oldExtraState, newExtraState)); @@ -454,23 +454,6 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) { } }; -/** - * Returns the extra state of the given block (either as XML or a JSO, depending - * on the block's definition). - * @param {!Blockly.BlockSvg} block The block to get the extra state of. - * @return {string} A strigified version of the extra state of the given block. - */ -Blockly.Mutator.prototype.getExtraBlockState_ = function(block) { - if (block.saveExtraState) { - var state = block.saveExtraState(); - return state ? JSON.stringify(state) : ''; - } else if (block.mutationToDom) { - var state = block.mutationToDom(); - return state ? Blockly.Xml.domToText(state) : ''; - } - return ''; -}; - /** * Dispose of this mutator. */ From b7ae8c8579938ae55221768f1892962a56c21d75 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 19 Aug 2021 20:13:31 +0000 Subject: [PATCH 5/5] fix: requires --- core/mutator.js | 1 - 1 file changed, 1 deletion(-) diff --git a/core/mutator.js b/core/mutator.js index 5e9c845284b..350dd0bfb76 100644 --- a/core/mutator.js +++ b/core/mutator.js @@ -28,7 +28,6 @@ goog.require('Blockly.utils.Svg'); goog.require('Blockly.utils.toolbox'); goog.require('Blockly.utils.xml'); goog.require('Blockly.WorkspaceSvg'); -goog.require('Blockly.Xml'); goog.requireType('Blockly.Block'); goog.requireType('Blockly.BlockSvg');