fix: drop IFocusableNode from trash and zoom

packages/blockly/core/css.ts

@@ -535,9 +535,9 @@ input[type=number] {
   .blocklyBubble,
   .blocklyIconGroup,
   .blocklyTextarea,
-  .blocklyZoom,
-  .blocklyTrash,
-) {
+),
+.blocklyZoom:focus,
+.blocklyTrash:focus {
   outline: none;
 }
 .hiddenForAria {
@@ -568,7 +568,9 @@ input[type=number] {
   > .blocklyIconShape:first-child,
 .blocklyKeyboardNavigation
   .blocklyActiveFocus
-  > .blocklyFocusRing {
+  > .blocklyFocusRing,
+.blocklyTrash:focus-visible > .blocklyFocusRing,
+.blocklyZoom:focus-visible > .blocklyFocusRing {
   stroke: var(--blockly-active-node-color);
   stroke-width: var(--blockly-selection-width);
 }

packages/blockly/core/trashcan.ts

@@ -25,7 +25,6 @@ import type {IAutoHideable} from './interfaces/i_autohideable.js';
 import type {IComponent} from './interfaces/i_component';
 import type {IDraggable} from './interfaces/i_draggable.js';
 import type {IFlyout} from './interfaces/i_flyout.js';
-import type {IFocusableNode} from './interfaces/i_focusable_node.js';
 import type {IPositionable} from './interfaces/i_positionable.js';
 import {KeyboardMover} from './keyboard_nav/keyboard_mover.js';
 import {keyboardNavigationController} from './keyboard_navigation_controller.js';
@@ -50,7 +49,7 @@ import type {WorkspaceSvg} from './workspace_svg.js';
  */
 export class Trashcan
   extends DeleteArea
-  implements IAutoHideable, IPositionable, IFocusableNode, IComponent
+  implements IAutoHideable, IPositionable, IComponent
 {
   /**
    * The id for this component that is used to register with the
@@ -256,6 +255,7 @@ export class Trashcan
       this.blockMouseDownWhenOpenable,
     );
     browserEvents.bind(this.svgGroup, 'pointerup', this, this.click);
+    browserEvents.bind(this.svgGroup, 'keydown', this, this.onKeyDown);
     return this.svgGroup;
   }
 
@@ -275,7 +275,6 @@ export class Trashcan
         ComponentManager.Capability.DELETE_AREA,
         ComponentManager.Capability.DRAG_TARGET,
         ComponentManager.Capability.POSITIONABLE,
-        ComponentManager.Capability.FOCUSABLE,
       ],
     });
     this.initialized = true;
@@ -530,6 +529,18 @@ export class Trashcan
     this.openFlyout();
   }
 
+  /**
+   * Activates the trashcan when Enter or Space is pressed.
+   *
+   * @param e A keydown event.
+   */
+  private onKeyDown(e: KeyboardEvent) {
+    if (e.key === 'Enter' || e.key === ' ') {
+      e.preventDefault();
+      this.click();
+    }
+  }
+
   /**
    * Fires a UI event for trashcan flyout open or close.
    *
@@ -653,28 +664,6 @@ export class Trashcan
     };
     return blockInfo;
   }
-
-  getFocusableElement() {
-    if (!this.svgGroup) {
-      throw new Error('Tried to focus uninitialized trashcan');
-    }
-    return this.svgGroup;
-  }
-
-  getFocusableTree() {
-    return this.workspace;
-  }
-
-  onNodeFocus() {}
-  onNodeBlur() {}
-
-  canBeFocused() {
-    return !!this.svgGroup;
-  }
-
-  performAction() {
-    this.click();
-  }
 }
 
 /** Width of both the trash can and lid images. */
@@ -705,13 +694,14 @@ const TRASH_FULL = 'blocklyTrashFull';
 const TRASH_OPEN = 'blocklyTrashOpen';
 
 Css.register(`
-  .blocklyTrash {
+  .blocklyTrash > image {
     opacity: 0.4;
-    transition: opacity 0.08 ease-out;
+    transition: opacity 0.08s ease-out;
   }
 
   .blocklyTrashLid {
-    transition: rotate 0.08s ease-out;
+    opacity: 0.4;
+    transition: rotate 0.08s ease-out, opacity 0.08s ease-out;
     transform-origin: 46px 12px;
     rotate: 0deg;
     pointer-events: none;
@@ -729,9 +719,12 @@ Css.register(`
     rotate: -5deg;
   }
 
-  .blocklyTrash.blocklyTrashOpen,
-  .blocklyTrash.blocklyTrashFull:hover,
-  .blocklyTrash.blocklyTrashFull:focus {
+  .blocklyTrash.blocklyTrashOpen > image,
+  .blocklyTrash.blocklyTrashOpen > .blocklyTrashLid,
+  .blocklyTrash.blocklyTrashFull:hover > image,
+  .blocklyTrash.blocklyTrashFull:hover > .blocklyTrashLid,
+  .blocklyTrash.blocklyTrashFull:focus > image,
+  .blocklyTrash.blocklyTrashFull:focus > .blocklyTrashLid {
     opacity: 0.8;
   }
 

packages/blockly/core/utils/focusable_tree_traverser.ts

@@ -72,8 +72,8 @@ export class FocusableTreeTraverser {
    * the specified IFocusableTree.
    *
    * If the element exists within the specified tree's DOM structure but does
-   * not directly correspond to a node, the nearest parent node (or the tree's
-   * root) will be returned to represent the provided element.
+   * not directly correspond to a node and is not a tab stop, the nearest parent
+   * node (or the tree's root) will be returned to represent the provided element.
    *
    * If the tree contains another nested IFocusableTree, the nested tree may be
    * traversed but its nodes will never be returned here per the contract of
@@ -114,9 +114,15 @@ export class FocusableTreeTraverser {
     const matchedChildNode = tree.lookUpFocusableNode(element.id) ?? null;
     if (matchedChildNode) return matchedChildNode;
 
-    // Fourth, recurse up to find the nearest tree/node if it's possible.
+    // Fourth, check if the element is a tab stop.
+    const tabIndexAttr = element.getAttribute('tabindex');
+    if (tabIndexAttr !== null && Number(tabIndexAttr) >= 0) {
+      return null;
+    }
+
+    // Fifth, recurse up to find the nearest tree/node if it's possible.
     const elementParent = element.parentElement;
-    if (!matchedChildNode && elementParent) {
+    if (elementParent) {
       return FocusableTreeTraverser.findFocusableNodeFor(elementParent, tree);
     }
 

packages/blockly/core/workspace_svg.ts

@@ -1016,7 +1016,7 @@ export class WorkspaceSvg
   addTrashcan() {
     this.trashcan = WorkspaceSvg.newTrashcan(this);
     const svgTrashcan = this.trashcan.createDom();
-    this.svgGroup_.insertBefore(svgTrashcan, this.getCanvas());
+    this.svgGroup_.appendChild(svgTrashcan);
   }
 
   /**

packages/blockly/core/zoom_controls.ts

@@ -16,8 +16,6 @@ import {ComponentManager} from './component_manager.js';
 import * as Css from './css.js';
 import {EventType} from './events/type.js';
 import * as eventUtils from './events/utils.js';
-import type {IComponent} from './interfaces/i_component.js';
-import {IFocusableNode} from './interfaces/i_focusable_node.js';
 import type {IPositionable} from './interfaces/i_positionable.js';
 import type {UiMetrics} from './metrics_manager.js';
 import {Msg} from './msg.js';
@@ -37,8 +35,9 @@ import type {WorkspaceSvg} from './workspace_svg.js';
  *
  * @internal
  */
-abstract class ZoomControl implements IFocusableNode, IComponent {
+abstract class ZoomControl {
   private pointerDownHandler: browserEvents.Data;
+  private keyDownHandler: browserEvents.Data;
   id: string;
 
   constructor(
@@ -51,6 +50,12 @@ abstract class ZoomControl implements IFocusableNode, IComponent {
       null,
       this.performAction.bind(this),
     );
+    this.keyDownHandler = browserEvents.bind(
+      group,
+      'keydown',
+      this,
+      this.onKeyDown,
+    );
 
     aria.setRole(group, aria.Role.BUTTON);
 
@@ -85,26 +90,30 @@ abstract class ZoomControl implements IFocusableNode, IComponent {
     eventUtils.fire(uiEvent);
   }
 
-  getFocusableElement() {
+  /**
+   * Returns the SVG group for this zoom button.
+   */
+  getGroup(): SVGGElement {
     return this.group;
   }
 
-  getFocusableTree() {
-    return this.workspace;
-  }
-
-  onNodeFocus() {}
-
-  onNodeBlur() {}
+  abstract performAction(_e: Event): void;
 
-  canBeFocused() {
-    return true;
+  /**
+   * Activates the control when Enter or Space is pressed.
+   *
+   * @param e A keydown event.
+   */
+  private onKeyDown(e: KeyboardEvent) {
+    if (e.key === 'Enter' || e.key === ' ') {
+      e.preventDefault();
+      this.performAction(e);
+    }
   }
 
-  abstract performAction(_e: Event): void;
-
   dispose() {
     browserEvents.unbind(this.pointerDownHandler);
+    browserEvents.unbind(this.keyDownHandler);
   }
 }
 
@@ -380,20 +389,6 @@ export class ZoomControls implements IPositionable {
       );
     }
 
-    for (const control of [
-      this.zoomOutControl,
-      this.zoomInControl,
-      this.zoomResetControl,
-    ]) {
-      if (!control) continue;
-
-      this.workspace.getComponentManager().addComponent({
-        component: control,
-        weight: ComponentManager.ComponentWeight.ZOOM_CONTROLS_WEIGHT,
-        capabilities: [ComponentManager.Capability.FOCUSABLE],
-      });
-    }
-
     return this.svgGroup;
   }
 
@@ -484,13 +479,13 @@ export class ZoomControls implements IPositionable {
     if (verticalPosition === uiPosition.verticalPosition.TOP) {
       const zoomInTranslateY = this.SMALL_SPACING + this.HEIGHT;
       this.zoomInControl
-        ?.getFocusableElement()
+        ?.getGroup()
         .setAttribute('transform', 'translate(0, ' + zoomInTranslateY + ')');
       if (this.zoomResetControl) {
         const zoomResetTranslateY =
           zoomInTranslateY + this.LARGE_SPACING + this.HEIGHT;
         this.zoomResetControl
-          .getFocusableElement()
+          .getGroup()
           .setAttribute(
             'transform',
             'translate(0, ' + zoomResetTranslateY + ')',
@@ -501,12 +496,12 @@ export class ZoomControls implements IPositionable {
         ? this.LARGE_SPACING + this.HEIGHT
         : 0;
       this.zoomInControl
-        ?.getFocusableElement()
+        ?.getGroup()
         .setAttribute('transform', 'translate(0, ' + zoomInTranslateY + ')');
       const zoomOutTranslateY =
         zoomInTranslateY + this.SMALL_SPACING + this.HEIGHT;
       this.zoomOutControl
-        ?.getFocusableElement()
+        ?.getGroup()
         .setAttribute('transform', 'translate(0, ' + zoomOutTranslateY + ')');
     }
 

packages/blockly/tests/mocha/focusable_tree_traverser_test.js

@@ -431,6 +431,36 @@ suite('FocusableTreeTraverser', function () {
       assert.strictEqual(finding, this.testFocusableTree1Node2);
     });
 
+    test('for unregistered tab stop in tree returns null', function () {
+      const tree = this.testFocusableTree1;
+      const unregElem = document.getElementById(
+        'testFocusableTree1.node2.unregisteredChild1',
+      );
+      unregElem.setAttribute('tabindex', '0');
+
+      const finding = FocusableTreeTraverser.findFocusableNodeFor(
+        unregElem,
+        tree,
+      );
+
+      assert.isNull(finding);
+    });
+
+    test('for unregistered tabindex -1 element in tree returns closest node', function () {
+      const tree = this.testFocusableTree1;
+      const unregElem = document.getElementById(
+        'testFocusableTree1.node2.unregisteredChild1',
+      );
+      unregElem.setAttribute('tabindex', '-1');
+
+      const finding = FocusableTreeTraverser.findFocusableNodeFor(
+        unregElem,
+        tree,
+      );
+
+      assert.strictEqual(finding, this.testFocusableTree1Node2);
+    });
+
     test('for nested node element in tree returns node', function () {
       const tree = this.testFocusableTree1;
       const nodeElem = this.testFocusableTree1Node1Child1.getFocusableElement();

packages/blockly/tests/mocha/trashcan_test.js

@@ -398,4 +398,19 @@ suite('Trashcan', function () {
       }
     });
   });
+  suite('Focus', function () {
+    test('is not claimed as a workspace focus node', function () {
+      const trashElement = this.workspace
+        .getParentSvg()
+        .querySelector('.blocklyTrash');
+      assert.isNotNull(trashElement);
+      assert.strictEqual(trashElement.getAttribute('tabindex'), '0');
+      assert.isNull(
+        Blockly.FocusableTreeTraverser.findFocusableNodeFor(
+          trashElement,
+          this.workspace,
+        ),
+      );
+    });
+  });
 });

packages/blockly/tests/mocha/zoom_controls_test.js

@@ -82,4 +82,20 @@ suite('Zoom Controls', function () {
       assert.equal(this.workspace.getScale(), 1);
     });
   });
+
+  suite('Focus', function () {
+    test('is not claimed as a workspace focus node', function () {
+      const zoomIn = this.workspace
+        .getParentSvg()
+        .querySelector('.blocklyZoomIn');
+      assert.isNotNull(zoomIn);
+      assert.strictEqual(zoomIn.getAttribute('tabindex'), '0');
+      assert.isNull(
+        Blockly.FocusableTreeTraverser.findFocusableNodeFor(
+          zoomIn,
+          this.workspace,
+        ),
+      );
+    });
+  });
 });