fix: Improve narration and navigation of C-shaped blocks.

Author: gonfunko

core/block_svg.ts

@@ -39,6 +39,7 @@ import {IconType} from './icons/icon_types.js';
 import {MutatorIcon} from './icons/mutator_icon.js';
 import {WarningIcon} from './icons/warning_icon.js';
 import type {Input} from './inputs/input.js';
+import {inputTypes} from './inputs/input_types.js';
 import type {IBoundedElement} from './interfaces/i_bounded_element.js';
 import {IContextMenu} from './interfaces/i_contextmenu.js';
 import type {ICopyable} from './interfaces/i_copyable.js';
@@ -267,6 +268,20 @@ export class BlockSvg
       blockTypeText = 'C-shaped block';
     }
 
+    let prefix = '';
+    const parentInput = (
+      this.previousConnection ?? this.outputConnection
+    )?.targetConnection?.getParentInput();
+    if (parentInput && parentInput.type === inputTypes.STATEMENT) {
+      prefix = `Begin ${parentInput.getFieldRowLabel()}, `;
+    } else if (
+      parentInput &&
+      parentInput.type === inputTypes.VALUE &&
+      this.getParent()?.statementInputCount
+    ) {
+      prefix = `${parentInput.getFieldRowLabel()} `;
+    }
+
     let additionalInfo = blockTypeText;
     if (inputSummary && !nestedStatementBlockCount) {
       additionalInfo = `${additionalInfo} with ${inputSummary}`;
@@ -279,7 +294,7 @@ export class BlockSvg
       }
     }
 
-    return blockSummary + ', ' + additionalInfo;
+    return prefix + blockSummary + ', ' + additionalInfo;
   }
 
   private computeAriaRole() {

core/inputs/input.ts

@@ -303,6 +303,20 @@ export class Input {
     }
   }
 
+  /**
+   * Returns a label for this input's row on its parent block.
+   *
+   * Generally this consists of the labels/values of the preceding fields, and
+   * is intended for accessibility descriptions.
+   * @internal
+   * @returns A description of this input's row on its parent block.
+   */
+  getFieldRowLabel() {
+    return this.fieldRow.reduce((label, field) => {
+      return `${label} ${field.EDITABLE ? field.getAriaName() : field.getValue()}`;
+    }, '');
+  }
+
   /**
    * Constructs a connection based on the type of this input's source block.
    * Properly handles constructing headless connections for headless blocks

core/keyboard_nav/block_navigation_policy.ts

@@ -125,6 +125,13 @@ function getBlockNavigationCandidates(
   for (const input of block.inputList) {
     if (!input.isVisible()) continue;
     candidates.push(...input.fieldRow);
+    if (
+      input.connection?.type === ConnectionType.NEXT_STATEMENT &&
+      !input.connection.targetBlock()
+    ) {
+      candidates.push(input.connection as RenderedConnection);
+    }
+
     if (input.connection?.targetBlock()) {
       const connectedBlock = input.connection.targetBlock() as BlockSvg;
       if (input.connection.type === ConnectionType.NEXT_STATEMENT && !forward) {
@@ -142,6 +149,15 @@ function getBlockNavigationCandidates(
     }
   }
 
+  if (
+    block.nextConnection &&
+    !block.nextConnection.targetBlock() &&
+    (block.lastConnectionInStack(true) !== block.nextConnection ||
+      !!block.getSurroundParent())
+  ) {
+    candidates.push(block.nextConnection);
+  }
+
   return candidates;
 }
 

core/keyboard_nav/line_cursor.ts

@@ -15,12 +15,24 @@
 
 import {BlockSvg} from '../block_svg.js';
 import {RenderedWorkspaceComment} from '../comments/rendered_workspace_comment.js';
+import {ConnectionType} from '../connection_type.js';
 import {getFocusManager} from '../focus_manager.js';
 import type {IFocusableNode} from '../interfaces/i_focusable_node.js';
 import * as registry from '../registry.js';
+import {RenderedConnection} from '../rendered_connection.js';
 import type {WorkspaceSvg} from '../workspace_svg.js';
 import {Marker} from './marker.js';
 
+/**
+ * Representation of the direction of travel within a navigation context.
+ */
+export enum NavigationDirection {
+  NEXT,
+  PREVIOUS,
+  IN,
+  OUT,
+}
+
 /**
  * Class for a line cursor.
  */
@@ -51,13 +63,7 @@ export class LineCursor extends Marker {
     }
     const newNode = this.getNextNode(
       curNode,
-      (candidate: IFocusableNode | null) => {
-        return (
-          (candidate instanceof BlockSvg &&
-            !candidate.outputConnection?.targetBlock()) ||
-          candidate instanceof RenderedWorkspaceComment
-        );
-      },
+      this.getValidationFunction(NavigationDirection.NEXT),
       true,
     );
 
@@ -80,7 +86,11 @@ export class LineCursor extends Marker {
       return null;
     }
 
-    const newNode = this.getNextNode(curNode, () => true, true);
+    const newNode = this.getNextNode(
+      curNode,
+      this.getValidationFunction(NavigationDirection.IN),
+      true,
+    );
 
     if (newNode) {
       this.setCurNode(newNode);
@@ -101,13 +111,7 @@ export class LineCursor extends Marker {
     }
     const newNode = this.getPreviousNode(
       curNode,
-      (candidate: IFocusableNode | null) => {
-        return (
-          (candidate instanceof BlockSvg &&
-            !candidate.outputConnection?.targetBlock()) ||
-          candidate instanceof RenderedWorkspaceComment
-        );
-      },
+      this.getValidationFunction(NavigationDirection.PREVIOUS),
       true,
     );
 
@@ -130,7 +134,11 @@ export class LineCursor extends Marker {
       return null;
     }
 
-    const newNode = this.getPreviousNode(curNode, () => true, true);
+    const newNode = this.getPreviousNode(
+      curNode,
+      this.getValidationFunction(NavigationDirection.OUT),
+      true,
+    );
 
     if (newNode) {
       this.setCurNode(newNode);
@@ -147,15 +155,14 @@ export class LineCursor extends Marker {
   atEndOfLine(): boolean {
     const curNode = this.getCurNode();
     if (!curNode) return false;
-    const inNode = this.getNextNode(curNode, () => true, true);
+    const inNode = this.getNextNode(
+      curNode,
+      this.getValidationFunction(NavigationDirection.IN),
+      true,
+    );
     const nextNode = this.getNextNode(
       curNode,
-      (candidate: IFocusableNode | null) => {
-        return (
-          candidate instanceof BlockSvg &&
-          !candidate.outputConnection?.targetBlock()
-        );
-      },
+      this.getValidationFunction(NavigationDirection.NEXT),
       true,
     );
 
@@ -298,6 +305,82 @@ export class LineCursor extends Marker {
     return this.getRightMostChild(newNode, stopIfFound);
   }
 
+  /**
+   * Returns a function that will be used to determine whether a candidate for
+   * navigation is valid.
+   *
+   * @param direction The direction in which the user is navigating.
+   * @returns A function that takes a proposed navigation candidate and returns
+   *     true if navigation should be allowed to proceed to it, or false to find
+   *     a different candidate.
+   */
+  getValidationFunction(
+    direction: NavigationDirection,
+  ): (node: IFocusableNode | null) => boolean {
+    switch (direction) {
+      case NavigationDirection.IN:
+      case NavigationDirection.OUT:
+        return () => true;
+      case NavigationDirection.NEXT:
+      case NavigationDirection.PREVIOUS:
+        return (candidate: IFocusableNode | null) => {
+          if (
+            (candidate instanceof BlockSvg &&
+              !candidate.outputConnection?.targetBlock()) ||
+            candidate instanceof RenderedWorkspaceComment ||
+            (candidate instanceof RenderedConnection &&
+              (candidate.type === ConnectionType.NEXT_STATEMENT ||
+                (candidate.type === ConnectionType.INPUT_VALUE &&
+                  candidate.getSourceBlock().statementInputCount &&
+                  candidate.getSourceBlock().inputList[0] !==
+                    candidate.getParentInput())))
+          ) {
+            return true;
+          }
+
+          const current = this.getSourceBlockFromNode(this.getCurNode());
+          if (candidate instanceof BlockSvg && current instanceof BlockSvg) {
+            // If the candidate's parent uses inline inputs, disallow the
+            // candidate; it follows that it must be on the same row as its
+            // parent.
+            if (candidate.outputConnection?.targetBlock()?.getInputsInline()) {
+              return false;
+            }
+
+            const candidateParents = new Set<BlockSvg>();
+            let parent = candidate.getParent();
+            while (parent) {
+              // If the candidate block is an (in)direct child of the current
+              // block, disallow it; it cannot be on a different row than the
+              // current block.
+              if (parent === current && current === this.getCurNode()) {
+                return false;
+              }
+              candidateParents.add(parent);
+              parent = parent.getParent();
+            }
+
+            const currentParents = new Set<BlockSvg>();
+            parent = current.getParent();
+            while (parent) {
+              currentParents.add(parent);
+              parent = parent.getParent();
+            }
+
+            const sharedParents = currentParents.intersection(candidateParents);
+            // Allow the candidate if it and the current block have no parents
+            // in common, or if they have a shared parent with external inputs.
+            const result =
+              !sharedParents.size ||
+              sharedParents.values().some((block) => !block.getInputsInline());
+            return result;
+          }
+
+          return false;
+        };
+    }
+  }
+
   /**
    * Prepare for the deletion of a block by making a list of nodes we
    * could move the cursor to afterwards and save it to

core/rendered_connection.ts

@@ -20,6 +20,7 @@ import {ConnectionType} from './connection_type.js';
 import * as ContextMenu from './contextmenu.js';
 import {ContextMenuRegistry} from './contextmenu_registry.js';
 import * as eventUtils from './events/utils.js';
+import {inputTypes} from './inputs/input_types.js';
 import {IContextMenu} from './interfaces/i_contextmenu.js';
 import type {IFocusableNode} from './interfaces/i_focusable_node.js';
 import type {IFocusableTree} from './interfaces/i_focusable_tree.js';
@@ -334,7 +335,30 @@ export class RenderedConnection
     if (highlightSvg) {
       highlightSvg.style.display = '';
       aria.setRole(highlightSvg, aria.Role.FIGURE);
-      aria.setState(highlightSvg, aria.State.LABEL, 'Open connection');
+      aria.setState(highlightSvg, aria.State.ROLEDESCRIPTION, 'Connection');
+      if (this.type === ConnectionType.NEXT_STATEMENT) {
+        const parentInput =
+          this.getParentInput() ??
+          this.getSourceBlock()
+            .getTopStackBlock()
+            .previousConnection?.targetConnection?.getParentInput();
+        if (parentInput && parentInput.type === inputTypes.STATEMENT) {
+          aria.setState(
+            highlightSvg,
+            aria.State.LABEL,
+            `${this.getParentInput() ? 'Begin' : 'End'} ${parentInput.getFieldRowLabel()}`,
+          );
+        }
+      } else if (
+        this.type === ConnectionType.INPUT_VALUE &&
+        this.getSourceBlock().statementInputCount
+      ) {
+        aria.setState(
+          highlightSvg,
+          aria.State.LABEL,
+          `${this.getParentInput()?.getFieldRowLabel()}`,
+        );
+      }
     }
   }
 

tests/mocha/cursor_test.js

@@ -136,22 +136,22 @@ suite('Cursor', function () {
       assert.equal(curNode, fieldBlock);
     });
 
-    test('Prev - From previous connection does skip over next connection', function () {
+    test('Prev - From previous connection does not skip over next connection', function () {
       const prevConnection = this.blocks.B.previousConnection;
       const prevConnectionNode = prevConnection;
       this.cursor.setCurNode(prevConnectionNode);
       this.cursor.prev();
       const curNode = this.cursor.getCurNode();
-      assert.equal(curNode, this.blocks.A);
+      assert.equal(curNode, this.blocks.A.nextConnection);
     });
 
-    test('Prev - From first block loop to last block', function () {
+    test('Prev - From first block loop to last statement input', function () {
       const prevConnection = this.blocks.A;
       const prevConnectionNode = prevConnection;
       this.cursor.setCurNode(prevConnectionNode);
       this.cursor.prev();
       const curNode = this.cursor.getCurNode();
-      assert.equal(curNode, this.blocks.D);
+      assert.equal(curNode, this.blocks.D.getInput('NAME4').connection);
     });
 
     test('Out - From field does not skip over block node', function () {
@@ -253,26 +253,34 @@ suite('Cursor', function () {
     test('In - from field in nested statement block to next nested statement block', function () {
       this.cursor.setCurNode(this.secondStatement.getField('NAME'));
       this.cursor.in();
+      // Skip over the next connection
+      this.cursor.in();
       const curNode = this.cursor.getCurNode();
       assert.equal(curNode, this.thirdStatement);
     });
     test('In - from field in nested statement block to next stack', function () {
       this.cursor.setCurNode(this.thirdStatement.getField('NAME'));
       this.cursor.in();
+      // Skip over the next connection
+      this.cursor.in();
       const curNode = this.cursor.getCurNode();
       assert.equal(curNode, this.multiStatement2);
     });
 
     test('Out - from nested statement block to last field of previous nested statement block', function () {
       this.cursor.setCurNode(this.thirdStatement);
       this.cursor.out();
+      // Skip over the previous next connection
+      this.cursor.out();
       const curNode = this.cursor.getCurNode();
       assert.equal(curNode, this.secondStatement.getField('NAME'));
     });
 
     test('Out - from root block to last field of last nested statement block in previous stack', function () {
       this.cursor.setCurNode(this.multiStatement2);
       this.cursor.out();
+      // Skip over the previous next connection
+      this.cursor.out();
       const curNode = this.cursor.getCurNode();
       assert.equal(curNode, this.thirdStatement.getField('NAME'));
     });
@@ -395,7 +403,7 @@ suite('Cursor', function () {
       });
       test('getLastNode', function () {
         const node = this.cursor.getLastNode();
-        assert.equal(node, this.blockA);
+        assert.equal(node, this.blockA.inputList[0].connection);
       });
     });