From 9eacd7d3b9f1b2bb8b25d051663a05375343ff3c Mon Sep 17 00:00:00 2001 From: RoboErikG Date: Thu, 23 Aug 2018 11:18:29 -0700 Subject: [PATCH] Disable/enable function calls along with their definitions (#2019) Fixes #1344 Extends the event listener on procedure caller blocks to also check for their definition being enabled/disabled and update their own state in response. --- blocks/procedures.js | 29 ++ core/block_svg.js | 7 + tests/workspace_svg/index.html | 1 + tests/workspace_svg/procedure_svg_test.js | 373 ++++++++++++++++++++++ 4 files changed, 410 insertions(+) create mode 100644 tests/workspace_svg/procedure_svg_test.js diff --git a/blocks/procedures.js b/blocks/procedures.js index d69507d7e..e0908ef62 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -569,7 +569,9 @@ Blockly.Blocks['procedures_callnoreturn'] = { this.argumentVarModels_ = []; this.quarkConnections_ = {}; this.quarkIds_ = null; + this.previousDisabledState_ = false; }, + /** * Returns the name of the procedure this block calls. * @return {string} Procedure name. @@ -789,6 +791,10 @@ Blockly.Blocks['procedures_callnoreturn'] = { // Block is deleted or is in a flyout. return; } + if (!event.recordUndo) { + // Events not generated by user. Skip handling. + return; + } if (event.type == Blockly.Events.BLOCK_CREATE && event.ids.indexOf(this.id) != -1) { // Look for the case where a procedure call was created (usually through @@ -843,6 +849,27 @@ Blockly.Blocks['procedures_callnoreturn'] = { this.dispose(true, false); Blockly.Events.setGroup(false); } + } else if (event.type == Blockly.Events.CHANGE && event.element == 'disabled') { + var name = this.getProcedureCall(); + var def = Blockly.Procedures.getDefinition(name, this.workspace); + if (def && def.id == event.blockId) { + // in most cases the old group should be '' + var oldGroup = Blockly.Events.getGroup(); + if (oldGroup) { + // This should only be possible programatically and may indicate a problem + // with event grouping. If you see this message please investigate. If the + // use ends up being valid we may need to reorder events in the undo stack. + console.log('Saw an existing group while responding to a definition change'); + } + Blockly.Events.setGroup(event.group); + if (event.newValue) { + this.previousDisabledState_ = this.disabled; + this.setDisabled(true); + } else { + this.setDisabled(this.previousDisabledState_); + } + Blockly.Events.setGroup(oldGroup); + } } }, /** @@ -882,7 +909,9 @@ Blockly.Blocks['procedures_callreturn'] = { this.arguments_ = []; this.quarkConnections_ = {}; this.quarkIds_ = null; + this.previousDisabledState_ = false; }, + getProcedureCall: Blockly.Blocks['procedures_callnoreturn'].getProcedureCall, renameProcedure: Blockly.Blocks['procedures_callnoreturn'].renameProcedure, setProcedureParameters_: diff --git a/core/block_svg.js b/core/block_svg.js index b95504cba..661609f0b 100644 --- a/core/block_svg.js +++ b/core/block_svg.js @@ -669,7 +669,14 @@ Blockly.BlockSvg.prototype.showContextMenu_ = function(e) { Blockly.Msg['ENABLE_BLOCK'] : Blockly.Msg['DISABLE_BLOCK'], enabled: !this.getInheritedDisabled(), callback: function() { + var group = Blockly.Events.getGroup(); + if (!group) { + Blockly.Events.setGroup(true); + } block.setDisabled(!block.disabled); + if (!group) { + Blockly.Events.setGroup(false); + } } }; menuOptions.push(disableOption); diff --git a/tests/workspace_svg/index.html b/tests/workspace_svg/index.html index 41d4a88e5..53006a634 100644 --- a/tests/workspace_svg/index.html +++ b/tests/workspace_svg/index.html @@ -53,6 +53,7 @@ h1 {

Blockly Workspace testing

+ diff --git a/tests/workspace_svg/procedure_svg_test.js b/tests/workspace_svg/procedure_svg_test.js new file mode 100644 index 000000000..42ee75dec --- /dev/null +++ b/tests/workspace_svg/procedure_svg_test.js @@ -0,0 +1,373 @@ +/** + * @license + * Blockly Tests + * + * Copyright 2018 Google Inc. + * https://developers.google.com/blockly/ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +'use strict'; + +goog.require('goog.testing'); +goog.require('goog.testing.MockControl'); + +var savedFireFunc = Blockly.Events.fire; +var workspace; + +function procedureSvgTest_setup() { + Blockly.Events.fire = temporary_fireEvent; + temporary_fireEvent.firedEvents_ = []; + workspace = procedureSvgTest_createWorkspaceWithToolbox(); +} + +function procedureSvgTest_teardown() { + Blockly.Events.fire = savedFireFunc; + workspace.dispose(); +} + +function procedureSvgTest_createWorkspaceWithToolbox() { + var toolbox = document.getElementById('toolbox-categories'); + return Blockly.inject('blocklyDiv', {toolbox: toolbox}); +} + +function test_procedureReturnSetDisabledUpdatesCallers() { + procedureSvgTest_setup(); + try { + var dom = Blockly.Xml.textToDom( + '' + + '' + + 'bar' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + ''); + + + Blockly.Xml.appendDomToWorkspace(dom, workspace); + var barDef = workspace.getBlockById('bar-def'); + var barCalls = [workspace.getBlockById('bar-c1'), workspace.getBlockById('bar-c2')]; + + workspace.clearUndo(); + Blockly.Events.setGroup('g1'); + barDef.setDisabled(true); + Blockly.Events.setGroup(false); + + assertTrue('Callers are disabled when their definition is disabled.', + barCalls[0].disabled && barCalls[1].disabled); + + var firedEvents = workspace.undoStack_; + assertEquals('An event was fired for the definition and each caller.', + 3, firedEvents.length); + assertTrue('Disable events are in the same group.', + 'g1' == firedEvents[0].group + && 'g1' == firedEvents[1].group + && 'g1' == firedEvents[2].group); + + workspace.clearUndo(); + Blockly.Events.setGroup('g2'); + barDef.setDisabled(false); + Blockly.Events.setGroup(false); + + assertTrue('Callers are enabled when their definition is enabled.', + !barCalls[0].disabled && !barCalls[1].disabled); + + assertEquals('An event was fired for the definition and each caller.', + 3, firedEvents.length); + assertTrue('Enable events are in the same group.', + 'g2' == firedEvents[0].group + && 'g2' == firedEvents[1].group + && 'g2' == firedEvents[2].group); + + + } finally { + procedureSvgTest_teardown(); + } +} + + +function test_procedureReturnEnablingRemembersOldCallerState() { + procedureSvgTest_setup(); + try { + var dom = Blockly.Xml.textToDom( + '' + + '' + + 'bar' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + ''); + + + Blockly.Xml.appendDomToWorkspace(dom, workspace); + var barDef = workspace.getBlockById('bar-def'); + var barCalls = [workspace.getBlockById('bar-c1'), workspace.getBlockById('bar-c2')]; + + barCalls[0].setDisabled(true); + workspace.clearUndo(); + Blockly.Events.setGroup('g1'); + barDef.setDisabled(true); + Blockly.Events.setGroup(false); + + assertTrue('Callers are disabled when their definition is disabled.', + barCalls[0].disabled && barCalls[1].disabled); + + var firedEvents = workspace.undoStack_; + assertEquals('An event was fired for the definition and the enabled caller.', + 2, firedEvents.length); + assertTrue('Disable events are in the same group.', + 'g1' == firedEvents[0].group + && 'g1' == firedEvents[1].group); + + workspace.clearUndo(); + Blockly.Events.setGroup('g2'); + barDef.setDisabled(false); + Blockly.Events.setGroup(false); + + + assertTrue('Callers return to their previous state when the definition is enabled.', + barCalls[0].disabled && !barCalls[1].disabled); + + assertEquals('An event was fired for the definition and the enabled caller.', + 2, firedEvents.length); + assertTrue('Disable events are in the same group.', + 'g2' == firedEvents[0].group + && 'g2' == firedEvents[1].group); + + } finally { + procedureSvgTest_teardown(); + } +} + +function test_procedureNoReturnSetDisabledUpdatesCallers() { + procedureSvgTest_setup(); + try { + var dom = Blockly.Xml.textToDom( + '' + + '' + + 'bar' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + ''); + + + Blockly.Xml.appendDomToWorkspace(dom, workspace); + var barDef = workspace.getBlockById('bar-def'); + var barCalls = [workspace.getBlockById('bar-c1'), workspace.getBlockById('bar-c2')]; + + workspace.clearUndo(); + Blockly.Events.setGroup('g1'); + barDef.setDisabled(true); + Blockly.Events.setGroup(false); + + assertTrue('Callers are disabled when their definition is disabled.', + barCalls[0].disabled && barCalls[1].disabled); + + var firedEvents = workspace.undoStack_; + assertEquals('An event was fired for the definition and each caller.', + 3, firedEvents.length); + assertTrue('Disable events are in the same group.', + 'g1' == firedEvents[0].group + && 'g1' == firedEvents[1].group + && 'g1' == firedEvents[2].group); + + workspace.clearUndo(); + Blockly.Events.setGroup('g2'); + barDef.setDisabled(false); + Blockly.Events.setGroup(false); + + assertTrue('Callers are enabled when their definition is enabled.', + !barCalls[0].disabled && !barCalls[1].disabled); + + assertEquals('An event was fired for the definition and each caller.', + 3, firedEvents.length); + assertTrue('Enable events are in the same group.', + 'g2' == firedEvents[0].group + && 'g2' == firedEvents[1].group + && 'g2' == firedEvents[2].group); + + + } finally { + procedureSvgTest_teardown(); + } +} + + +function test_procedureNoReturnEnablingRemembersOldCallerState() { + procedureSvgTest_setup(); + try { + var dom = Blockly.Xml.textToDom( + '' + + '' + + 'bar' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + ''); + + + Blockly.Xml.appendDomToWorkspace(dom, workspace); + var barDef = workspace.getBlockById('bar-def'); + var barCalls = [workspace.getBlockById('bar-c1'), workspace.getBlockById('bar-c2')]; + + barCalls[0].setDisabled(true); + workspace.clearUndo(); + Blockly.Events.setGroup('g1'); + barDef.setDisabled(true); + Blockly.Events.setGroup(false); + + assertTrue('Callers are disabled when their definition is disabled.', + barCalls[0].disabled && barCalls[1].disabled); + + var firedEvents = workspace.undoStack_; + assertEquals('An event was fired for the definition and the enabled caller.', + 2, firedEvents.length); + assertTrue('Disable events are in the same group.', + 'g1' == firedEvents[0].group + && 'g1' == firedEvents[1].group); + + workspace.clearUndo(); + Blockly.Events.setGroup('g2'); + barDef.setDisabled(false); + Blockly.Events.setGroup(false); + + + assertTrue('Callers return to their previous state when the definition is enabled.', + barCalls[0].disabled && !barCalls[1].disabled); + + assertEquals('An event was fired for the definition and the enabled caller.', + 2, firedEvents.length); + assertTrue('Disable events are in the same group.', + 'g2' == firedEvents[0].group + && 'g2' == firedEvents[1].group); + + } finally { + procedureSvgTest_teardown(); + } +} + +/** +* This is a somewhat large test as it's checking interactions between +* three procedure definitions. +*/ +function test_procedureEnableDisableInteractions() { + procedureSvgTest_setup(); + try { + var dom = Blockly.Xml.textToDom( + '' + + '' + + 'bar' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + 'foo' + + '' + + '' + + 'baz' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + ''); + Blockly.Events.disable(); + Blockly.Xml.appendDomToWorkspace(dom, workspace); + Blockly.Events.enable(); + + var barDef = workspace.getBlockById('bar-def'); + var fooDef = workspace.getBlockById('foo-def'); + var bazDef = workspace.getBlockById('baz-def'); + + var barCalls = [workspace.getBlockById('bar-c1'), workspace.getBlockById('bar-c2')]; + var fooCalls = [workspace.getBlockById('foo-c1'), workspace.getBlockById('foo-c2')]; + var bazCall = workspace.getBlockById('baz-c1'); + + barDef.setDisabled(true); + + assertTrue('Callers are disabled when their definition is disabled.', + barCalls[0].disabled && barCalls[1].disabled); + assertTrue('Callers in definitions are disabled by inheritence.', + !fooCalls[0].disabled && fooCalls[0].getInheritedDisabled()); + + fooDef.setDisabled(true); + + assertTrue('Callers are disabled when their definition is disabled', + fooCalls[0].disabled && fooCalls[1].disabled); + + barDef.setDisabled(false); + + assertTrue('Callers are reenabled with their definition', + !barCalls[0].disabled && !barCalls[0].disabled); + + assertTrue('Nested disabled callers remain disabled, not by inheritence.', + fooCalls[0].disabled && !fooCalls[0].getInheritedDisabled()); + + bazDef.setDisabled(true); + + assertTrue('Caller is disabled with its definition', + bazCall.disabled); + + assertTrue('Caller in the return is disabled by inheritence.', + !barCalls[1].disabled && barCalls[1].getInheritedDisabled()); + + barDef.setDisabled(true); + assertTrue('Callers are disabled when their definition is disabled.', + barCalls[0].disabled && barCalls[1].disabled); + + bazDef.setDisabled(false); + + assertTrue('Caller in the return remains disabled, not by inheritence.', + barCalls[1].disabled && !barCalls[1].getInheritedDisabled()); + + } finally { + procedureSvgTest_teardown(); + } + +} \ No newline at end of file