From 4116083287c482b3bbc58fe1d5609eda14722046 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Mon, 11 May 2026 14:05:04 -0700 Subject: [PATCH] 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 | 10 ++-- packages/blockly/core/dropdowndiv.ts | 12 +++++ packages/blockly/core/inject.ts | 6 +++ packages/blockly/core/widgetdiv.ts | 12 +++++ packages/blockly/core/workspace_svg.ts | 52 +++++++++++++++++++ .../blockly/tests/mocha/dropdowndiv_test.js | 10 ++++ .../blockly/tests/mocha/widget_div_test.js | 10 ++++ 7 files changed, 107 insertions(+), 5 deletions(-) diff --git a/packages/blockly/core/css.ts b/packages/blockly/core/css.ts index b4ecf9e4e..0c1865451 100644 --- a/packages/blockly/core/css.ts +++ b/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); diff --git a/packages/blockly/core/dropdowndiv.ts b/packages/blockly/core/dropdowndiv.ts index f35ee2fc8..5d9194264 100644 --- a/packages/blockly/core/dropdowndiv.ts +++ b/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( 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(); diff --git a/packages/blockly/core/inject.ts b/packages/blockly/core/inject.ts index dffdeef47..8afe54181 100644 --- a/packages/blockly/core/inject.ts +++ b/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; } diff --git a/packages/blockly/core/widgetdiv.ts b/packages/blockly/core/widgetdiv.ts index 6d8ad58d6..05b3db8be 100644 --- a/packages/blockly/core/widgetdiv.ts +++ b/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); } /** diff --git a/packages/blockly/core/workspace_svg.ts b/packages/blockly/core/workspace_svg.ts index c886219d4..b197baa0d 100644 --- a/packages/blockly/core/workspace_svg.ts +++ b/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)}`, + ); } /** diff --git a/packages/blockly/tests/mocha/dropdowndiv_test.js b/packages/blockly/tests/mocha/dropdowndiv_test.js index 5b9a9e41d..62e8c3dc9 100644 --- a/packages/blockly/tests/mocha/dropdowndiv_test.js +++ b/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'), + ); }); }); diff --git a/packages/blockly/tests/mocha/widget_div_test.js b/packages/blockly/tests/mocha/widget_div_test.js index 56c6f9e91..ef05a1943 100644 --- a/packages/blockly/tests/mocha/widget_div_test.js +++ b/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'), + ); }); }); });