Disconnect child block always for block.dispose(/* healStack */ true) (#3073)

* Disconnecting child block even if it cannot be connected to parent block.

* Bump disconnected child block.

* Added method for behavior on failed connection and updated expected behavior of unplug in unit tests.

* Removing obsolete TODO and calling new helper method in tests.
This commit is contained in:
Monica Kozbial
2019-09-25 10:47:29 -07:00
committed by GitHub
parent a03660243e
commit e1e9513e86
5 changed files with 68 additions and 31 deletions

View File

@@ -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);

View File

@@ -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.

View File

@@ -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.

View File

@@ -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();
}

View File

@@ -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;