diff --git a/core/block.js b/core/block.js index 3d3af48e0..7b786aa78 100644 --- a/core/block.js +++ b/core/block.js @@ -410,12 +410,14 @@ Blockly.Block.prototype.unplugFromRow_ = function(opt_healStack) { return; } - // Only disconnect the child if it's possible to move it to the parent. var childConnection = thisConnection.targetConnection; + // Disconnect the child block. + childConnection.disconnect(); + // Connect child to the parent if possible, otherwise bump away. if (childConnection.checkType_(parentConnection)) { - // Disconnect the child block. - childConnection.disconnect(); parentConnection.connect(childConnection); + } else { + childConnection.onFailedConnect(parentConnection); } }; @@ -464,7 +466,6 @@ Blockly.Block.prototype.unplugFromStack_ = function(opt_healStack) { // 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/core/connection.js b/core/connection.js index 7594642a2..afabedec5 100644 --- a/core/connection.js +++ b/core/connection.js @@ -212,9 +212,9 @@ Blockly.Connection.prototype.connect_ = function(childConnection) { if (orphanBlock.workspace && !orphanBlock.getParent()) { Blockly.Events.setGroup(group); if (orphanBlock.outputConnection) { - orphanBlock.outputConnection.bumpAwayFrom_(parentConnection); + orphanBlock.outputConnection.onFailedConnect(parentConnection); } else if (orphanBlock.previousConnection) { - orphanBlock.previousConnection.bumpAwayFrom_(parentConnection); + orphanBlock.previousConnection.onFailedConnect(parentConnection); } Blockly.Events.setGroup(false); } @@ -456,6 +456,16 @@ Blockly.Connection.prototype.isConnectionAllowed = function(candidate) { return true; }; +/** + * Behavior after a connection attempt fails. + * @param {Blockly.Connection} _otherConnection Connection that this connection + * failed to connect to. + * @package + */ +Blockly.Connection.prototype.onFailedConnect = function(_otherConnection) { + // NOP +}; + /** * Connect this connection to another connection. * @param {!Blockly.Connection} otherConnection Connection to connect to. diff --git a/core/rendered_connection.js b/core/rendered_connection.js index 95d5499e7..907e20db6 100644 --- a/core/rendered_connection.js +++ b/core/rendered_connection.js @@ -348,6 +348,18 @@ Blockly.RenderedConnection.prototype.isConnectionAllowed = function(candidate, candidate); }; +/** + * Behavior after a connection attempt fails. + * @param {Blockly.Connection} otherConnection Connection that this connection + * failed to connect to. + * @package + */ +Blockly.RenderedConnection.prototype.onFailedConnect = function( + otherConnection) { + this.bumpAwayFrom_(otherConnection); +}; + + /** * Disconnect two blocks that are connected by this connection. * @param {!Blockly.Block} parentBlock The superior block. diff --git a/tests/jsunit/block_test.js b/tests/jsunit/block_test.js index ee61edad6..b7c9c6cf6 100644 --- a/tests/jsunit/block_test.js +++ b/tests/jsunit/block_test.js @@ -85,6 +85,17 @@ function assertUnpluggedHealed(blocks) { assertNull(blocks.B.getParent()); } +function assertUnpluggedHealFailed(blocks) { + // A has nothing connected to it. + assertEquals(0, blocks.A.getChildren().length); + // B has nothing connected to it. + assertEquals(0, blocks.B.getChildren().length); + // B is the top of its stack. + assertNull(blocks.B.getParent()); + // C is the top of its stack. + assertNull(blocks.C.getParent()); +} + function setUpRowBlocks() { var blockA = workspace.newBlock('row_block'); var blockB = workspace.newBlock('row_block'); @@ -154,20 +165,7 @@ function test_block_stack_unplug_heal_bad_checks() { // The types don't work. blocks.B.unplug(true); - // 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, 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(blocks.B.getParent()); - // C is the top of its stack. - assertNull(blocks.C.getParent()); + assertUnpluggedHealFailed(blocks); } finally { blockTest_tearDown(); } @@ -207,7 +205,7 @@ function test_block_row_unplug_heal_bad_checks() { // Each block has only one input, but the types don't work. blocks.B.unplug(true); - assertUnpluggedNoheal(blocks); + assertUnpluggedHealFailed(blocks); } finally { blockTest_tearDown(); } diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 500854889..e76ce17c1 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -69,6 +69,16 @@ suite('Blocks', function() { // B is the top of its stack. assertNull(blocks.B.getParent()); } + function assertUnpluggedHealFailed(blocks) { + // A has nothing connected to it. + assertEquals(0, blocks.A.getChildren().length); + // B has nothing connected to it. + assertEquals(0, blocks.B.getChildren().length); + // B is the top of its stack. + assertNull(blocks.B.getParent()); + // C is the top of its stack. + assertNull(blocks.C.getParent()); + } suite('Row', function() { setup(function() { @@ -106,7 +116,7 @@ suite('Blocks', function() { // Each block has only one input, but the types don't work. blocks.B.unplug(true); - assertUnpluggedNoheal(blocks); + assertUnpluggedHealFailed(blocks); }); test('A has multiple inputs', function() { var blocks = this.blocks; @@ -165,7 +175,7 @@ suite('Blocks', function() { this.blocks.B.unplug(true); assertUnpluggedHealed(this.blocks); }); - test.skip('Heal with bad checks', function() { + test('Heal with bad checks', function() { var blocks = this.blocks; // A and C can't connect, but both can connect to B. blocks.A.nextConnection.setCheck('type1'); @@ -174,9 +184,7 @@ suite('Blocks', function() { // The types don't work. blocks.B.unplug(true); - // TODO (#1994): Check types before unplugging. Currently - // everything disconnects, when C should stick with B - assertUnpluggedNoheal(); + assertUnpluggedHealFailed(blocks); }); test('C is Shadow', function() { var blocks = this.blocks; @@ -207,6 +215,16 @@ suite('Blocks', function() { // B is disposed. chai.assert.isTrue(blocks.B.disposed); } + function assertDisposedHealFailed(blocks) { + chai.assert.isFalse(blocks.A.disposed); + chai.assert.isFalse(blocks.C.disposed); + // A has nothing connected to it. + chai.assert.equal(0, blocks.A.getChildren().length); + // B is disposed. + chai.assert.isTrue(blocks.B.disposed); + // C is the top of its stack. + assertNull(blocks.C.getParent()); + } suite('Row', function() { setup(function() { @@ -244,7 +262,7 @@ suite('Blocks', function() { // Each block has only one input, but the types don't work. blocks.B.dispose(true); - assertDisposedNoheal(blocks); + assertDisposedHealFailed(blocks); }); test('A has multiple inputs', function() { var blocks = this.blocks; @@ -303,7 +321,7 @@ suite('Blocks', function() { this.blocks.B.dispose(true); assertDisposedHealed(this.blocks); }); - test.skip('Heal with bad checks', function() { + test('Heal with bad checks', function() { var blocks = this.blocks; // A and C can't connect, but both can connect to B. blocks.A.nextConnection.setCheck('type1'); @@ -312,9 +330,7 @@ suite('Blocks', function() { // The types don't work. blocks.B.dispose(true); - // TODO (#1994): Check types before unplugging. Current C gets - // left behind when it should get disposed with B. - assertDisposedNoheal(blocks); + assertDisposedHealFailed(blocks); }); test('C is Shadow', function() { var blocks = this.blocks;