From a14275c04b82c84cf484498aef54a18f791de085 Mon Sep 17 00:00:00 2001 From: alschmiedt Date: Mon, 26 Aug 2019 10:14:18 -0700 Subject: [PATCH] Update connect to work with multiple connecting blocks (#2905) * Update connect to work with multiple connecting blocks * Make logic for connect more readable --- core/keyboard_nav/navigation.js | 111 +++++++++++++++++++++----- tests/mocha/navigation_modify_test.js | 12 ++- tests/mocha/navigation_test.js | 66 +++++++++++++++ 3 files changed, 167 insertions(+), 22 deletions(-) diff --git a/core/keyboard_nav/navigation.js b/core/keyboard_nav/navigation.js index d855d354a..c50db02ad 100644 --- a/core/keyboard_nav/navigation.js +++ b/core/keyboard_nav/navigation.js @@ -484,34 +484,107 @@ Blockly.navigation.modify = function() { }; /** - * Connect the moving connection to the targetConnection. Disconnect the moving - * connection if necessary, and and position the blocks so that the target - * connection does not move. - * @param {Blockly.RenderedConnection} movingConnection The connection to move. - * @param {Blockly.RenderedConnection} targetConnection The connection that - * stays stationary as the movingConnection attaches to it. - * @return {boolean} Whether the connection was successful. + * If the two blocks are compatible move the moving connection to the target + * connection and connect them. + * @param {Blockly.Connection} movingConnection The connection that is being + * moved. + * @param {Blockly.Connection} destConnection The connection to be moved to. + * @return {boolean} True if the connections were connected, false otherwise. + * @private + */ +Blockly.navigation.moveAndConnect_ = function(movingConnection, destConnection) { + var movingBlock = movingConnection.getSourceBlock(); + + if (destConnection.canConnectWithReason_(movingConnection) == + Blockly.Connection.CAN_CONNECT) { + if (destConnection.type == Blockly.PREVIOUS_STATEMENT || + destConnection.type == Blockly.OUTPUT_VALUE) { + movingBlock.positionNearConnection(movingConnection, destConnection); + } + destConnection.connect(movingConnection); + return true; + } + return false; +}; + +/** + * If the given connection is superior find the inferior connection on the + * source block. + * @param {Blockly.Connection} connection The connection trying to be connected. + * @return {Blockly.Connection} The inferior connection or null if none exists. + * @private + */ +Blockly.navigation.getInferiorConnection_ = function(connection) { + var block = connection.getSourceBlock(); + if (!connection.isSuperior()) { + return connection; + } else if (block.previousConnection) { + return block.previousConnection; + } else if (block.outputConnection) { + return block.outputConnection; + } else { + return null; + } +}; + +/** + * If the given connection is inferior tries to find a superior connection to + * connect to. + * @param {Blockly.Connection} connection The connection trying to be connected. + * @return {Blockly.Connection} The superior connection or null if none exists. + * @private + */ +Blockly.navigation.getSuperiorConnection_ = function(connection) { + if (connection.isSuperior()) { + return connection; + } else if (connection.targetConnection) { + return connection.targetConnection; + } + return null; +}; + +/** + * Tries to connect the given connections. + * + * If the given connections are not compatible try finding compatible connections + * on the source blocks of the given connections. + * + * @param {Blockly.Connection} movingConnection The connection that is being + * moved. + * @param {Blockly.Connection} destConnection The connection to be moved to. + * @return {boolean} True if the two connections or their target connections + * were connected, false otherwise. * @package */ -Blockly.navigation.connect = function(movingConnection, targetConnection) { - if (movingConnection) { - var movingBlock = movingConnection.getSourceBlock(); - if (targetConnection.type == Blockly.PREVIOUS_STATEMENT || - targetConnection.type == Blockly.OUTPUT_VALUE) { - movingBlock.positionNearConnection(movingConnection, targetConnection); - } +Blockly.navigation.connect = function(movingConnection, destConnection) { + if (!movingConnection || !destConnection) { + return false; + } + + var movingInferior = Blockly.navigation.getInferiorConnection_(movingConnection); + var destSuperior = Blockly.navigation.getSuperiorConnection_(destConnection); + + var movingSuperior = Blockly.navigation.getSuperiorConnection_(movingConnection); + var destInferior = Blockly.navigation.getInferiorConnection_(destConnection); + + if (movingInferior && destSuperior && + Blockly.navigation.moveAndConnect_(movingInferior, destSuperior)) { + return true; + // Try swapping the inferior and superior connections on the blocks. + } else if (movingSuperior && destInferior && + Blockly.navigation.moveAndConnect_(movingSuperior, destInferior)) { + return true; + // If nothing else worked try connecting the given connections and report + // any errors. + } else { try { - targetConnection.connect(movingConnection); - return true; + destConnection.connect(movingConnection); } catch (e) { - // TODO: Is there anything else useful to do at this catch? - // Perhaps position the block near the target connection? Blockly.navigation.warn('Connection failed with error: ' + e); return false; } } - return false; }; /** diff --git a/tests/mocha/navigation_modify_test.js b/tests/mocha/navigation_modify_test.js index 2d89c0e64..8ffe834f4 100644 --- a/tests/mocha/navigation_modify_test.js +++ b/tests/mocha/navigation_modify_test.js @@ -55,8 +55,10 @@ suite('Insert/Modify', function() { Blockly.navigation.cursor_.setLocation( Blockly.ASTNode.createConnectionNode( this.stack_block_2.nextConnection)); - chai.assert.isFalse(Blockly.navigation.modify()); - chai.assert.isNull(this.stack_block_1.getNextBlock()); + // Connect method will try to find a way to connect blocks with + // incompatible types. + chai.assert.isTrue(Blockly.navigation.modify()); + chai.assert.equal(this.stack_block_1.getNextBlock(), this.stack_block_2); }); test('Cursor on really incompatible connection', function() { Blockly.navigation.cursor_.setLocation( @@ -134,7 +136,11 @@ suite('Insert/Modify', function() { Blockly.navigation.cursor_.setLocation( Blockly.ASTNode.createConnectionNode( this.row_block_2.inputList[0].connection)); - chai.assert.isFalse(Blockly.navigation.modify()); + // Connect method will try to find a way to connect blocks with + // incompatible types. + chai.assert.isTrue(Blockly.navigation.modify()); + chai.assert.equal(this.row_block_1.inputList[0].connection.targetBlock(), + this.row_block_2); }); test('Cursor on really incompatible connection', function() { Blockly.navigation.cursor_.setLocation( diff --git a/tests/mocha/navigation_test.js b/tests/mocha/navigation_test.js index 722c094cd..71ec15533 100644 --- a/tests/mocha/navigation_test.js +++ b/tests/mocha/navigation_test.js @@ -444,6 +444,72 @@ suite('Navigation', function() { }); }); + suite('Connect Blocks', function() { + setup(function() { + Blockly.defineBlocksWithJsonArray([{ + "type": "basic_block", + "message0": "", + "previousStatement": null, + "nextStatement": null, + }]); + + var toolbox = document.getElementById('toolbox-categories'); + this.workspace = Blockly.inject('blocklyDiv', {toolbox: toolbox}); + + var basicBlock = this.workspace.newBlock('basic_block'); + var basicBlock2 = this.workspace.newBlock('basic_block'); + var basicBlock3 = this.workspace.newBlock('basic_block'); + var basicBlock4 = this.workspace.newBlock('basic_block'); + + this.basicBlock = basicBlock; + this.basicBlock2 = basicBlock2; + this.basicBlock3 = basicBlock3; + this.basicBlock4 = basicBlock4; + + this.basicBlock.nextConnection.connect(this.basicBlock2.previousConnection); + + this.basicBlock3.nextConnection.connect(this.basicBlock4.previousConnection); + }); + + teardown(function() { + delete Blockly.Blocks['basic_block']; + this.workspace.dispose(); + Blockly.navigation.currentCategory_ = null; + }); + + test('Connect cursor on previous into stack', function() { + var markedLocation = this.basicBlock2.previousConnection; + var cursorLocation = this.basicBlock3.previousConnection; + + Blockly.navigation.connect(cursorLocation, markedLocation); + + chai.assert.equal(this.basicBlock.nextConnection.targetBlock(), this.basicBlock3); + chai.assert.equal(this.basicBlock2.previousConnection.targetBlock(), this.basicBlock4); + }); + + test('Connect marker on previous into stack', function() { + var markedLocation = this.basicBlock3.previousConnection; + var cursorLocation = this.basicBlock2.previousConnection; + + Blockly.navigation.connect(cursorLocation, markedLocation); + + chai.assert.equal(this.basicBlock.nextConnection.targetBlock(), this.basicBlock3); + chai.assert.equal(this.basicBlock2.previousConnection.targetBlock(), this.basicBlock4); + + }); + + test('Connect cursor on next into stack', function() { + var markedLocation = this.basicBlock2.previousConnection; + var cursorLocation = this.basicBlock4.nextConnection; + + Blockly.navigation.connect(cursorLocation, markedLocation); + + chai.assert.equal(this.basicBlock.nextConnection.targetBlock(), this.basicBlock4); + chai.assert.equal(this.basicBlock3.nextConnection.targetConnection, null); + }); + }); + + suite('Test cursor move on block delete', function() { setup(function() { Blockly.defineBlocksWithJsonArray([{