fix: Fix bug that caused blocks to be inserted into an immovable stack. (#750)

* fix: Fix bug that caused blocks to be inserted into an immovable stack.

* fix: Fix bug that caused input blocks to bump immovable blocks on insert.

Author: gonfunko

src/navigation.ts

@@ -414,24 +414,59 @@ export class Navigation {
       const inputType = movingHasOutput
         ? Blockly.inputs.inputTypes.VALUE
         : Blockly.inputs.inputTypes.STATEMENT;
-      const compatibleInputs = stationaryNode.inputList.filter(
-        (input) => input.type === inputType,
-      );
-      const input = compatibleInputs.length > 0 ? compatibleInputs[0] : null;
-      let connection = input?.connection;
-      if (connection) {
+      const compatibleConnections = stationaryNode.inputList
+        .filter((input) => input.type === inputType)
+        .map((input) => input.connection);
+      for (const connection of compatibleConnections) {
+        let targetConnection: Blockly.Connection | null | undefined =
+          connection;
         if (inputType === Blockly.inputs.inputTypes.STATEMENT) {
-          while (connection.targetBlock()?.nextConnection) {
-            // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-            connection = connection.targetBlock()!.nextConnection!;
+          while (targetConnection?.targetBlock()?.nextConnection) {
+            targetConnection = targetConnection?.targetBlock()?.nextConnection;
           }
         }
-        return connection as Blockly.RenderedConnection;
+
+        if (
+          targetConnection &&
+          movingBlock.workspace.connectionChecker.canConnect(
+            movingHasOutput
+              ? movingBlock.outputConnection
+              : movingBlock.previousConnection,
+            targetConnection,
+            true,
+            // Since we're connecting programmatically, we don't care how
+            // close the blocks are when determining if they can be connected.
+            Infinity,
+          )
+        ) {
+          return targetConnection as Blockly.RenderedConnection;
+        }
       }
 
-      // 2. Connect statement blocks to next connection.
+      // 2. Connect statement blocks to next connection. Only return a next
+      // connection to which the statement block can actually connect; some
+      // may be ineligible because they are e.g. in the middle of an immovable
+      // stack.
       if (stationaryNode.nextConnection && !movingHasOutput) {
-        return stationaryNode.nextConnection;
+        let nextConnection: Blockly.RenderedConnection | null =
+          stationaryNode.nextConnection;
+        while (nextConnection) {
+          if (
+            movingBlock.workspace.connectionChecker.canConnect(
+              movingBlock.previousConnection,
+              nextConnection,
+              true,
+              // Since we're connecting programmatically, we don't care how
+              // close the blocks are when determining if they can be connected.
+              Infinity,
+            )
+          ) {
+            return nextConnection;
+          }
+          nextConnection =
+            nextConnection.getSourceBlock().getNextBlock()?.nextConnection ??
+            null;
+        }
       }
 
       // 3. Output connection. This will wrap around or displace.

test/loadTestBlocks.js

@@ -343,7 +343,7 @@ const moreBlocks = {
                         'DO0': {
                           'block': {
                             'type': 'text_print',
-                            'id': 'uSxT~QT8p%D2o)b~)Dki',
+                            'id': 'text_print_2',
                             'inputs': {
                               'TEXT': {
                                 'shadow': {
@@ -388,7 +388,7 @@ const moreBlocks = {
                       'next': {
                         'block': {
                           'type': 'text_print',
-                          'id': '-bTQ2YVSuBS/SYn[C^LX',
+                          'id': 'text_print_3',
                           'inputs': {
                             'TEXT': {
                               'shadow': {

test/webdriverio/test/insert_test.ts

@@ -5,6 +5,7 @@
  */
 
 import * as chai from 'chai';
+import * as Blockly from 'blockly';
 import {Key} from 'webdriverio';
 import {
   getFocusedBlockType,
@@ -16,6 +17,7 @@ import {
   testSetup,
   sendKeyAndWait,
   keyRight,
+  keyDown,
   getCurrentFocusedBlockId,
   blockIsPresent,
   keyUp,
@@ -103,4 +105,81 @@ suite('Insert test', function () {
       await getFocusedBlockType(this.browser),
     );
   });
+
+  test('Does not insert between immovable blocks', async function () {
+    // Focus the create canvas block; we want to ensure that the newly
+    // inserted block is not attached to its next connection, because doing
+    // so would splice it into an immovable stack.
+    await focusOnBlock(this.browser, 'create_canvas_1');
+    await this.browser.execute(() => {
+      Blockly.getMainWorkspace()
+        .getAllBlocks()
+        .forEach((b) => b.setMovable(false));
+    });
+    await tabNavigateToToolbox(this.browser);
+
+    // Insert 'if' block
+    await keyRight(this.browser);
+    // Choose.
+    await sendKeyAndWait(this.browser, Key.Enter);
+    // Confirm position.
+    await sendKeyAndWait(this.browser, Key.Enter);
+
+    // Assert inserted inside first block p5_setup not at top-level.
+    chai.assert.equal('controls_if', await getFocusedBlockType(this.browser));
+    await keyUp(this.browser);
+    chai.assert.equal(
+      'p5_background_color',
+      await getFocusedBlockType(this.browser),
+    );
+  });
+});
+
+suite('Insert test with more blocks', function () {
+  // Disable timeouts when non-zero PAUSE_TIME is used to watch tests run.
+  if (PAUSE_TIME) this.timeout(0);
+
+  // Clear the workspace and load start blocks.
+  setup(async function () {
+    this.browser = await testSetup(
+      testFileLocations.MORE_BLOCKS,
+      this.timeout(),
+    );
+    await this.browser.pause(PAUSE_TIME);
+  });
+
+  test('Does not bump immovable input blocks on insert', async function () {
+    // Focus the print block with a connected input block. Ordinarily, inserting
+    // an input block would connect it to this block and bump its child, but
+    // if all blocks are immovable the connected input block should not move
+    // and the newly inserted block should be added as a top-level block on the
+    // workspace.
+    await focusOnBlock(this.browser, 'text_print_2');
+    await this.browser.execute(() => {
+      Blockly.getMainWorkspace()
+        .getAllBlocks()
+        .forEach((b) => b.setMovable(false));
+    });
+    await tabNavigateToToolbox(this.browser);
+
+    // Insert number block
+    await keyDown(this.browser, 2);
+    await keyRight(this.browser);
+    // Choose.
+    await sendKeyAndWait(this.browser, Key.Enter);
+    // Confirm position.
+    await sendKeyAndWait(this.browser, Key.Enter);
+
+    // Assert inserted at the top-level due to immovable block occupying the
+    // selected block's input.
+    chai.assert.equal('math_number', await getFocusedBlockType(this.browser));
+    const focusedBlockIsParentless = await this.browser.execute(() => {
+      const focusedNode = Blockly.getFocusManager().getFocusedNode();
+      return (
+        focusedNode instanceof Blockly.BlockSvg &&
+        focusedNode.getParent() === null
+      );
+    });
+    chai.assert.isTrue(focusedBlockIsParentless);
+  });
 });