fix!: Improve keyboard navigation of icons and bubbles (#9737)

* fix!: Improve keyboard navigation of icons and bubbles

* chore: Fix docstrings

* chore: Remove debugging

packages/blockly/core/blockly.ts

@@ -438,8 +438,8 @@ Names.prototype.populateProcedures = function (
 // clang-format on
 
 export * from './interfaces/i_navigation_policy.js';
-export * from './keyboard_nav/navigation_policies/block_comment_navigation_policy.js';
 export * from './keyboard_nav/navigation_policies/block_navigation_policy.js';
+export * from './keyboard_nav/navigation_policies/bubble_navigation_policy.js';
 export * from './keyboard_nav/navigation_policies/comment_bar_button_navigation_policy.js';
 export * from './keyboard_nav/navigation_policies/comment_editor_navigation_policy.js';
 export * from './keyboard_nav/navigation_policies/connection_navigation_policy.js';

packages/blockly/core/bubbles/mini_workspace_bubble.ts

@@ -7,6 +7,8 @@
 import type {BlocklyOptions} from '../blockly_options.js';
 import {Abstract as AbstractEvent} from '../events/events_abstract.js';
 import {getFocusManager} from '../focus_manager.js';
+import type {IFocusableNode} from '../interfaces/i_focusable_node.js';
+import type {IHasBubble} from '../interfaces/i_has_bubble.js';
 import {KeyboardMover} from '../keyboard_nav/keyboard_mover.js';
 import {Options} from '../options.js';
 import {Coordinate} from '../utils/coordinate.js';
@@ -52,8 +54,9 @@ export class MiniWorkspaceBubble extends Bubble {
     public readonly workspace: WorkspaceSvg,
     protected anchor: Coordinate,
     protected ownerRect?: Rect,
+    protected owner?: IHasBubble & IFocusableNode,
   ) {
-    super(workspace, anchor, ownerRect);
+    super(workspace, anchor, ownerRect, undefined, owner);
     const options = new Options(workspaceOptions);
     this.validateWorkspaceOptions(options);
 

packages/blockly/core/bubbles/text_bubble.ts

@@ -4,6 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import type {IFocusableNode} from '../interfaces/i_focusable_node.js';
+import type {IHasBubble} from '../interfaces/i_has_bubble.js';
 import {Coordinate} from '../utils/coordinate.js';
 import * as dom from '../utils/dom.js';
 import {Rect} from '../utils/rect.js';
@@ -23,8 +25,9 @@ export class TextBubble extends Bubble {
     public readonly workspace: WorkspaceSvg,
     protected anchor: Coordinate,
     protected ownerRect?: Rect,
+    protected owner?: IHasBubble & IFocusableNode,
   ) {
-    super(workspace, anchor, ownerRect);
+    super(workspace, anchor, ownerRect, undefined, owner);
     this.paragraph = this.stringToSvg(text, this.contentContainer);
     this.updateBubbleSize();
     dom.addClass(this.svgRoot, 'blocklyTextBubble');

packages/blockly/core/css.ts

@@ -644,7 +644,7 @@ input[type=number] {
 /* The workspace itself is the active node. */
 .blocklyKeyboardNavigation
   .blocklyBubble.blocklyActiveFocus
-  .blocklyDraggable {
+  .blocklyEmboss .blocklyDraggable {
   stroke: var(--blockly-active-node-color);
   stroke-width: var(--blockly-selection-width);
 }

packages/blockly/core/icons/mutator_icon.ts

@@ -176,6 +176,7 @@ export class MutatorIcon extends Icon implements IHasBubble {
         this.sourceBlock.workspace,
         this.getAnchorLocation(),
         this.getBubbleOwnerRect(),
+        this,
       );
       this.applyColour();
       this.createRootBlock();

packages/blockly/core/icons/warning_icon.ts

@@ -182,6 +182,7 @@ export class WarningIcon extends Icon implements IHasBubble {
         this.sourceBlock.workspace,
         this.getAnchorLocation(),
         this.getBubbleOwnerRect(),
+        this,
       );
       this.applyColour();
     } else {

packages/blockly/core/keyboard_nav/navigation_policies/block_comment_navigation_policy.ts

@@ -1,86 +0,0 @@
-/**
- * @license
- * Copyright 2025 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-
-import {TextInputBubble} from '../../bubbles/textinput_bubble.js';
-import type {IFocusableNode} from '../../interfaces/i_focusable_node.js';
-import type {INavigationPolicy} from '../../interfaces/i_navigation_policy.js';
-
-/**
- * Set of rules controlling keyboard navigation from an TextInputBubble.
- */
-export class BlockCommentNavigationPolicy
-  implements INavigationPolicy<TextInputBubble>
-{
-  /**
-   * Returns the first child of the given block comment.
-   *
-   * @param current The block comment to return the first child of.
-   * @returns The text editor of the given block comment bubble.
-   */
-  getFirstChild(current: TextInputBubble): IFocusableNode | null {
-    return current.getEditor();
-  }
-
-  /**
-   * Returns the parent of the given block comment.
-   *
-   * @param current The block comment to return the parent of.
-   * @returns The parent block of the given block comment.
-   */
-  getParent(current: TextInputBubble): IFocusableNode | null {
-    return current.getOwner() ?? null;
-  }
-
-  /**
-   * Returns the next peer node of the given block comment.
-   *
-   * @param _current The block comment to find the following element of.
-   * @returns Null.
-   */
-  getNextSibling(_current: TextInputBubble): IFocusableNode | null {
-    return null;
-  }
-
-  /**
-   * Returns the previous peer node of the given block comment.
-   *
-   * @param _current The block comment to find the preceding element of.
-   * @returns Null.
-   */
-  getPreviousSibling(_current: TextInputBubble): IFocusableNode | null {
-    return null;
-  }
-
-  /**
-   * Returns the row ID of the given block comment.
-   *
-   * @param current The block comment to retrieve the row ID of.
-   * @returns The row ID of the given block comment.
-   */
-  getRowId(current: TextInputBubble) {
-    return current.id;
-  }
-
-  /**
-   * Returns whether or not the given block comment can be navigated to.
-   *
-   * @param current The instance to check for navigability.
-   * @returns True if the given block comment can be focused.
-   */
-  isNavigable(current: TextInputBubble): boolean {
-    return current.canBeFocused();
-  }
-
-  /**
-   * Returns whether the given object can be navigated from by this policy.
-   *
-   * @param current The object to check if this policy applies to.
-   * @returns True if the object is an TextInputBubble.
-   */
-  isApplicable(current: any): current is TextInputBubble {
-    return current instanceof TextInputBubble;
-  }
-}

packages/blockly/core/keyboard_nav/navigation_policies/block_navigation_policy.ts

@@ -6,6 +6,7 @@
 
 import {BlockSvg} from '../../block_svg.js';
 import type {IFocusableNode} from '../../interfaces/i_focusable_node.js';
+import {hasBubble} from '../../interfaces/i_has_bubble.js';
 import type {INavigationPolicy} from '../../interfaces/i_navigation_policy.js';
 import {RenderedConnection} from '../../rendered_connection.js';
 
@@ -121,8 +122,20 @@ function getBlockNavigationCandidates(block: BlockSvg): IFocusableNode[] {
   // Collapsed blocks have no navigable children.
   if (block.isCollapsed()) return [];
 
-  // Icons are navigable.
-  const candidates: IFocusableNode[] = block.getIcons();
+  const candidates: IFocusableNode[] = [];
+
+  // Icons and open bubbles are navigable.
+  for (const icon of block.getIcons()) {
+    candidates.push(icon);
+    let bubble;
+    if (
+      hasBubble(icon) &&
+      icon.bubbleIsVisible() &&
+      (bubble = icon.getBubble())
+    ) {
+      candidates.push(bubble);
+    }
+  }
 
   for (const input of block.inputList) {
     // Invisible inputs are not valid navigation candidates.

packages/blockly/core/keyboard_nav/navigation_policies/bubble_navigation_policy.ts

@@ -0,0 +1,101 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import type {BlockSvg} from '../../block_svg.js';
+import {Bubble} from '../../bubbles/bubble.js';
+import type {Icon} from '../../icons/icon.js';
+import type {IFocusableNode} from '../../interfaces/i_focusable_node.js';
+import type {INavigationPolicy} from '../../interfaces/i_navigation_policy.js';
+import {navigateBlock} from './block_navigation_policy.js';
+
+/**
+ * Set of rules controlling keyboard navigation from a Bubble.
+ */
+export class BubbleNavigationPolicy implements INavigationPolicy<Bubble> {
+  /**
+   * Returns the first child of the given bubble.
+   *
+   * @param _current The bubble to return the first child of.
+   * @returns Null.
+   */
+  getFirstChild(_current: Bubble): IFocusableNode | null {
+    return null;
+  }
+
+  /**
+   * Returns the parent of the given bubble.
+   *
+   * @param current The bubble to return the parent of.
+   * @returns The parent block of the given bubble.
+   */
+  getParent(current: Bubble): IFocusableNode | null {
+    return current.getOwner() ?? null;
+  }
+
+  /**
+   * Returns the next peer node of the given bubble.
+   *
+   * @param current The bubble to find the following element of.
+   * @returns The next navigable item on the bubble's icon's parent block.
+   */
+  getNextSibling(current: Bubble): IFocusableNode | null {
+    return navigateBlock(
+      (current.getOwner() as Icon | undefined)?.getSourceBlock() as BlockSvg,
+      current,
+      1,
+    );
+  }
+
+  /**
+   * Returns the previous peer node of the given bubble.
+   *
+   * @param current The bubble to find the preceding element of.
+   * @returns The previous navigable item on the bubble's icon's parent block.
+   */
+  getPreviousSibling(current: Bubble): IFocusableNode | null {
+    return navigateBlock(
+      (current.getOwner() as Icon | undefined)?.getSourceBlock() as BlockSvg,
+      current,
+      -1,
+    );
+  }
+
+  /**
+   * Returns the row ID of the given bubble.
+   *
+   * @param current The bubble to retrieve the row ID of.
+   * @returns The row ID of the given bubble.
+   */
+  getRowId(current: Bubble) {
+    return (
+      (
+        (current.getOwner() as Icon | undefined)?.getSourceBlock() as
+          | BlockSvg
+          | undefined
+      )?.getRowId() ?? ''
+    );
+  }
+
+  /**
+   * Returns whether or not the given bubble can be navigated to.
+   *
+   * @param current The instance to check for navigability.
+   * @returns True if the given bubble can be focused.
+   */
+  isNavigable(current: Bubble): boolean {
+    return current.canBeFocused();
+  }
+
+  /**
+   * Returns whether the given object can be navigated from by this policy.
+   *
+   * @param current The object to check if this policy applies to.
+   * @returns True if the object is an Bubble.
+   */
+  isApplicable(current: any): current is Bubble {
+    return current instanceof Bubble;
+  }
+}

packages/blockly/core/keyboard_nav/navigation_policies/icon_navigation_policy.ts

@@ -5,10 +5,7 @@
  */
 
 import {BlockSvg} from '../../block_svg.js';
-import {getFocusManager} from '../../focus_manager.js';
-import {CommentIcon} from '../../icons/comment_icon.js';
 import {Icon} from '../../icons/icon.js';
-import {MutatorIcon} from '../../icons/mutator_icon.js';
 import type {IFocusableNode} from '../../interfaces/i_focusable_node.js';
 import type {INavigationPolicy} from '../../interfaces/i_navigation_policy.js';
 import {navigateBlock} from './block_navigation_policy.js';
@@ -20,24 +17,10 @@ export class IconNavigationPolicy implements INavigationPolicy<Icon> {
   /**
    * Returns the first child of the given icon.
    *
-   * @param current The icon to return the first child of.
+   * @param _current The icon to return the first child of.
    * @returns Null.
    */
-  getFirstChild(current: Icon): IFocusableNode | null {
-    if (
-      current instanceof MutatorIcon &&
-      current.bubbleIsVisible() &&
-      getFocusManager().getFocusedNode() === current
-    ) {
-      return current.getBubble()?.getWorkspace() ?? null;
-    } else if (
-      current instanceof CommentIcon &&
-      current.bubbleIsVisible() &&
-      getFocusManager().getFocusedNode() === current
-    ) {
-      return current.getBubble()?.getEditor() ?? null;
-    }
-
+  getFirstChild(_current: Icon): IFocusableNode | null {
     return null;
   }
 

packages/blockly/core/keyboard_nav/navigators/navigator.ts

@@ -12,8 +12,8 @@ import {Icon} from '../../icons/icon.js';
 import type {IFocusableNode} from '../../interfaces/i_focusable_node.js';
 import type {INavigationPolicy} from '../../interfaces/i_navigation_policy.js';
 import {RenderedConnection} from '../../rendered_connection.js';
-import {BlockCommentNavigationPolicy} from '../navigation_policies/block_comment_navigation_policy.js';
 import {BlockNavigationPolicy} from '../navigation_policies/block_navigation_policy.js';
+import {BubbleNavigationPolicy} from '../navigation_policies/bubble_navigation_policy.js';
 import {CommentBarButtonNavigationPolicy} from '../navigation_policies/comment_bar_button_navigation_policy.js';
 import {CommentEditorNavigationPolicy} from '../navigation_policies/comment_editor_navigation_policy.js';
 import {ConnectionNavigationPolicy} from '../navigation_policies/connection_navigation_policy.js';
@@ -51,7 +51,7 @@ export class Navigator {
     new IconNavigationPolicy(),
     new WorkspaceCommentNavigationPolicy(),
     new CommentBarButtonNavigationPolicy(),
-    new BlockCommentNavigationPolicy(),
+    new BubbleNavigationPolicy(),
     new CommentEditorNavigationPolicy(),
   ];
 

packages/blockly/tests/mocha/keyboard_navigation_test.js

@@ -183,6 +183,55 @@ suite('Keyboard navigation on Blocks', function () {
     assert.equal(getFocusedBlockId(), 'p5_setup_1');
   });
 
+  test('Right from block selects first icon', function () {
+    this.workspace.getBlockById('p5_canvas_1').setCommentText('hello');
+    focusBlock(this.workspace, 'p5_canvas_1');
+    pressKey(this.workspace, Blockly.utils.KeyCodes.RIGHT);
+    assert.equal(
+      Blockly.getFocusManager().getFocusedNode(),
+      this.workspace
+        .getBlockById('p5_canvas_1')
+        .getIcon(Blockly.icons.IconType.COMMENT),
+    );
+  });
+
+  test('Right from icon selects next icon', function () {
+    const block = this.workspace.getBlockById('p5_canvas_1');
+    block.setCommentText('hello');
+    block.setWarningText('danger!');
+    const commentIcon = block.getIcon(Blockly.icons.IconType.COMMENT);
+    const warningIcon = block.getIcon(Blockly.icons.IconType.WARNING);
+
+    Blockly.getFocusManager().focusNode(warningIcon);
+    pressKey(this.workspace, Blockly.utils.KeyCodes.RIGHT);
+    assert.equal(Blockly.getFocusManager().getFocusedNode(), commentIcon);
+  });
+
+  test('Right from icon selects bubble', async function () {
+    const block = this.workspace.getBlockById('p5_canvas_1');
+    block.setCommentText('hello');
+    const commentIcon = block.getIcon(Blockly.icons.IconType.COMMENT);
+    await commentIcon.setBubbleVisible(true);
+
+    Blockly.getFocusManager().focusNode(commentIcon);
+    pressKey(this.workspace, Blockly.utils.KeyCodes.RIGHT);
+    assert.equal(
+      Blockly.getFocusManager().getFocusedNode(),
+      commentIcon.getBubble(),
+    );
+  });
+
+  test('Right from last icon selects field', function () {
+    this.workspace.getBlockById('p5_canvas_1').setCommentText('hello');
+    const icon = this.workspace
+      .getBlockById('p5_canvas_1')
+      .getIcon(Blockly.icons.IconType.COMMENT);
+    Blockly.getFocusManager().focusNode(icon);
+    pressKey(this.workspace, Blockly.utils.KeyCodes.RIGHT);
+    assert.include(getFocusNodeId(), 'p5_canvas_1_field_');
+    assert.equal(getFocusedFieldName(), 'WIDTH');
+  });
+
   test('Right from block selects first field', function () {
     focusBlock(this.workspace, 'p5_canvas_1');
     pressKey(this.workspace, Blockly.utils.KeyCodes.RIGHT);
@@ -223,6 +272,70 @@ suite('Keyboard navigation on Blocks', function () {
     assert.equal(getFocusedBlockId(), 'math_number_2');
   });
 
+  test('Left from icon selects block', function () {
+    const block = this.workspace.getBlockById('p5_canvas_1');
+    block.setCommentText('hello');
+    Blockly.getFocusManager().focusNode(
+      block.getIcon(Blockly.icons.IconType.COMMENT),
+    );
+    pressKey(this.workspace, Blockly.utils.KeyCodes.LEFT);
+    assert.equal(Blockly.getFocusManager().getFocusedNode(), block);
+  });
+
+  test('Left from icon selects previous icon', function () {
+    const block = this.workspace.getBlockById('p5_canvas_1');
+    block.setCommentText('hello');
+    block.setWarningText('danger!');
+    const commentIcon = block.getIcon(Blockly.icons.IconType.COMMENT);
+    const warningIcon = block.getIcon(Blockly.icons.IconType.WARNING);
+
+    Blockly.getFocusManager().focusNode(commentIcon);
+    pressKey(this.workspace, Blockly.utils.KeyCodes.LEFT);
+    assert.equal(Blockly.getFocusManager().getFocusedNode(), warningIcon);
+  });
+
+  test('Left from icon selects bubble', async function () {
+    const block = this.workspace.getBlockById('p5_canvas_1');
+    block.setCommentText('hello');
+    block.setWarningText('danger!');
+    const commentIcon = block.getIcon(Blockly.icons.IconType.COMMENT);
+    const warningIcon = block.getIcon(Blockly.icons.IconType.WARNING);
+    const bubbleVisible = warningIcon.setBubbleVisible(true);
+    this.clock.runAll();
+    await bubbleVisible;
+
+    Blockly.getFocusManager().focusNode(commentIcon);
+    pressKey(this.workspace, Blockly.utils.KeyCodes.LEFT);
+    assert.equal(
+      Blockly.getFocusManager().getFocusedNode(),
+      warningIcon.getBubble(),
+    );
+  });
+
+  test('Left from field selects icon', function () {
+    this.workspace.getBlockById('p5_canvas_1').setCommentText('hello');
+    const commentIcon = this.workspace
+      .getBlockById('p5_canvas_1')
+      .getIcon(Blockly.icons.IconType.COMMENT);
+    focusBlockField(this.workspace, 'p5_canvas_1', 'WIDTH');
+    pressKey(this.workspace, Blockly.utils.KeyCodes.LEFT);
+    assert.equal(Blockly.getFocusManager().getFocusedNode(), commentIcon);
+  });
+
+  test('Left from field selects bubble', async function () {
+    this.workspace.getBlockById('p5_canvas_1').setCommentText('hello');
+    const commentIcon = this.workspace
+      .getBlockById('p5_canvas_1')
+      .getIcon(Blockly.icons.IconType.COMMENT);
+    await commentIcon.setBubbleVisible(true);
+    focusBlockField(this.workspace, 'p5_canvas_1', 'WIDTH');
+    pressKey(this.workspace, Blockly.utils.KeyCodes.LEFT);
+    assert.equal(
+      Blockly.getFocusManager().getFocusedNode(),
+      commentIcon.getBubble(),
+    );
+  });
+
   test('Right from last inline input block selects next child field', function () {
     focusBlock(this.workspace, 'colour_picker_1');
     // Go right twice; should not wrap to next row.