From a96eec8a70627485562f2619d5b31450ea837b23 Mon Sep 17 00:00:00 2001 From: Maribeth Bottorff Date: Fri, 3 Feb 2023 14:44:53 -0800 Subject: [PATCH] fix: don't splice into unmovable stacks (#6800) * fix: don't splice into unmovable stacks * chore: fix the tests and comments * chore: format * chore: clean up xml * chore: use test row blocks --- core/connection_checker.ts | 8 +- tests/mocha/connection_checker_test.js | 107 +++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/core/connection_checker.ts b/core/connection_checker.ts index ccb54f539..498245303 100644 --- a/core/connection_checker.ts +++ b/core/connection_checker.ts @@ -203,7 +203,7 @@ export class ConnectionChecker implements IConnectionChecker { /** * Check whether this connection can be made by dragging. * - * @param a Connection to compare. + * @param a Connection to compare (on the block that's being dragged). * @param b Connection to compare against. * @param distance The maximum allowable distance between connections. * @returns True if the connection is allowed during a drag. @@ -250,6 +250,12 @@ export class ConnectionChecker implements IConnectionChecker { !b.targetBlock()!.isShadow() && b.targetBlock()!.nextConnection) { return false; } + + // Don't offer to splice into a stack where the connected block is + // immovable. + if (b.targetBlock() && !b.targetBlock()!.isMovable()) { + return false; + } break; } default: diff --git a/tests/mocha/connection_checker_test.js b/tests/mocha/connection_checker_test.js index d86d48bfb..960161316 100644 --- a/tests/mocha/connection_checker_test.js +++ b/tests/mocha/connection_checker_test.js @@ -341,4 +341,111 @@ suite('Connection checker', function() { chai.assert.isFalse(this.checker.doTypeChecks(this.con1, this.con2)); }); }); + suite('Dragging Checks', function() { + suite('Stacks', function() { + setup(function() { + this.workspace = Blockly.inject('blocklyDiv'); + // Load in three blocks: A and B are connected (next/prev); B is unmovable. + Blockly.Xml.domToWorkspace(Blockly.Xml.textToDom(` + + + + + + + + `), this.workspace); + [this.blockA, this.blockB, this.blockC] = this.workspace.getAllBlocks(true); + this.checker = this.workspace.connectionChecker; + }); + + test('Connect a stack', function() { + // block C is not connected to block A; both are movable. + chai.assert.isTrue( + this.checker.doDragChecks( + this.blockC.nextConnection, this.blockA.previousConnection, 9000), + 'Should connect two compatible stack blocks'); + }); + + test('Do not splice into unmovable stack', function() { + // Try to connect blockC above blockB. It shouldn't work because B is not movable + // and is already connected to A's nextConnection. + chai.assert.isFalse( + this.checker.doDragChecks( + this.blockC.previousConnection, this.blockA.nextConnection, 9000), + 'Should not splice in a block above an unmovable block'); + }); + + test('Connect to bottom of unmovable stack', function() { + // Try to connect blockC below blockB. + // This is allowed even though B is not movable because it is on B's nextConnection. + chai.assert.isTrue( + this.checker.doDragChecks( + this.blockC.previousConnection, this.blockB.nextConnection, 9000), + 'Should connect below an unmovable stack block'); + }); + + test('Connect to unconnected unmovable block', function() { + // Delete blockA. + this.blockA.dispose(); + + // Try to connect blockC above blockB. + // This is allowed because we're not splicing into a stack. + chai.assert.isTrue( + this.checker.doDragChecks( + this.blockC.nextConnection, this.blockB.previousConnection, 9000), + 'Should connect above an unconnected unmovable block' + ); + }); + }); + suite('Rows', function() { + setup(function() { + this.workspace = Blockly.inject('blocklyDiv'); + // Load 3 blocks: A and B are connected (input/output); B is unmovable. + Blockly.Xml.domToWorkspace(Blockly.Xml.textToDom(` + + + + + + + `), this.workspace); + [this.blockA, this.blockB, this.blockC] = this.workspace.getAllBlocks(true); + this.checker = this.workspace.connectionChecker; + }); + + test('Do not splice into unmovable block row', function() { + // Try to connect C's output to A's input. Should fail because + // A is already connected to B, which is unmovable. + const inputConnection = this.blockA.inputList[0].connection; + chai.assert.isFalse( + this.checker.doDragChecks(this.blockC.outputConnection, inputConnection, 9000), + 'Should not splice in a block before an unmovable block' + ); + }); + + test('Connect to end of unmovable block', function() { + // Make blockC unmovable + this.blockC.setMovable(false); + // Try to connect A's output to C's input. This is allowed. + const inputConnection = this.blockC.inputList[0].connection; + chai.assert.isTrue( + this.checker.doDragChecks(this.blockA.outputConnection, inputConnection, 9000), + 'Should connect to end of unmovable block' + ); + }); + + test('Connect to unconnected unmovable block', function() { + // Delete blockA + this.blockA.dispose(); + + // Try to connect C's input to B's output. Allowed because B is now unconnected. + const inputConnection = this.blockC.inputList[0].connection; + chai.assert.isTrue( + this.checker.doDragChecks(inputConnection, this.blockB.outputConnection, 9000), + 'Should connect to unconnected unmovable block' + ); + }); + }); + }); });