fix: Don't visit collapsed blocks (#9090)

* WIP on line by line navigation

Doesn't work, likely due to isValid check.

* Add all inputs to the list of siblings

* Fix formatting

* Add tests

* Remove dupe keys

* fix: Make blocks with display: none not focusable

* Undo changes to canBeFocused

* Don't traverse inputs that are invisible

Author: RoboErikG

core/keyboard_nav/block_navigation_policy.ts

@@ -25,6 +25,9 @@ export class BlockNavigationPolicy implements INavigationPolicy<BlockSvg> {
     if (icons.length) return icons[0];
 
     for (const input of current.inputList) {
+      if (!input.isVisible()) {
+        continue;
+      }
       for (const field of input.fieldRow) {
         return field;
       }
@@ -70,6 +73,9 @@ export class BlockNavigationPolicy implements INavigationPolicy<BlockSvg> {
     let siblings: (BlockSvg | Field)[] = [];
     if (parent instanceof BlockSvg) {
       for (let i = 0, input; (input = parent.inputList[i]); i++) {
+        if (!input.isVisible()) {
+          continue;
+        }
         siblings.push(...input.fieldRow);
         const child = input.connection?.targetBlock();
         if (child) {
@@ -112,6 +118,9 @@ export class BlockNavigationPolicy implements INavigationPolicy<BlockSvg> {
     let siblings: (BlockSvg | Field)[] = [];
     if (parent instanceof BlockSvg) {
       for (let i = 0, input; (input = parent.inputList[i]); i++) {
+        if (!input.isVisible()) {
+          continue;
+        }
         siblings.push(...input.fieldRow);
         const child = input.connection?.targetBlock();
         if (child) {

tests/mocha/navigation_test.js

@@ -369,15 +369,26 @@ suite('Navigation', function () {
       this.blocks.secondBlock = secondBlock;
       this.blocks.outputNextBlock = outputNextBlock;
 
-      const buttonBlock = this.workspace.newBlock('buttons');
-      const buttonInput1 = this.workspace.newBlock('field_input');
-      const buttonInput2 = this.workspace.newBlock('field_input');
+      const buttonBlock = this.workspace.newBlock('buttons', 'button_block');
+      const buttonInput1 = this.workspace.newBlock(
+        'field_input',
+        'button_input1',
+      );
+      const buttonInput2 = this.workspace.newBlock(
+        'field_input',
+        'button_input2',
+      );
+      const buttonNext = this.workspace.newBlock(
+        'input_statement',
+        'button_next',
+      );
       buttonBlock.inputList[0].connection.connect(
         buttonInput1.outputConnection,
       );
       buttonBlock.inputList[2].connection.connect(
         buttonInput2.outputConnection,
       );
+      buttonBlock.nextConnection.connect(buttonNext.previousConnection);
       // Make buttons by adding a click handler
       const clickHandler = function () {
         return;
@@ -388,6 +399,7 @@ suite('Navigation', function () {
       this.blocks.buttonBlock = buttonBlock;
       this.blocks.buttonInput1 = buttonInput1;
       this.blocks.buttonInput2 = buttonInput2;
+      this.blocks.buttonNext = buttonNext;
 
       this.workspace.cleanUp();
     });
@@ -505,17 +517,22 @@ suite('Navigation', function () {
       });
       test('fromBlockToFieldInNextInput', function () {
         const field = this.blocks.buttonBlock.getField('BUTTON2');
-        const prevNode = this.navigator.getNextSibling(
+        const nextNode = this.navigator.getNextSibling(
           this.blocks.buttonInput1,
         );
-        assert.equal(prevNode, field);
+        assert.equal(nextNode, field);
       });
       test('fromBlockToFieldSkippingInput', function () {
         const field = this.blocks.buttonBlock.getField('BUTTON3');
-        const prevNode = this.navigator.getNextSibling(
+        const nextNode = this.navigator.getNextSibling(
           this.blocks.buttonInput2,
         );
-        assert.equal(prevNode, field);
+        assert.equal(nextNode, field);
+      });
+      test('skipsChildrenOfCollapsedBlocks', function () {
+        this.blocks.buttonBlock.setCollapsed(true);
+        const nextNode = this.navigator.getNextSibling(this.blocks.buttonBlock);
+        assert.equal(nextNode.id, this.blocks.buttonNext.id);
       });
     });
 
@@ -645,6 +662,13 @@ suite('Navigation', function () {
         );
         assert.equal(prevNode, field);
       });
+      test('skipsChildrenOfCollapsedBlocks', function () {
+        this.blocks.buttonBlock.setCollapsed(true);
+        const prevNode = this.navigator.getPreviousSibling(
+          this.blocks.buttonNext,
+        );
+        assert.equal(prevNode.id, this.blocks.buttonBlock.id);
+      });
     });
 
     suite('In', function () {
@@ -725,6 +749,11 @@ suite('Navigation', function () {
         const inNode = this.navigator.getFirstChild(icons[0]);
         assert.isNull(inNode);
       });
+      test('skipsChildrenOfCollapsedBlocks', function () {
+        this.blocks.buttonBlock.setCollapsed(true);
+        const inNode = this.navigator.getFirstChild(this.blocks.buttonBlock);
+        assert.isNull(inNode);
+      });
     });
 
     suite('Out', function () {