fix: Improve display of workspace focus rings (#9848)

* fix: Improve display of workspace focus rings

* fix: Only bind tab listener once

* refactor: Make setup of workspace focus rings more consistent with other DOM elements

* fix: Fix major performance regression in test suite (and perhaps IRL)

* fix: Fix adding of class in `DropDownDiv`

* test: Add tests for adding/removing showing*Div classes

packages/blockly/core/css.ts

@@ -632,11 +632,11 @@ input[type=number] {
   .blocklyWorkspace.blocklyActiveFocus
   .blocklyWorkspaceFocusRing,
 /* Focus in widget/dropdown div considered to be in workspace. */
-.blocklyKeyboardNavigation:has(
-  .blocklyWidgetDiv:focus-within,
-  .blocklyDropDownDiv:focus-within
-)
-  .blocklyWorkspace
+  .blocklyKeyboardNavigation
+  .blocklyWorkspace.blocklyShowingDropDownDiv
+  .blocklyWorkspaceFocusRing,
+.blocklyKeyboardNavigation
+  .blocklyWorkspace.blocklyShowingWidgetDiv
   .blocklyWorkspaceFocusRing {
   stroke: var(--blockly-active-tree-color);
   stroke-width: calc(var(--blockly-selection-width) * 2);

packages/blockly/core/dropdowndiv.ts

@@ -50,6 +50,12 @@ export const PADDING_Y = 16;
 /** Length of animations in seconds. */
 export const ANIMATION_TIME = 0.25;
 
+/**
+ * Class applied to the element that is displaying the DropDownDiv, used to
+ * apply focus styles.
+ */
+const SHOWING_DROPDOWNDIV_SELECTOR = 'blocklyShowingDropDownDiv';
+
 /**
  * Timer for animation out, to be cleared if we need to immediately hide
  * without disrupting new shows.
@@ -411,6 +417,9 @@ export function show<T>(
     aria.State.OWNS,
     existingOwnership ? [existingOwnership, div.id] : div.id,
   );
+  mainWorkspace
+    .getFocusableElement()
+    .classList.add(SHOWING_DROPDOWNDIV_SELECTOR);
 
   // When we change `translate` multiple times in close succession,
   // Chrome may choose to wait and apply them all at once.
@@ -735,6 +744,9 @@ export function hideWithoutAnimation() {
     aria.State.OWNS,
     existingOwnership.replace(div.id, ''),
   );
+  workspace
+    .getFocusableElement()
+    .classList.remove(SHOWING_DROPDOWNDIV_SELECTOR);
 
   workspace.markFocused();
 

packages/blockly/core/inject.ts

@@ -13,6 +13,7 @@ import * as common from './common.js';
 import * as Css from './css.js';
 import * as dropDownDiv from './dropdowndiv.js';
 import {Grid} from './grid.js';
+import {keyboardNavigationController} from './keyboard_navigation_controller.js';
 import {Options} from './options.js';
 import {ScrollbarPair} from './scrollbar_pair.js';
 import * as Tooltip from './tooltip.js';
@@ -318,6 +319,11 @@ function bindDocumentEvents() {
     // should run regardless of what other touch event handlers have run.
     browserEvents.bind(document, 'touchend', null, Touch.longStop);
     browserEvents.bind(document, 'touchcancel', null, Touch.longStop);
+    browserEvents.bind(document, 'keydown', null, function (e: KeyboardEvent) {
+      if (e.key === 'Tab') {
+        keyboardNavigationController.setIsActive(true);
+      }
+    });
   }
   documentEventsBound = true;
 }

packages/blockly/core/widgetdiv.ts

@@ -41,6 +41,12 @@ let containerDiv: HTMLDivElement | null;
 /** Callback to FocusManager to return ephemeral focus when the div closes. */
 let returnEphemeralFocus: ReturnEphemeralFocus | null = null;
 
+/**
+ * Class applied to the element that is displaying the WidgetDiv, used to apply
+ * focus styles.
+ */
+const SHOWING_WIDGETDIV_SELECTOR = 'blocklyShowingWidgetDiv';
+
 /**
  * Returns the HTML container for editor widgets.
  *
@@ -139,6 +145,9 @@ export function show(
     aria.State.OWNS,
     existingOwnership ? [existingOwnership, div.id] : div.id,
   );
+  ownerWorkspace
+    .getFocusableElement()
+    .classList.add(SHOWING_WIDGETDIV_SELECTOR);
 
   const parentDiv = common.getParentContainer();
   parentDiv?.appendChild(div);
@@ -209,6 +218,9 @@ export function hide() {
     aria.State.OWNS,
     existingOwnership.replace(containerDiv.id, ''),
   );
+  ownerWorkspace
+    .getFocusableElement()
+    .classList.remove(SHOWING_WIDGETDIV_SELECTOR);
 }
 
 /**

packages/blockly/core/workspace_svg.ts

@@ -333,6 +333,16 @@ export class WorkspaceSvg
   svgBubbleCanvas_!: SVGElement;
   zoomControls_: ZoomControls | null = null;
 
+  /**
+   * Focus ring in the workspace.
+   */
+  private workspaceFocusRing: Element | null = null;
+
+  /**
+   * Selection ring inside the workspace.
+   */
+  private workspaceSelectionRing: Element | null = null;
+
   /**
    * Navigator that handles moving focus between items in this workspace in
    * response to keyboard navigation commands.
@@ -791,6 +801,23 @@ export class WorkspaceSvg
       }
     }
 
+    this.workspaceSelectionRing = dom.createSvgElement(
+      Svg.RECT,
+      {
+        fill: 'none',
+        class: 'blocklyWorkspaceSelectionRing',
+      },
+      this.svgGroup_,
+    );
+    this.workspaceFocusRing = dom.createSvgElement(
+      Svg.RECT,
+      {
+        fill: 'none',
+        class: 'blocklyWorkspaceFocusRing',
+      },
+      this.svgGroup_,
+    );
+
     this.layerManager = new LayerManager(this);
     // Assign the canvases for backwards compatibility.
     this.svgBlockCanvas_ = this.layerManager.getBlockLayer();
@@ -929,6 +956,9 @@ export class WorkspaceSvg
       this.dummyWheelListener = null;
     }
 
+    this.workspaceFocusRing?.remove();
+    this.workspaceSelectionRing?.remove();
+
     if (getFocusManager().isRegistered(this)) {
       getFocusManager().unregisterTree(this);
     }
@@ -1108,6 +1138,28 @@ export class WorkspaceSvg
       this.scrollbar.resize();
     }
     this.updateScreenCalculations();
+
+    if (!this.workspaceFocusRing || !this.workspaceSelectionRing) return;
+    this.resizeWorkspaceRing(this.workspaceSelectionRing, 5);
+    this.resizeWorkspaceRing(this.workspaceFocusRing, 0);
+  }
+
+  /**
+   * Sizes the given focus/selection ring inside the bounds of the workspace.
+   *
+   * @param ring The interior workspace ring indicator to resize.
+   * @param inset How many pixels in from the bounds of the workspace the ring
+   *     should be positioned.
+   */
+  private resizeWorkspaceRing(ring: Element, inset: number) {
+    const metrics = this.getMetrics();
+    ring.setAttribute('x', `${metrics.absoluteLeft + inset}`);
+    ring.setAttribute('y', `${metrics.absoluteTop + inset}`);
+    ring.setAttribute('width', `${Math.max(0, metrics.viewWidth - inset * 2)}`);
+    ring.setAttribute(
+      'height',
+      `${Math.max(0, metrics.svgHeight - inset * 2)}`,
+    );
   }
 
   /**

packages/blockly/tests/mocha/dropdowndiv_test.js

@@ -207,6 +207,11 @@ suite('DropDownDiv', function () {
         ),
         Blockly.DropDownDiv.getContentDiv().parentElement.id,
       );
+      assert.isTrue(
+        Blockly.getMainWorkspace()
+          .getFocusableElement()
+          .classList.contains('blocklyShowingDropDownDiv'),
+      );
     });
   });
 
@@ -416,6 +421,11 @@ suite('DropDownDiv', function () {
             Blockly.utils.aria.State.OWNS,
           ),
         );
+        assert.isFalse(
+          Blockly.getMainWorkspace()
+            .getFocusableElement()
+            .classList.contains('blocklyShowingDropDownDiv'),
+        );
       });
     });
 

packages/blockly/tests/mocha/widget_div_test.js

@@ -376,6 +376,11 @@ suite('WidgetDiv', function () {
         ),
         Blockly.WidgetDiv.getDiv().id,
       );
+      assert.isTrue(
+        Blockly.getMainWorkspace()
+          .getFocusableElement()
+          .classList.contains('blocklyShowingWidgetDiv'),
+      );
     });
   });
 
@@ -454,6 +459,11 @@ suite('WidgetDiv', function () {
           Blockly.utils.aria.State.OWNS,
         ),
       );
+      assert.isFalse(
+        Blockly.getMainWorkspace()
+          .getFocusableElement()
+          .classList.contains('blocklyShowingWidgetDiv'),
+      );
     });
   });
 });