feat!: Make everything ISelectable focusable (#9004)

* feat!: Make bubbles, comments, and icons focusable

* feat!: Make ISelectable and ICopyable focusable.

* feat: Consolidate selection calls.

Now everything is based on focus with selection only being used as a
proxy.

* feat: Invert responsibility for setSelected().

Now setSelected() is only for quasi-external use.

* feat: Push up shadow check to getters.

Needed new common-level helper.

* chore: Lint fixes.

* feat!: Allow IFocusableNode to disable focus.

* chore: post-merge lint fixes

* fix: Fix tests + text bubble focusing.

This fixed then regressed a circular dependency causing the node and
advanced compilation steps to fail. This investigation is ongoing.

* fix: Clean up & fix imports.

This ensures the node and advanced compilation test steps now pass.

* fix: Lint fixes + revert commented out logic.

* chore: Remove unnecessary cast.

Addresses reviewer comment.

* fix: Some issues and a bunch of clean-ups.

This addresses a bunch of review comments, and fixes selecting workspace
comments.

* chore: Lint fix.

* fix: Remove unnecessary shadow consideration.

* chore: Revert import.

* chore: Some doc updates & added a warn statement.

Author: BenHenning

blocks/procedures.ts

@@ -9,7 +9,6 @@
 import type {Block} from '../core/block.js';
 import type {BlockSvg} from '../core/block_svg.js';
 import type {BlockDefinition} from '../core/blocks.js';
-import * as common from '../core/common.js';
 import {defineBlocks} from '../core/common.js';
 import {config} from '../core/config.js';
 import type {Connection} from '../core/connection.js';
@@ -27,6 +26,7 @@ import {FieldCheckbox} from '../core/field_checkbox.js';
 import {FieldLabel} from '../core/field_label.js';
 import * as fieldRegistry from '../core/field_registry.js';
 import {FieldTextInput} from '../core/field_textinput.js';
+import {getFocusManager} from '../core/focus_manager.js';
 import '../core/icons/comment_icon.js';
 import {MutatorIcon as Mutator} from '../core/icons/mutator_icon.js';
 import '../core/icons/warning_icon.js';
@@ -1178,7 +1178,7 @@ const PROCEDURE_CALL_COMMON = {
         const def = Procedures.getDefinition(name, workspace);
         if (def) {
           (workspace as WorkspaceSvg).centerOnBlock(def.id);
-          common.setSelected(def as BlockSvg);
+          getFocusManager().focusNode(def as BlockSvg);
         }
       },
     });

core/block_svg.ts

@@ -264,20 +264,14 @@ export class BlockSvg
 
   /** Selects this block. Highlights the block visually. */
   select() {
-    if (this.isShadow()) {
-      this.getParent()?.select();
-      return;
-    }
     this.addSelect();
+    common.fireSelectedEvent(this);
   }
 
   /** Unselects this block. Unhighlights the block visually. */
   unselect() {
-    if (this.isShadow()) {
-      this.getParent()?.unselect();
-      return;
-    }
     this.removeSelect();
+    common.fireSelectedEvent(null);
   }
 
   /**
@@ -860,25 +854,6 @@ export class BlockSvg
       blockAnimations.disposeUiEffect(this);
     }
 
-    // Selecting a shadow block highlights an ancestor block, but that highlight
-    // should be removed if the shadow block will be deleted. So, before
-    // deleting blocks and severing the connections between them, check whether
-    // doing so would delete a selected block and make sure that any associated
-    // parent is updated.
-    const selection = common.getSelected();
-    if (selection instanceof Block) {
-      let selectionAncestor: Block | null = selection;
-      while (selectionAncestor !== null) {
-        if (selectionAncestor === this) {
-          // The block to be deleted contains the selected block, so remove any
-          // selection highlight associated with the selected block before
-          // deleting them.
-          selection.unselect();
-        }
-        selectionAncestor = selectionAncestor.getParent();
-      }
-    }
-
     super.dispose(!!healStack);
     dom.removeNode(this.svgGroup);
   }
@@ -891,8 +866,7 @@ export class BlockSvg
     this.disposing = true;
     super.disposeInternal();
 
-    if (common.getSelected() === this) {
-      this.unselect();
+    if (getFocusManager().getFocusedNode() === this) {
       this.workspace.cancelCurrentGesture();
     }
 
@@ -1837,14 +1811,17 @@ export class BlockSvg
 
   /** See IFocusableNode.onNodeFocus. */
   onNodeFocus(): void {
-    common.setSelected(this);
+    this.select();
   }
 
   /** See IFocusableNode.onNodeBlur. */
   onNodeBlur(): void {
-    if (common.getSelected() === this) {
-      common.setSelected(null);
-    }
+    this.unselect();
+  }
+
+  /** See IFocusableNode.canBeFocused. */
+  canBeFocused(): boolean {
+    return true;
   }
 
   /**

core/bubbles/bubble.ts

@@ -4,11 +4,13 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import {ISelectable} from '../blockly.js';
 import * as browserEvents from '../browser_events.js';
 import * as common from '../common.js';
 import {BubbleDragStrategy} from '../dragging/bubble_drag_strategy.js';
+import {getFocusManager} from '../focus_manager.js';
 import {IBubble} from '../interfaces/i_bubble.js';
+import type {IFocusableTree} from '../interfaces/i_focusable_tree.js';
+import {ISelectable} from '../interfaces/i_selectable.js';
 import {ContainerRegion} from '../metrics_manager.js';
 import {Scrollbar} from '../scrollbar.js';
 import {Coordinate} from '../utils/coordinate.js';
@@ -86,17 +88,24 @@ export abstract class Bubble implements IBubble, ISelectable {
 
   private dragStrategy = new BubbleDragStrategy(this, this.workspace);
 
+  private focusableElement: SVGElement | HTMLElement;
+
   /**
    * @param workspace The workspace this bubble belongs to.
    * @param anchor The anchor location of the thing this bubble is attached to.
    *     The tail of the bubble will point to this location.
    * @param ownerRect An optional rect we don't want the bubble to overlap with
    *     when automatically positioning.
+   * @param overriddenFocusableElement An optional replacement to the focusable
+   *     element that's represented by this bubble (as a focusable node). This
+   *     element will have its ID and tabindex overwritten. If not provided, the
+   *     focusable element of this node will default to the bubble's SVG root.
    */
   constructor(
     public readonly workspace: WorkspaceSvg,
     protected anchor: Coordinate,
     protected ownerRect?: Rect,
+    overriddenFocusableElement?: SVGElement | HTMLElement,
   ) {
     this.id = idGenerator.getNextUniqueId();
     this.svgRoot = dom.createSvgElement(
@@ -127,6 +136,10 @@ export abstract class Bubble implements IBubble, ISelectable {
     );
     this.contentContainer = dom.createSvgElement(Svg.G, {}, this.svgRoot);
 
+    this.focusableElement = overriddenFocusableElement ?? this.svgRoot;
+    this.focusableElement.setAttribute('id', this.id);
+    this.focusableElement.setAttribute('tabindex', '-1');
+
     browserEvents.conditionalBind(
       this.background,
       'pointerdown',
@@ -208,11 +221,13 @@ export abstract class Bubble implements IBubble, ISelectable {
     this.background.setAttribute('fill', colour);
   }
 
-  /** Brings the bubble to the front and passes the pointer event off to the gesture system. */
+  /**
+   * Passes the pointer event off to the gesture system and ensures the bubble
+   * is focused.
+   */
   private onMouseDown(e: PointerEvent) {
     this.workspace.getGesture(e)?.handleBubbleStart(e, this);
-    this.bringToFront();
-    common.setSelected(this);
+    getFocusManager().focusNode(this);
   }
 
   /** Positions the bubble relative to its anchor. Does not render its tail. */
@@ -647,9 +662,37 @@ export abstract class Bubble implements IBubble, ISelectable {
 
   select(): void {
     // Bubbles don't have any visual for being selected.
+    common.fireSelectedEvent(this);
   }
 
   unselect(): void {
     // Bubbles don't have any visual for being selected.
+    common.fireSelectedEvent(null);
+  }
+
+  /** See IFocusableNode.getFocusableElement. */
+  getFocusableElement(): HTMLElement | SVGElement {
+    return this.focusableElement;
+  }
+
+  /** See IFocusableNode.getFocusableTree. */
+  getFocusableTree(): IFocusableTree {
+    return this.workspace;
+  }
+
+  /** See IFocusableNode.onNodeFocus. */
+  onNodeFocus(): void {
+    this.select();
+    this.bringToFront();
+  }
+
+  /** See IFocusableNode.onNodeBlur. */
+  onNodeBlur(): void {
+    this.unselect();
+  }
+
+  /** See IFocusableNode.canBeFocused. */
+  canBeFocused(): boolean {
+    return true;
   }
 }

core/bubbles/textinput_bubble.ts

@@ -80,11 +80,10 @@ export class TextInputBubble extends Bubble {
     protected anchor: Coordinate,
     protected ownerRect?: Rect,
   ) {
-    super(workspace, anchor, ownerRect);
+    super(workspace, anchor, ownerRect, TextInputBubble.createTextArea());
     dom.addClass(this.svgRoot, 'blocklyTextInputBubble');
-    ({inputRoot: this.inputRoot, textArea: this.textArea} = this.createEditor(
-      this.contentContainer,
-    ));
+    this.textArea = this.getFocusableElement() as HTMLTextAreaElement;
+    this.inputRoot = this.createEditor(this.contentContainer, this.textArea);
     this.resizeGroup = this.createResizeHandle(this.svgRoot, workspace);
     this.setSize(this.DEFAULT_SIZE, true);
   }
@@ -131,11 +130,21 @@ export class TextInputBubble extends Bubble {
     this.locationChangeListeners.push(listener);
   }
 
-  /** Creates the editor UI for this bubble. */
-  private createEditor(container: SVGGElement): {
-    inputRoot: SVGForeignObjectElement;
-    textArea: HTMLTextAreaElement;
-  } {
+  /** Creates and returns the editable text area for this bubble's editor. */
+  private static createTextArea(): HTMLTextAreaElement {
+    const textArea = document.createElementNS(
+      dom.HTML_NS,
+      'textarea',
+    ) as HTMLTextAreaElement;
+    textArea.className = 'blocklyTextarea blocklyText';
+    return textArea;
+  }
+
+  /** Creates and returns the UI container element for this bubble's editor. */
+  private createEditor(
+    container: SVGGElement,
+    textArea: HTMLTextAreaElement,
+  ): SVGForeignObjectElement {
     const inputRoot = dom.createSvgElement(
       Svg.FOREIGNOBJECT,
       {
@@ -149,22 +158,13 @@ export class TextInputBubble extends Bubble {
     body.setAttribute('xmlns', dom.HTML_NS);
     body.className = 'blocklyMinimalBody';
 
-    const textArea = document.createElementNS(
-      dom.HTML_NS,
-      'textarea',
-    ) as HTMLTextAreaElement;
-    textArea.className = 'blocklyTextarea blocklyText';
     textArea.setAttribute('dir', this.workspace.RTL ? 'RTL' : 'LTR');
-
     body.appendChild(textArea);
     inputRoot.appendChild(body);
 
     this.bindTextAreaEvents(textArea);
-    setTimeout(() => {
-      textArea.focus();
-    }, 0);
 
-    return {inputRoot, textArea};
+    return inputRoot;
   }
 
   /** Binds events to the text area element. */
@@ -174,13 +174,6 @@ export class TextInputBubble extends Bubble {
       e.stopPropagation();
     });
 
-    browserEvents.conditionalBind(
-      textArea,
-      'focus',
-      this,
-      this.onStartEdit,
-      true,
-    );
     browserEvents.conditionalBind(textArea, 'change', this, this.onTextChange);
   }
 
@@ -314,17 +307,6 @@ export class TextInputBubble extends Bubble {
     this.onSizeChange();
   }
 
-  /**
-   * Handles starting an edit of the text area. Brings the bubble to the front.
-   */
-  private onStartEdit() {
-    if (this.bringToFront()) {
-      // Since the act of moving this node within the DOM causes a loss of
-      // focus, we need to reapply the focus.
-      this.textArea.focus();
-    }
-  }
-
   /** Handles a text change event for the text area. Calls event listeners. */
   private onTextChange() {
     this.text = this.textArea.value;

core/clipboard/workspace_comment_paster.ts

@@ -5,9 +5,9 @@
  */
 
 import {RenderedWorkspaceComment} from '../comments/rendered_workspace_comment.js';
-import * as common from '../common.js';
 import {EventType} from '../events/type.js';
 import * as eventUtils from '../events/utils.js';
+import {getFocusManager} from '../focus_manager.js';
 import {ICopyData} from '../interfaces/i_copyable.js';
 import {IPaster} from '../interfaces/i_paster.js';
 import * as commentSerialiation from '../serialization/workspace_comments.js';
@@ -49,7 +49,7 @@ export class WorkspaceCommentPaster
     if (eventUtils.isEnabled()) {
       eventUtils.fire(new (eventUtils.get(EventType.COMMENT_CREATE))(comment));
     }
-    common.setSelected(comment);
+    getFocusManager().focusNode(comment);
     return comment;
   }
 }