diff --git a/core/block.js b/core/block.js index f5727690e..d73336697 100644 --- a/core/block.js +++ b/core/block.js @@ -314,27 +314,92 @@ Blockly.Block.prototype.initModel = function() { */ Blockly.Block.prototype.unplug = function(opt_healStack) { if (this.outputConnection) { - if (this.outputConnection.isConnected()) { - // Disconnect from any superior block. - this.outputConnection.disconnect(); - } + this.unplugFromRow_(opt_healStack); } else if (this.previousConnection) { - var previousTarget = null; - if (this.previousConnection.isConnected()) { - // Remember the connection that any next statements need to connect to. - previousTarget = this.previousConnection.targetConnection; - // Detach this block from the parent's tree. - this.previousConnection.disconnect(); - } - var nextBlock = this.getNextBlock(); - if (opt_healStack && nextBlock) { - // Disconnect the next statement. - var nextTarget = this.nextConnection.targetConnection; - nextTarget.disconnect(); - if (previousTarget && previousTarget.checkType_(nextTarget)) { - // Attach the next statement to the previous statement. - previousTarget.connect(nextTarget); + this.unplugFromStack_(opt_healStack); + } +}; + +/** + * Unplug this block's output from an input on another block. Optionally + * reconnect the block's parent to the only child block, if possible. + * @param {boolean=} opt_healStack Disconnect right-side block and connect to + * left-side block. Defaults to false. + * @private + */ +Blockly.Block.prototype.unplugFromRow_ = function(opt_healStack) { + var parentConnection = null; + if (this.outputConnection.isConnected()) { + parentConnection = this.outputConnection.targetConnection; + // Disconnect from any superior block. + this.outputConnection.disconnect(); + } + + // Return early in obvious cases. + if (!parentConnection || !opt_healStack) { + return; + } + + var thisConnection = this.getOnlyValueConnection_(); + if (!thisConnection || !thisConnection.isConnected()) { + // Too many or too few possible connections on this block, or there's + // nothing on the other side of this connection. + return; + } + + // Only disconnect the child if it's possible to move it to the parent. + var childConnection = thisConnection.targetConnection; + if (childConnection.checkType_(parentConnection)) { + // Disconnect the child block. + childConnection.disconnect(); + parentConnection.connect(childConnection); + } +}; + +/** + * Returns the connection on the only value input on the block, or null if the + * number of value inputs is not one. + * @return {Blockly.Connection} The connection on the value input, or null. + * @private + */ +Blockly.Block.prototype.getOnlyValueConnection_ = function() { + var connection = false; + for (var i = 0; i < this.inputList.length; i++) { + var thisConnection = this.inputList[i].connection; + if (thisConnection && thisConnection.type == Blockly.INPUT_VALUE) { + if (connection) { + return null; // More than one value input found. } + connection = thisConnection; + } + } + return connection; +}; + +/** + * Unplug this statement block from its superior block. Optionally reconnect + * the block underneath with the block on top. + * @param {boolean=} opt_healStack Disconnect child statement and reconnect + * stack. Defaults to false. + * @private + */ +Blockly.Block.prototype.unplugFromStack_ = function(opt_healStack) { + var previousTarget = null; + if (this.previousConnection.isConnected()) { + // Remember the connection that any next statements need to connect to. + previousTarget = this.previousConnection.targetConnection; + // Detach this block from the parent's tree. + this.previousConnection.disconnect(); + } + var nextBlock = this.getNextBlock(); + if (opt_healStack && nextBlock) { + // Disconnect the next statement. + var nextTarget = this.nextConnection.targetConnection; + nextTarget.disconnect(); + // TODO (#1994): Check types before unplugging. + if (previousTarget && previousTarget.checkType_(nextTarget)) { + // Attach the next statement to the previous statement. + previousTarget.connect(nextTarget); } } }; diff --git a/tests/jsunit/block_test.js b/tests/jsunit/block_test.js index f2e5f6b8e..d7efed435 100644 --- a/tests/jsunit/block_test.js +++ b/tests/jsunit/block_test.js @@ -61,26 +61,66 @@ function blockTest_tearDown() { workspace.dispose(); } +function assertUnpluggedNoheal(blocks) { + // A has nothing connected to it. + assertEquals(0, blocks.A.getChildren().length); + // B and C are still connected. + assertEquals(blocks.B, blocks.C.getParent()); + // B is the top of its stack. + assertNull(blocks.B.getParent()); +} + +function assertUnpluggedHealed(blocks) { + // A and C are connected. + assertEquals(1, blocks.A.getChildren().length); + assertEquals(blocks.A, blocks.C.getParent()); + // B has nothing connected to it. + assertEquals(0, blocks.B.getChildren().length); + // B is the top of its stack. + assertNull(blocks.B.getParent()); +} + +function setUpRowBlocks() { + var blockA = workspace.newBlock('row_block'); + var blockB = workspace.newBlock('row_block'); + var blockC = workspace.newBlock('row_block'); + + blockA.inputList[0].connection.connect(blockB.outputConnection); + blockB.inputList[0].connection.connect(blockC.outputConnection); + + assertEquals(blockB, blockC.getParent()); + + return { + A: blockA, + B: blockB, + C: blockC + } +} + +function setUpStackBlocks() { + var blockA = workspace.newBlock('stack_block'); + var blockB = workspace.newBlock('stack_block'); + var blockC = workspace.newBlock('stack_block'); + + blockA.nextConnection.connect(blockB.previousConnection); + blockB.nextConnection.connect(blockC.previousConnection); + + assertEquals(blockB, blockC.getParent()); + + return { + A: blockA, + B: blockB, + C: blockC + } +} + + function test_block_stack_unplug_noheal() { blockTest_setUp(); try { - var blockA = workspace.newBlock('stack_block'); - var blockB = workspace.newBlock('stack_block'); - var blockC = workspace.newBlock('stack_block'); - - blockA.nextConnection.connect(blockB.previousConnection); - blockB.nextConnection.connect(blockC.previousConnection); - - assertEquals(blockB, blockC.getParent()); - - blockB.unplug(); - - // A has nothing connected to it. - assertEquals(0, blockA.getChildren().length); - // B and C are still connected. - assertEquals(blockB, blockC.getParent()); - // B is the top of its stack. - assertNull(blockB.getParent()); + var blocks = setUpStackBlocks(); + blocks.B.unplug(); + assertUnpluggedNoheal(blocks); } finally { blockTest_tearDown(); } @@ -89,24 +129,40 @@ function test_block_stack_unplug_noheal() { function test_block_stack_unplug_heal() { blockTest_setUp(); try { - var blockA = workspace.newBlock('stack_block'); - var blockB = workspace.newBlock('stack_block'); - var blockC = workspace.newBlock('stack_block'); + var blocks = setUpStackBlocks(); + blocks.B.unplug(true); + assertUnpluggedHealed(blocks); + } finally { + blockTest_tearDown(); + } +} - blockA.nextConnection.connect(blockB.previousConnection); - blockB.nextConnection.connect(blockC.previousConnection); +function test_block_stack_unplug_heal_bad_checks() { + blockTest_setUp(); + try { + var blocks = setUpStackBlocks(); - assertEquals(blockB, blockC.getParent()); + // A and C can't connect, but both can connect to B. + blocks.A.nextConnection.setCheck('type1'); + blocks.C.previousConnection.setCheck('type2'); - blockB.unplug(true); + // The types don't work. + blocks.B.unplug(true); - // A and C are connected. - assertEquals(1, blockA.getChildren().length); - assertEquals(blockA, blockC.getParent()); + // Stack blocks unplug before checking whether the types match. + // TODO (#1994): Check types before unplugging. + // A has nothing connected to it. + assertEquals(0, blocks.A.getChildren().length); // B has nothing connected to it. - assertEquals(0, blockB.getChildren().length); + assertEquals(0, blocks.B.getChildren().length); + // C has nothing connected to it. + assertEquals(0, blocks.C.getChildren().length); + // A is the top of its stack. + assertNull(blocks.A.getParent()); // B is the top of its stack. - assertNull(blockB.getParent()); + assertNull(blocks.B.getParent()); + // C is the top of its stack. + assertNull(blocks.C.getParent()); } finally { blockTest_tearDown(); } @@ -115,23 +171,9 @@ function test_block_stack_unplug_heal() { function test_block_row_unplug_noheal() { blockTest_setUp(); try { - var blockA = workspace.newBlock('row_block'); - var blockB = workspace.newBlock('row_block'); - var blockC = workspace.newBlock('row_block'); - - blockA.inputList[0].connection.connect(blockB.outputConnection); - blockB.inputList[0].connection.connect(blockC.outputConnection); - - assertEquals(blockB, blockC.getParent()); - - blockB.unplug(false); - - // A has nothing connected to it. - assertEquals(0, blockA.getChildren().length); - // B and C are still connected. - assertEquals(blockB, blockC.getParent()); - // B is the top of its stack. - assertNull(blockB.getParent()); + var blocks = setUpRowBlocks(); + blocks.B.unplug(false); + assertUnpluggedNoheal(blocks); } finally { blockTest_tearDown(); } @@ -140,27 +182,72 @@ function test_block_row_unplug_noheal() { function test_block_row_unplug_heal() { blockTest_setUp(); try { - var blockA = workspace.newBlock('row_block'); - var blockB = workspace.newBlock('row_block'); - var blockC = workspace.newBlock('row_block'); - - blockA.inputList[0].connection.connect(blockB.outputConnection); - blockB.inputList[0].connection.connect(blockC.outputConnection); - - assertEquals(blockB, blockC.getParent()); - - blockB.unplug(true); - - // Unplugging in a row doesn't heal the stack, regardless of the value of - // the healStack argument. These are the same asserts as - // test_block_row_unplug_noheal. - - // A has nothing connected to it. - assertEquals(0, blockA.getChildren().length); - // B and C are still connected. - assertEquals(blockB, blockC.getParent()); - // B is the top of its stack. - assertNull(blockB.getParent()); + var blocks = setUpRowBlocks(); + // Each block has only one input, and the types work. + blocks.B.unplug(true); + assertUnpluggedHealed(blocks); + } finally { + blockTest_tearDown(); + } +} + +function test_block_row_unplug_heal_bad_checks() { + blockTest_setUp(); + try { + var blocks = setUpRowBlocks(); + + // A and C can't connect, but both can connect to B. + blocks.A.inputList[0].connection.setCheck('type1'); + blocks.C.outputConnection.setCheck('type2'); + + // Each block has only one input, but the types don't work. + blocks.B.unplug(true); + assertUnpluggedNoheal(blocks); + } finally { + blockTest_tearDown(); + } +} + +function test_block_row_unplug_multi_inputs_parent() { + blockTest_setUp(); + try { + var blocks = setUpRowBlocks(); + // Add extra input to parent + blocks.A.appendValueInput("INPUT").setCheck(null); + + // Parent block has multiple inputs. + blocks.B.unplug(true); + assertUnpluggedHealed(blocks); + } finally { + blockTest_tearDown(); + } +} + +function test_block_row_unplug_multi_inputs_middle() { + blockTest_setUp(); + try { + var blocks = setUpRowBlocks(); + // Add extra input to middle block + blocks.B.appendValueInput("INPUT").setCheck(null); + + // Middle block has multiple inputs. + blocks.B.unplug(true); + assertUnpluggedNoheal(blocks); + } finally { + blockTest_tearDown(); + } +} + +function test_block_row_unplug_multi_inputs_child() { + blockTest_setUp(); + try { + var blocks = setUpRowBlocks(); + // Add extra input to child block + blocks.C.appendValueInput("INPUT").setCheck(null); + + // Child block input count doesn't matter. + blocks.B.unplug(true); + assertUnpluggedHealed(blocks); } finally { blockTest_tearDown(); }