From 41c8215465a538edd59ffefae067c920c1ce147d Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Mon, 21 Apr 2025 23:26:46 +0000 Subject: [PATCH 01/27] feat: Make WorkspaceSvg focusable. This adds basic support for WorkspaceSvg being focusable via its blocks, though fields and connections are not yet supported. --- core/block_svg.ts | 30 +++++++++++- core/inject.ts | 5 -- core/workspace_svg.ts | 77 ++++++++++++++++++++++++++++++- tests/mocha/workspace_svg_test.js | 2 +- 4 files changed, 105 insertions(+), 9 deletions(-) diff --git a/core/block_svg.ts b/core/block_svg.ts index b8712b01914..7b11e142148 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -44,6 +44,8 @@ import {IContextMenu} from './interfaces/i_contextmenu.js'; import type {ICopyable} from './interfaces/i_copyable.js'; import {IDeletable} from './interfaces/i_deletable.js'; import type {IDragStrategy, IDraggable} from './interfaces/i_draggable.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import {IIcon} from './interfaces/i_icon.js'; import * as internalConstants from './internal_constants.js'; import {MarkerManager} from './marker_manager.js'; @@ -76,7 +78,8 @@ export class BlockSvg IContextMenu, ICopyable, IDraggable, - IDeletable + IDeletable, + IFocusableNode { /** * Constant for identifying rows that are to be rendered inline. @@ -210,6 +213,7 @@ export class BlockSvg // Expose this block's ID on its top-level SVG group. this.svgGroup.setAttribute('data-id', this.id); + svgPath.id = this.id; this.doInit_(); } @@ -1819,4 +1823,28 @@ export class BlockSvg ); } } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + return this.pathObject.svgPath; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.workspace; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void { + if (!this.isShadow()) { + common.setSelected(this); + } + } + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void { + if (common.getSelected() === this) { + common.setSelected(null); + } + } } diff --git a/core/inject.ts b/core/inject.ts index de78fbfae75..34d9c1795f8 100644 --- a/core/inject.ts +++ b/core/inject.ts @@ -13,13 +13,11 @@ import * as common from './common.js'; import * as Css from './css.js'; import * as dropDownDiv from './dropdowndiv.js'; import {Grid} from './grid.js'; -import {Msg} from './msg.js'; import {Options} from './options.js'; import {ScrollbarPair} from './scrollbar_pair.js'; import {ShortcutRegistry} from './shortcut_registry.js'; import * as Tooltip from './tooltip.js'; import * as Touch from './touch.js'; -import * as aria from './utils/aria.js'; import * as dom from './utils/dom.js'; import {Svg} from './utils/svg.js'; import * as WidgetDiv from './widgetdiv.js'; @@ -56,8 +54,6 @@ export function inject( if (opt_options?.rtl) { dom.addClass(subContainer, 'blocklyRTL'); } - subContainer.tabIndex = 0; - aria.setState(subContainer, aria.State.LABEL, Msg['WORKSPACE_ARIA_LABEL']); containerElement!.appendChild(subContainer); const svg = createDom(subContainer, options); @@ -126,7 +122,6 @@ function createDom(container: HTMLElement, options: Options): SVGElement { 'xmlns:xlink': dom.XLINK_NS, 'version': '1.1', 'class': 'blocklySvg', - 'tabindex': '0', }, container, ); diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 91668b744d4..6d6505d0545 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -37,6 +37,7 @@ import {EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; import {Flyout} from './flyout_base.js'; import type {FlyoutButton} from './flyout_button.js'; +import {getFocusManager} from './focus_manager.js'; import {Gesture} from './gesture.js'; import {Grid} from './grid.js'; import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js'; @@ -44,6 +45,8 @@ import type {IBoundedElement} from './interfaces/i_bounded_element.js'; import {IContextMenu} from './interfaces/i_contextmenu.js'; import type {IDragTarget} from './interfaces/i_drag_target.js'; import type {IFlyout} from './interfaces/i_flyout.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {IMetricsManager} from './interfaces/i_metrics_manager.js'; import type {IToolbox} from './interfaces/i_toolbox.js'; import type { @@ -54,6 +57,7 @@ import type {LineCursor} from './keyboard_nav/line_cursor.js'; import type {Marker} from './keyboard_nav/marker.js'; import {LayerManager} from './layer_manager.js'; import {MarkerManager} from './marker_manager.js'; +import {Msg} from './msg.js'; import {Options} from './options.js'; import * as Procedures from './procedures.js'; import * as registry from './registry.js'; @@ -66,6 +70,7 @@ import {Classic} from './theme/classic.js'; import {ThemeManager} from './theme_manager.js'; import * as Tooltip from './tooltip.js'; import type {Trashcan} from './trashcan.js'; +import * as aria from './utils/aria.js'; import * as arrayUtils from './utils/array.js'; import {Coordinate} from './utils/coordinate.js'; import * as dom from './utils/dom.js'; @@ -93,7 +98,7 @@ const ZOOM_TO_FIT_MARGIN = 20; */ export class WorkspaceSvg extends Workspace - implements IASTNodeLocationSvg, IContextMenu + implements IASTNodeLocationSvg, IContextMenu, IFocusableNode, IFocusableTree { /** * A wrapper function called when a resize event occurs. @@ -764,7 +769,19 @@ export class WorkspaceSvg * * */ - this.svgGroup_ = dom.createSvgElement(Svg.G, {'class': 'blocklyWorkspace'}); + this.svgGroup_ = dom.createSvgElement(Svg.G, { + 'class': 'blocklyWorkspace', + // Only the main workspace should be tabbable. + 'tabindex': injectionDiv ? '0' : '-1', + 'id': this.id, + }); + if (injectionDiv) { + aria.setState( + this.svgGroup_, + aria.State.LABEL, + Msg['WORKSPACE_ARIA_LABEL'], + ); + } // Note that a alone does not receive mouse events--it must have a // valid target inside it. If no background class is specified, as in the @@ -840,6 +857,9 @@ export class WorkspaceSvg this.getTheme(), isParentWorkspace ? this.getInjectionDiv() : undefined, ); + + getFocusManager().registerTree(this); + return this.svgGroup_; } @@ -924,6 +944,10 @@ export class WorkspaceSvg document.body.removeEventListener('wheel', this.dummyWheelListener); this.dummyWheelListener = null; } + + if (getFocusManager().isRegistered(this)) { + getFocusManager().unregisterTree(this); + } } /** @@ -2618,6 +2642,55 @@ export class WorkspaceSvg deltaY *= scale; this.scroll(this.scrollX + deltaX, this.scrollY + deltaY); } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + return this.svgGroup_; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} + + /** See IFocusableTree.getRootFocusableNode. */ + getRootFocusableNode(): IFocusableNode { + return this; + } + + /** See IFocusableTree.getRestoredFocusableNode. */ + getRestoredFocusableNode( + previousNode: IFocusableNode | null, + ): IFocusableNode | null { + if (!previousNode) { + return this.getTopBlocks(true)[0] ?? null; + } else return null; + } + + /** See IFocusableTree.getNestedTrees. */ + getNestedTrees(): Array { + return []; + } + + /** See IFocusableTree.lookUpFocusableNode. */ + lookUpFocusableNode(id: string): IFocusableNode | null { + return this.getBlockById(id) as IFocusableNode; + } + + /** See IFocusableTree.onTreeFocus. */ + onTreeFocus( + _node: IFocusableNode, + _previousTree: IFocusableTree | null, + ): void {} + + /** See IFocusableTree.onTreeBlur. */ + onTreeBlur(_nextTree: IFocusableTree | null): void {} } /** diff --git a/tests/mocha/workspace_svg_test.js b/tests/mocha/workspace_svg_test.js index 207cad45dc7..f13ba27b16b 100644 --- a/tests/mocha/workspace_svg_test.js +++ b/tests/mocha/workspace_svg_test.js @@ -18,7 +18,7 @@ import { } from './test_helpers/setup_teardown.js'; import {testAWorkspace} from './test_helpers/workspace.js'; -suite('WorkspaceSvg', function () { +suite.only('WorkspaceSvg', function () { setup(function () { this.clock = sharedTestSetup.call(this, {fireEventsNow: false}).clock; const toolbox = document.getElementById('toolbox-categories'); From 14b486e6cc25402d5cefb4852fce35f0018c55f0 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Mon, 21 Apr 2025 23:50:40 +0000 Subject: [PATCH 02/27] chore: remove accidental 'test.only'. --- tests/mocha/workspace_svg_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mocha/workspace_svg_test.js b/tests/mocha/workspace_svg_test.js index f13ba27b16b..207cad45dc7 100644 --- a/tests/mocha/workspace_svg_test.js +++ b/tests/mocha/workspace_svg_test.js @@ -18,7 +18,7 @@ import { } from './test_helpers/setup_teardown.js'; import {testAWorkspace} from './test_helpers/workspace.js'; -suite.only('WorkspaceSvg', function () { +suite('WorkspaceSvg', function () { setup(function () { this.clock = sharedTestSetup.call(this, {fireEventsNow: false}).clock; const toolbox = document.getElementById('toolbox-categories'); From 26cf8dbf8a2c0a59f6694b0fce2ee4d8107d8b51 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 22 Apr 2025 00:17:50 +0000 Subject: [PATCH 03/27] feat: Make Toolbox & Flyout focusable. This introduces the fundamental support needed to ensure that both toolboxes and flyouts are focusable using FocusManager. --- core/flyout_base.ts | 69 +++++++++++++++++++++++++++- core/interfaces/i_flyout.ts | 3 +- core/interfaces/i_toolbox.ts | 3 +- core/interfaces/i_toolbox_item.ts | 4 +- core/toolbox/category.ts | 2 + core/toolbox/separator.ts | 2 + core/toolbox/toolbox.ts | 74 ++++++++++++++++++++++++++++++- core/toolbox/toolbox_item.ts | 24 ++++++++++ core/workspace_svg.ts | 14 +++++- tests/mocha/toolbox_test.js | 3 ++ 10 files changed, 191 insertions(+), 7 deletions(-) diff --git a/core/flyout_base.ts b/core/flyout_base.ts index e738470a606..d24ea2758a0 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -21,9 +21,12 @@ import * as eventUtils from './events/utils.js'; import {FlyoutItem} from './flyout_item.js'; import {FlyoutMetricsManager} from './flyout_metrics_manager.js'; import {FlyoutSeparator, SeparatorAxis} from './flyout_separator.js'; +import {getFocusManager} from './focus_manager.js'; import {IAutoHideable} from './interfaces/i_autohideable.js'; import type {IFlyout} from './interfaces/i_flyout.js'; import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js'; +import {IFocusableNode} from './interfaces/i_focusable_node.js'; +import {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {Options} from './options.js'; import * as registry from './registry.js'; import * as renderManagement from './render_management.js'; @@ -43,7 +46,7 @@ import {WorkspaceSvg} from './workspace_svg.js'; */ export abstract class Flyout extends DeleteArea - implements IAutoHideable, IFlyout + implements IAutoHideable, IFlyout, IFocusableNode { /** * Position the flyout. @@ -303,6 +306,7 @@ export abstract class Flyout // hide/show code will set up proper visibility and size later. this.svgGroup_ = dom.createSvgElement(tagName, { 'class': 'blocklyFlyout', + 'tabindex': '0', }); this.svgGroup_.style.display = 'none'; this.svgBackground_ = dom.createSvgElement( @@ -317,6 +321,9 @@ export abstract class Flyout this.workspace_ .getThemeManager() .subscribe(this.svgBackground_, 'flyoutOpacity', 'fill-opacity'); + + getFocusManager().registerTree(this); + return this.svgGroup_; } @@ -398,6 +405,7 @@ export abstract class Flyout if (this.svgGroup_) { dom.removeNode(this.svgGroup_); } + getFocusManager().unregisterTree(this); } /** @@ -961,4 +969,63 @@ export abstract class Flyout return null; } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + if (!this.svgGroup_) throw new Error('Flyout DOM is not yet created.'); + return this.svgGroup_; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} + + /** See IFocusableTree.getRootFocusableNode. */ + getRootFocusableNode(): IFocusableNode { + return this; + } + + /** See IFocusableTree.getRestoredFocusableNode. */ + getRestoredFocusableNode( + _previousNode: IFocusableNode | null, + ): IFocusableNode | null { + return null; + } + + /** See IFocusableTree.getNestedTrees. */ + getNestedTrees(): Array { + return [this.workspace_]; + } + + /** See IFocusableTree.lookUpFocusableNode. */ + lookUpFocusableNode(_id: string): IFocusableNode | null { + // No focusable node needs to be returned since the flyout's subtree is a + // workspace that will manage its own focusable state. + return null; + } + + /** See IFocusableTree.onTreeFocus. */ + onTreeFocus( + _node: IFocusableNode, + _previousTree: IFocusableTree | null, + ): void {} + + /** See IFocusableTree.onTreeBlur. */ + onTreeBlur(nextTree: IFocusableTree | null): void { + const toolbox = this.targetWorkspace.getToolbox(); + // If focus is moving to either the toolbox or the flyout's workspace, do + // not close the flyout. For anything else, do close it since the flyout is + // no longer focused. + if (toolbox && nextTree === toolbox) return; + if (nextTree == this.workspace_) return; + if (toolbox) toolbox.clearSelection(); + this.autoHide(false); + } } diff --git a/core/interfaces/i_flyout.ts b/core/interfaces/i_flyout.ts index 42204775ece..067cd5ef20d 100644 --- a/core/interfaces/i_flyout.ts +++ b/core/interfaces/i_flyout.ts @@ -12,12 +12,13 @@ import type {Coordinate} from '../utils/coordinate.js'; import type {Svg} from '../utils/svg.js'; import type {FlyoutDefinition} from '../utils/toolbox.js'; import type {WorkspaceSvg} from '../workspace_svg.js'; +import {IFocusableTree} from './i_focusable_tree.js'; import type {IRegistrable} from './i_registrable.js'; /** * Interface for a flyout. */ -export interface IFlyout extends IRegistrable { +export interface IFlyout extends IRegistrable, IFocusableTree { /** Whether the flyout is laid out horizontally or not. */ horizontalLayout: boolean; diff --git a/core/interfaces/i_toolbox.ts b/core/interfaces/i_toolbox.ts index 1780af94d8a..f5d9c9fd7c6 100644 --- a/core/interfaces/i_toolbox.ts +++ b/core/interfaces/i_toolbox.ts @@ -9,13 +9,14 @@ import type {ToolboxInfo} from '../utils/toolbox.js'; import type {WorkspaceSvg} from '../workspace_svg.js'; import type {IFlyout} from './i_flyout.js'; +import type {IFocusableTree} from './i_focusable_tree.js'; import type {IRegistrable} from './i_registrable.js'; import type {IToolboxItem} from './i_toolbox_item.js'; /** * Interface for a toolbox. */ -export interface IToolbox extends IRegistrable { +export interface IToolbox extends IRegistrable, IFocusableTree { /** Initializes the toolbox. */ init(): void; diff --git a/core/interfaces/i_toolbox_item.ts b/core/interfaces/i_toolbox_item.ts index e3c9864f0c0..661624fd7e8 100644 --- a/core/interfaces/i_toolbox_item.ts +++ b/core/interfaces/i_toolbox_item.ts @@ -6,10 +6,12 @@ // Former goog.module ID: Blockly.IToolboxItem +import type {IFocusableNode} from './i_focusable_node.js'; + /** * Interface for an item in the toolbox. */ -export interface IToolboxItem { +export interface IToolboxItem extends IFocusableNode { /** * Initializes the toolbox item. * This includes creating the DOM and updating the state of any items based diff --git a/core/toolbox/category.ts b/core/toolbox/category.ts index d8ee8736ea6..fc7d1aa03cf 100644 --- a/core/toolbox/category.ts +++ b/core/toolbox/category.ts @@ -225,6 +225,8 @@ export class ToolboxCategory */ protected createContainer_(): HTMLDivElement { const container = document.createElement('div'); + container.tabIndex = -1; + container.id = this.getId(); const className = this.cssConfig_['container']; if (className) { dom.addClass(container, className); diff --git a/core/toolbox/separator.ts b/core/toolbox/separator.ts index 31ccb7e42f3..44ae358cf53 100644 --- a/core/toolbox/separator.ts +++ b/core/toolbox/separator.ts @@ -54,6 +54,8 @@ export class ToolboxSeparator extends ToolboxItem { */ protected createDom_(): HTMLDivElement { const container = document.createElement('div'); + container.tabIndex = -1; + container.id = this.getId(); const className = this.cssConfig_['container']; if (className) { dom.addClass(container, className); diff --git a/core/toolbox/toolbox.ts b/core/toolbox/toolbox.ts index b0fd82e97f2..f44ebac5348 100644 --- a/core/toolbox/toolbox.ts +++ b/core/toolbox/toolbox.ts @@ -22,11 +22,14 @@ import {DeleteArea} from '../delete_area.js'; import '../events/events_toolbox_item_select.js'; import {EventType} from '../events/type.js'; import * as eventUtils from '../events/utils.js'; +import {getFocusManager} from '../focus_manager.js'; import type {IAutoHideable} from '../interfaces/i_autohideable.js'; import type {ICollapsibleToolboxItem} from '../interfaces/i_collapsible_toolbox_item.js'; import {isDeletable} from '../interfaces/i_deletable.js'; 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 {IFocusableTree} from '../interfaces/i_focusable_tree.js'; import type {IKeyboardAccessible} from '../interfaces/i_keyboard_accessible.js'; import type {ISelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.js'; import {isSelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.js'; @@ -51,7 +54,12 @@ import {CollapsibleToolboxCategory} from './collapsible_category.js'; */ export class Toolbox extends DeleteArea - implements IAutoHideable, IKeyboardAccessible, IStyleable, IToolbox + implements + IAutoHideable, + IKeyboardAccessible, + IStyleable, + IToolbox, + IFocusableNode { /** * The unique ID for this component that is used to register with the @@ -163,6 +171,7 @@ export class Toolbox ComponentManager.Capability.DRAG_TARGET, ], }); + getFocusManager().registerTree(this); } /** @@ -177,7 +186,6 @@ export class Toolbox const container = this.createContainer_(); this.contentsDiv_ = this.createContentsContainer_(); - this.contentsDiv_.tabIndex = 0; aria.setRole(this.contentsDiv_, aria.Role.TREE); container.appendChild(this.contentsDiv_); @@ -194,6 +202,7 @@ export class Toolbox */ protected createContainer_(): HTMLDivElement { const toolboxContainer = document.createElement('div'); + toolboxContainer.tabIndex = 0; toolboxContainer.setAttribute('layout', this.isHorizontal() ? 'h' : 'v'); dom.addClass(toolboxContainer, 'blocklyToolbox'); toolboxContainer.setAttribute('dir', this.RTL ? 'RTL' : 'LTR'); @@ -1077,7 +1086,68 @@ export class Toolbox this.workspace_.getThemeManager().unsubscribe(this.HtmlDiv); dom.removeNode(this.HtmlDiv); } + + getFocusManager().unregisterTree(this); + } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + if (!this.HtmlDiv) throw Error('Toolbox DOM has not yet been created.'); + return this.HtmlDiv; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this; } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} + + /** See IFocusableTree.getRootFocusableNode. */ + getRootFocusableNode(): IFocusableNode { + return this; + } + + /** See IFocusableTree.getRestoredFocusableNode. */ + getRestoredFocusableNode( + previousNode: IFocusableNode | null, + ): IFocusableNode | null { + // Always try to select the first selectable toolbox item rather than the + // root of the toolbox. + if (!previousNode || previousNode === this) { + return this.getToolboxItems().find((item) => item.isSelectable()) ?? null; + } else return null; + } + + /** See IFocusableTree.getNestedTrees. */ + getNestedTrees(): Array { + return []; + } + + /** See IFocusableTree.lookUpFocusableNode. */ + lookUpFocusableNode(id: string): IFocusableNode | null { + return this.getToolboxItemById(id) as IFocusableNode; + } + + /** See IFocusableTree.onTreeFocus. */ + onTreeFocus( + node: IFocusableNode, + _previousTree: IFocusableTree | null, + ): void { + if (node !== this) { + // Only select the item if it isn't already selected so as to not toggle. + if (this.getSelectedItem() !== node) { + this.setSelectedItem(node as IToolboxItem); + } + } else this.clearSelection(); + } + + /** See IFocusableTree.onTreeBlur. */ + onTreeBlur(_nextTree: IFocusableTree | null): void {} } /** CSS for Toolbox. See css.js for use. */ diff --git a/core/toolbox/toolbox_item.ts b/core/toolbox/toolbox_item.ts index ef9d979ab43..0d46a5eadfd 100644 --- a/core/toolbox/toolbox_item.ts +++ b/core/toolbox/toolbox_item.ts @@ -12,6 +12,7 @@ // Former goog.module ID: Blockly.ToolboxItem import type {ICollapsibleToolboxItem} from '../interfaces/i_collapsible_toolbox_item.js'; +import type {IFocusableTree} from '../interfaces/i_focusable_tree.js'; import type {IToolbox} from '../interfaces/i_toolbox.js'; import type {IToolboxItem} from '../interfaces/i_toolbox_item.js'; import * as idGenerator from '../utils/idgenerator.js'; @@ -148,5 +149,28 @@ export class ToolboxItem implements IToolboxItem { * @param _isVisible True if category should be visible. */ setVisible_(_isVisible: boolean) {} + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + const div = this.getDiv(); + if (!div) { + throw Error('Trying to access toolbox item before DOM is initialized.'); + } + if (!(div instanceof HTMLElement)) { + throw Error('Toolbox item div is unexpectedly not an HTML element.'); + } + return div as HTMLElement; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.parentToolbox_; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} } // nop by default diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 6d6505d0545..66b21e9a731 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -2690,7 +2690,19 @@ export class WorkspaceSvg ): void {} /** See IFocusableTree.onTreeBlur. */ - onTreeBlur(_nextTree: IFocusableTree | null): void {} + onTreeBlur(nextTree: IFocusableTree | null): void { + // If the flyout loses focus, make sure to close it. + if (this.isFlyout && this.targetWorkspace) { + // Only hide the flyout if the flyout's workspace is losing focus and that + // focus isn't returning to the flyout itself or the toolbox. + const flyout = this.targetWorkspace.getFlyout(); + const toolbox = this.targetWorkspace.getToolbox(); + if (flyout && nextTree === flyout) return; + if (toolbox && nextTree === toolbox) return; + if (toolbox) toolbox.clearSelection(); + if (flyout && flyout instanceof Flyout) flyout.autoHide(false); + } + } } /** diff --git a/tests/mocha/toolbox_test.js b/tests/mocha/toolbox_test.js index 10bfd335223..f32319c6779 100644 --- a/tests/mocha/toolbox_test.js +++ b/tests/mocha/toolbox_test.js @@ -54,6 +54,7 @@ suite('Toolbox', function () { const themeManagerSpy = sinon.spy(themeManager, 'subscribe'); const componentManager = this.toolbox.workspace_.getComponentManager(); sinon.stub(componentManager, 'addComponent'); + this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited. this.toolbox.init(); sinon.assert.calledWith( themeManagerSpy, @@ -72,12 +73,14 @@ suite('Toolbox', function () { const renderSpy = sinon.spy(this.toolbox, 'render'); const componentManager = this.toolbox.workspace_.getComponentManager(); sinon.stub(componentManager, 'addComponent'); + this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited. this.toolbox.init(); sinon.assert.calledOnce(renderSpy); }); test('Init called -> Flyout is initialized', function () { const componentManager = this.toolbox.workspace_.getComponentManager(); sinon.stub(componentManager, 'addComponent'); + this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited. this.toolbox.init(); assert.isDefined(this.toolbox.getFlyout()); }); From d3acbff46f7826c78fcaaff2d8d28639aaf26a3f Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 22 Apr 2025 21:15:41 +0000 Subject: [PATCH 04/27] feat!: Force lifecycle methods for fields. This ensures that fields with editors properly signal when the editor is shown/hidden, and introduces show/hide callbacks that can be overridden to properly trigger ephemeral focus. --- blocks/procedures.ts | 6 ++-- core/field.ts | 23 +++++++++++--- core/field_checkbox.ts | 10 +++++- core/field_dropdown.ts | 18 ++++++++--- core/field_image.ts | 10 +++++- core/field_input.ts | 31 +++++++++++++++---- core/field_label.ts | 11 +++++++ tests/mocha/field_textinput_test.js | 4 +-- .../src/field/different_user_input.ts | 11 +++++++ 9 files changed, 104 insertions(+), 20 deletions(-) diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 7284973d9cc..317eb82190b 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -648,8 +648,10 @@ class ProcedureArgumentField extends FieldTextInput { * * @param e The event that triggered display of the field editor. */ - protected override showEditor_(e?: Event) { - super.showEditor_(e); + protected override showEditor_( + onEditorShown: () => void, onEditorHidden: () => void, e?: Event + ) { + super.showEditor_(onEditorShown, onEditorHidden, e); this.editingInteractively = true; this.editingVariable = undefined; } diff --git a/core/field.ts b/core/field.ts index 725a2867d9e..cecb90ff385 100644 --- a/core/field.ts +++ b/core/field.ts @@ -770,7 +770,7 @@ export abstract class Field */ showEditor(e?: Event) { if (this.isClickable()) { - this.showEditor_(e); + this.showEditor_(() => this.onShowEditor(), () => this.onHideEditor(), e); } } @@ -778,11 +778,26 @@ export abstract class Field * A developer hook to create an editor for the field. This is no-op by * default, and must be overriden to create an editor. * - * @param _e Optional mouse event that triggered the field to open, or + * @param onEditorShown Callback that must be called when the editor is shown. + * @param onEditorHidden Callback that must be called when the editor hides. + * @param e Optional mouse event that triggered the field to open, or * undefined if triggered programmatically. */ - protected showEditor_(_e?: Event): void {} - // NOP + protected abstract showEditor_( + onEditorShown: () => void, onEditorHidden: () => void, e?: Event + ): void; + + /** + * Called when an editor is shown. This is expected to be used for ensuring + * that the editor has proper focus. + */ + protected abstract onShowEditor(): void; + + /** + * Called when an editor is hidden. This is expected to be used for ensuring + * that the editor no longer proper focus. + */ + protected abstract onHideEditor(): void; /** * A developer hook to reposition the WidgetDiv during a window resize. You diff --git a/core/field_checkbox.ts b/core/field_checkbox.ts index 55ed42cbf4b..3d32585ad98 100644 --- a/core/field_checkbox.ts +++ b/core/field_checkbox.ts @@ -136,10 +136,18 @@ export class FieldCheckbox extends Field { } /** Toggle the state of the checkbox on click. */ - protected override showEditor_() { + protected override showEditor_( + onEditorShown: () => void, onEditorHidden: () => void + ) { + onEditorShown(); this.setValue(!this.value_); + onEditorHidden(); } + protected override onShowEditor(): void {} + + protected override onHideEditor(): void {} + /** * Ensure that the input value is valid ('TRUE' or 'FALSE'). * diff --git a/core/field_dropdown.ts b/core/field_dropdown.ts index 81279e2a1f5..4afa2dffca7 100644 --- a/core/field_dropdown.ts +++ b/core/field_dropdown.ts @@ -269,7 +269,9 @@ export class FieldDropdown extends Field { * @param e Optional mouse event that triggered the field to open, or * undefined if triggered programmatically. */ - protected override showEditor_(e?: MouseEvent) { + protected override showEditor_( + onEditorShown: () => void, onEditorHidden: () => void, e?: MouseEvent + ) { const block = this.getSourceBlock(); if (!block) { throw new UnattachedFieldError(); @@ -277,6 +279,7 @@ export class FieldDropdown extends Field { this.dropdownCreate(); if (!this.menu_) return; + onEditorShown(); if (e && typeof e.clientX === 'number') { this.menu_.openingCoords = new Coordinate(e.clientX, e.clientY); } else { @@ -295,7 +298,10 @@ export class FieldDropdown extends Field { dropDownDiv.setColour(primaryColour, borderColour); } - dropDownDiv.showPositionedByField(this, this.dropdownDispose_.bind(this)); + dropDownDiv.showPositionedByField(this, () => { + this.dropdownDispose_.bind(this); + onEditorHidden(); + }); dropDownDiv.getContentDiv().style.height = `${this.menu_.getSize().height}px`; @@ -311,6 +317,10 @@ export class FieldDropdown extends Field { this.applyColour(); } + protected override onShowEditor(): void {} + + protected override onHideEditor(): void {} + /** Create the dropdown editor. */ private dropdownCreate() { const block = this.getSourceBlock(); @@ -769,7 +779,7 @@ export class FieldDropdown extends Field { } else if (typeof option[1] !== 'string') { foundError = true; console.error( - `Invalid option[${i}]: Each FieldDropdown option id must be a string. + `Invalid option[${i}]: Each FieldDropdown option id must be a string. Found ${option[1]} in: ${option}`, ); } else if ( @@ -780,7 +790,7 @@ export class FieldDropdown extends Field { ) { foundError = true; console.error( - `Invalid option[${i}]: Each FieldDropdown option must have a string + `Invalid option[${i}]: Each FieldDropdown option must have a string label, image description, or HTML element. Found ${option[0]} in: ${option}`, ); } diff --git a/core/field_image.ts b/core/field_image.ts index 650575f59a3..9b809e4058b 100644 --- a/core/field_image.ts +++ b/core/field_image.ts @@ -217,12 +217,20 @@ export class FieldImage extends Field { * If field click is called, and click handler defined, * call the handler. */ - protected override showEditor_() { + protected override showEditor_( + _onEditorShown: () => void, _onEditorHidden: () => void + ) { + // Note that an editor shouldn't be shown for this field, so the callbacks + // are ignored. if (this.clickHandler) { this.clickHandler(this); } } + protected override onShowEditor(): void {} + + protected override onHideEditor(): void {} + /** * Set the function that is called when this image is clicked. * diff --git a/core/field_input.ts b/core/field_input.ts index 2cdd8056553..615521c41cc 100644 --- a/core/field_input.ts +++ b/core/field_input.ts @@ -327,30 +327,42 @@ export abstract class FieldInput extends Field< * Shows a prompt editor for mobile browsers if the modalInputs option is * enabled. * + * @param onEditorShown Callback that must be called when the editor is shown. + * @param onEditorHidden Callback that must be called when the editor hides. * @param _e Optional mouse event that triggered the field to open, or * undefined if triggered programmatically. * @param quietInput True if editor should be created without focus. * Defaults to false. */ - protected override showEditor_(_e?: Event, quietInput = false) { + protected override showEditor_( + onEditorShown: () => void, onEditorHidden: () => void, _e?: Event, + quietInput: boolean = false + ) { this.workspace_ = (this.sourceBlock_ as BlockSvg).workspace; if ( !quietInput && this.workspace_.options.modalInputs && (userAgent.MOBILE || userAgent.ANDROID || userAgent.IPAD) ) { - this.showPromptEditor(); + this.showPromptEditor(onEditorShown, onEditorHidden); } else { - this.showInlineEditor(quietInput); + this.showInlineEditor(onEditorShown, onEditorHidden, quietInput); } } + protected override onShowEditor(): void {} + + protected override onHideEditor(): void {} + /** * Create and show a text input editor that is a prompt (usually a popup). * Mobile browsers may have issues with in-line textareas (focus and * keyboards). */ - private showPromptEditor() { + private showPromptEditor( + onEditorShown: () => void, onEditorHidden: () => void + ) { + onEditorShown(); dialog.prompt( Msg['CHANGE_VALUE_TITLE'], this.getText(), @@ -360,6 +372,7 @@ export abstract class FieldInput extends Field< this.setValue(this.getValueFromEditorText_(text)); } this.onFinishEditing_(this.value_); + onEditorHidden(); }, ); } @@ -369,7 +382,9 @@ export abstract class FieldInput extends Field< * * @param quietInput True if editor should be created without focus. */ - private showInlineEditor(quietInput: boolean) { + private showInlineEditor( + onEditorShown: () => void, onEditorHidden: () => void, quietInput: boolean + ) { const block = this.getSourceBlock(); if (!block) { throw new UnattachedFieldError(); @@ -377,9 +392,13 @@ export abstract class FieldInput extends Field< WidgetDiv.show( this, block.RTL, - this.widgetDispose_.bind(this), + () => { + this.widgetDispose_(); + onEditorHidden(); + }, this.workspace_, ); + onEditorShown(); this.htmlInput_ = this.widgetCreate_() as HTMLInputElement; this.isBeingEdited_ = true; this.valueWhenEditorWasOpened_ = this.value_; diff --git a/core/field_label.ts b/core/field_label.ts index 236154cc7b1..d108ac499fe 100644 --- a/core/field_label.ts +++ b/core/field_label.ts @@ -94,6 +94,17 @@ export class FieldLabel extends Field { return `${newValue}`; } + protected override showEditor_( + _onEditorShown: () => void, _onEditorHidden: () => void + ) { + // Note that an editor shouldn't be shown for this field, so the callbacks + // are ignored. + } + + protected override onShowEditor(): void {} + + protected override onHideEditor(): void {} + /** * Set the CSS class applied to the field's textElement_. * diff --git a/tests/mocha/field_textinput_test.js b/tests/mocha/field_textinput_test.js index 82c1a645e6d..4ab40d4f506 100644 --- a/tests/mocha/field_textinput_test.js +++ b/tests/mocha/field_textinput_test.js @@ -228,7 +228,7 @@ suite('Text Input Fields', function () { this.assertSpellcheck = function (field, value) { this.prepField(field); - field.showEditor_(); + field.showEditor_(() => {}, () => {}); assert.equal( field.htmlInput_.getAttribute('spellcheck'), value.toString(), @@ -265,7 +265,7 @@ suite('Text Input Fields', function () { test('setSpellcheck Editor Shown', function () { const field = new Blockly.FieldTextInput('test'); this.prepField(field); - field.showEditor_(); + field.showEditor_(() => {}, () => {}); field.setSpellcheck(false); assert.equal(field.htmlInput_.getAttribute('spellcheck'), 'false'); }); diff --git a/tests/typescript/src/field/different_user_input.ts b/tests/typescript/src/field/different_user_input.ts index f91f25bbc86..afc150da48d 100644 --- a/tests/typescript/src/field/different_user_input.ts +++ b/tests/typescript/src/field/different_user_input.ts @@ -61,6 +61,17 @@ class FieldMitosis extends Field { }); this.value_ = {cells}; } + + protected override showEditor_( + _onEditorShown: () => void, _onEditorHidden: () => void + ) { + // Note that an editor shouldn't be shown for this field, so the callbacks + // are ignored. + } + + protected override onShowEditor(): void {} + + protected override onHideEditor(): void {} } fieldRegistry.register('field_mitosis', FieldMitosis); From ed0f14070d80611da409c9b11701cd87d06b1db5 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 22 Apr 2025 21:29:54 +0000 Subject: [PATCH 05/27] feat: Make fields ephemerally focusable. --- core/field.ts | 43 +++++++++++++++++++++++++++++++++++++++--- core/field_checkbox.ts | 6 +++--- core/field_dropdown.ts | 15 +++++++++++++-- core/field_input.ts | 12 ++++++++++-- 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/core/field.ts b/core/field.ts index cecb90ff385..bc091955f76 100644 --- a/core/field.ts +++ b/core/field.ts @@ -25,6 +25,8 @@ import * as eventUtils from './events/utils.js'; import type {Input} from './inputs/input.js'; import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js'; import type {IASTNodeLocationWithBlock} from './interfaces/i_ast_node_location_with_block.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {IKeyboardAccessible} from './interfaces/i_keyboard_accessible.js'; import type {IRegistrable} from './interfaces/i_registrable.js'; import {ISerializable} from './interfaces/i_serializable.js'; @@ -42,7 +44,8 @@ import {Svg} from './utils/svg.js'; import * as userAgent from './utils/useragent.js'; import * as utilsXml from './utils/xml.js'; import * as WidgetDiv from './widgetdiv.js'; -import type {WorkspaceSvg} from './workspace_svg.js'; +import {WorkspaceSvg} from './workspace_svg.js'; +import * as idGenerator from './utils/idgenerator.js'; /** * A function that is called to validate changes to the field's value before @@ -72,7 +75,8 @@ export abstract class Field IASTNodeLocationWithBlock, IKeyboardAccessible, IRegistrable, - ISerializable + ISerializable, + IFocusableNode { /** * To overwrite the default value which is set in **Field**, directly update @@ -191,6 +195,8 @@ export abstract class Field */ SERIALIZABLE = false; + private id_: string | null = null; + /** * @param value The initial value of the field. * Also accepts Field.SKIP_SETUP if you wish to skip setup (only used by @@ -255,6 +261,7 @@ export abstract class Field throw Error('Field already bound to a block'); } this.sourceBlock_ = block; + this.id_ = `${block.id}_field_${idGenerator.getNextUniqueId()}`; } /** @@ -298,7 +305,13 @@ export abstract class Field // Field has already been initialized once. return; } - this.fieldGroup_ = dom.createSvgElement(Svg.G, {}); + const id = this.id_; + if (!id) throw new Error('Expected ID to be defined prior to init.'); + this.fieldGroup_ = dom.createSvgElement(Svg.G, { + 'tabindex': '-1', + 'id': id, + 'aria-label': 'Field ' + this.name, + }); if (!this.isVisible()) { this.fieldGroup_.style.display = 'none'; } @@ -1416,6 +1429,30 @@ export abstract class Field } } + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + if (!this.fieldGroup_) { + throw Error("This field currently has no representative DOM element."); + } + return this.fieldGroup_; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + const block = this.getSourceBlock(); + if (!block) { + throw new UnattachedFieldError(); + } + // TODO: Remove the latter two casts once WorkspaceSvg is a focusable tree. + return block.workspace as WorkspaceSvg as unknown as IFocusableTree; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} + /** * Subclasses should reimplement this method to construct their Field * subclass from a JSON arg object. diff --git a/core/field_checkbox.ts b/core/field_checkbox.ts index 3d32585ad98..ac4cda28df0 100644 --- a/core/field_checkbox.ts +++ b/core/field_checkbox.ts @@ -137,11 +137,11 @@ export class FieldCheckbox extends Field { /** Toggle the state of the checkbox on click. */ protected override showEditor_( - onEditorShown: () => void, onEditorHidden: () => void + _onEditorShown: () => void, _onEditorHidden: () => void ) { - onEditorShown(); + // There's no explicit editor for checkboxes, so let DOM focus be handled + // normally. this.setValue(!this.value_); - onEditorHidden(); } protected override onShowEditor(): void {} diff --git a/core/field_dropdown.ts b/core/field_dropdown.ts index 4afa2dffca7..ca5694fcf2f 100644 --- a/core/field_dropdown.ts +++ b/core/field_dropdown.ts @@ -22,6 +22,7 @@ import { UnattachedFieldError, } from './field.js'; import * as fieldRegistry from './field_registry.js'; +import { getFocusManager } from './focus_manager.js'; import {Menu} from './menu.js'; import {MenuSeparator} from './menu_separator.js'; import {MenuItem} from './menuitem.js'; @@ -88,6 +89,8 @@ export class FieldDropdown extends Field { private selectedOption!: MenuOption; override clickTarget_: SVGElement | null = null; + private returnFocusCallback: (() => void) | null = null; + /** * The y offset from the top of the field to the top of the image, if an image * is selected. @@ -317,9 +320,17 @@ export class FieldDropdown extends Field { this.applyColour(); } - protected override onShowEditor(): void {} + protected override onShowEditor(): void { + const menuElement = this.menu_?.getElement(); + if (menuElement) { + this.returnFocusCallback = + getFocusManager().takeEphemeralFocus(menuElement); + } + } - protected override onHideEditor(): void {} + protected override onHideEditor(): void { + if (this.returnFocusCallback) this.returnFocusCallback(); + } /** Create the dropdown editor. */ private dropdownCreate() { diff --git a/core/field_input.ts b/core/field_input.ts index 615521c41cc..d9851d3fb17 100644 --- a/core/field_input.ts +++ b/core/field_input.ts @@ -35,6 +35,7 @@ import {Size} from './utils/size.js'; import * as userAgent from './utils/useragent.js'; import * as WidgetDiv from './widgetdiv.js'; import type {WorkspaceSvg} from './workspace_svg.js'; +import { getFocusManager } from './focus_manager.js'; /** * Supported types for FieldInput subclasses. @@ -100,6 +101,8 @@ export abstract class FieldInput extends Field< */ override SERIALIZABLE = true; + private returnFocusCallback: (() => void) | null = null; + /** * @param value The initial value of the field. Should cast to a string. * Defaults to an empty string if null or undefined. Also accepts @@ -350,9 +353,14 @@ export abstract class FieldInput extends Field< } } - protected override onShowEditor(): void {} + protected override onShowEditor(): void { + this.returnFocusCallback = + getFocusManager().takeEphemeralFocus(document.body); + } - protected override onHideEditor(): void {} + protected override onHideEditor(): void { + if (this.returnFocusCallback) this.returnFocusCallback(); + } /** * Create and show a text input editor that is a prompt (usually a popup). From 94672d98460f49994e02580bace4233a805cd53c Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 22 Apr 2025 21:32:50 +0000 Subject: [PATCH 06/27] chore: Lint fixes. --- blocks/procedures.ts | 4 +++- core/field.ts | 14 +++++++++---- core/field_checkbox.ts | 3 ++- core/field_dropdown.ts | 6 ++++-- core/field_image.ts | 3 ++- core/field_input.ts | 20 ++++++++++++------- core/field_label.ts | 3 ++- tests/mocha/field_textinput_test.js | 10 ++++++++-- .../src/field/different_user_input.ts | 3 ++- 9 files changed, 46 insertions(+), 20 deletions(-) diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 317eb82190b..656794002b9 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -649,7 +649,9 @@ class ProcedureArgumentField extends FieldTextInput { * @param e The event that triggered display of the field editor. */ protected override showEditor_( - onEditorShown: () => void, onEditorHidden: () => void, e?: Event + onEditorShown: () => void, + onEditorHidden: () => void, + e?: Event, ) { super.showEditor_(onEditorShown, onEditorHidden, e); this.editingInteractively = true; diff --git a/core/field.ts b/core/field.ts index bc091955f76..8b912d237bb 100644 --- a/core/field.ts +++ b/core/field.ts @@ -36,6 +36,7 @@ import type {KeyboardShortcut} from './shortcut_registry.js'; import * as Tooltip from './tooltip.js'; import type {Coordinate} from './utils/coordinate.js'; import * as dom from './utils/dom.js'; +import * as idGenerator from './utils/idgenerator.js'; import * as parsing from './utils/parsing.js'; import {Rect} from './utils/rect.js'; import {Size} from './utils/size.js'; @@ -45,7 +46,6 @@ import * as userAgent from './utils/useragent.js'; import * as utilsXml from './utils/xml.js'; import * as WidgetDiv from './widgetdiv.js'; import {WorkspaceSvg} from './workspace_svg.js'; -import * as idGenerator from './utils/idgenerator.js'; /** * A function that is called to validate changes to the field's value before @@ -783,7 +783,11 @@ export abstract class Field */ showEditor(e?: Event) { if (this.isClickable()) { - this.showEditor_(() => this.onShowEditor(), () => this.onHideEditor(), e); + this.showEditor_( + () => this.onShowEditor(), + () => this.onHideEditor(), + e, + ); } } @@ -797,7 +801,9 @@ export abstract class Field * undefined if triggered programmatically. */ protected abstract showEditor_( - onEditorShown: () => void, onEditorHidden: () => void, e?: Event + onEditorShown: () => void, + onEditorHidden: () => void, + e?: Event, ): void; /** @@ -1432,7 +1438,7 @@ export abstract class Field /** See IFocusableNode.getFocusableElement. */ getFocusableElement(): HTMLElement | SVGElement { if (!this.fieldGroup_) { - throw Error("This field currently has no representative DOM element."); + throw Error('This field currently has no representative DOM element.'); } return this.fieldGroup_; } diff --git a/core/field_checkbox.ts b/core/field_checkbox.ts index ac4cda28df0..1feb65a85c3 100644 --- a/core/field_checkbox.ts +++ b/core/field_checkbox.ts @@ -137,7 +137,8 @@ export class FieldCheckbox extends Field { /** Toggle the state of the checkbox on click. */ protected override showEditor_( - _onEditorShown: () => void, _onEditorHidden: () => void + _onEditorShown: () => void, + _onEditorHidden: () => void, ) { // There's no explicit editor for checkboxes, so let DOM focus be handled // normally. diff --git a/core/field_dropdown.ts b/core/field_dropdown.ts index ca5694fcf2f..616c00ce689 100644 --- a/core/field_dropdown.ts +++ b/core/field_dropdown.ts @@ -22,7 +22,7 @@ import { UnattachedFieldError, } from './field.js'; import * as fieldRegistry from './field_registry.js'; -import { getFocusManager } from './focus_manager.js'; +import {getFocusManager} from './focus_manager.js'; import {Menu} from './menu.js'; import {MenuSeparator} from './menu_separator.js'; import {MenuItem} from './menuitem.js'; @@ -273,7 +273,9 @@ export class FieldDropdown extends Field { * undefined if triggered programmatically. */ protected override showEditor_( - onEditorShown: () => void, onEditorHidden: () => void, e?: MouseEvent + onEditorShown: () => void, + onEditorHidden: () => void, + e?: MouseEvent, ) { const block = this.getSourceBlock(); if (!block) { diff --git a/core/field_image.ts b/core/field_image.ts index 9b809e4058b..f83fdeb5b27 100644 --- a/core/field_image.ts +++ b/core/field_image.ts @@ -218,7 +218,8 @@ export class FieldImage extends Field { * call the handler. */ protected override showEditor_( - _onEditorShown: () => void, _onEditorHidden: () => void + _onEditorShown: () => void, + _onEditorHidden: () => void, ) { // Note that an editor shouldn't be shown for this field, so the callbacks // are ignored. diff --git a/core/field_input.ts b/core/field_input.ts index d9851d3fb17..2d8be8c089a 100644 --- a/core/field_input.ts +++ b/core/field_input.ts @@ -27,6 +27,7 @@ import { FieldValidator, UnattachedFieldError, } from './field.js'; +import {getFocusManager} from './focus_manager.js'; import {Msg} from './msg.js'; import * as renderManagement from './render_management.js'; import * as aria from './utils/aria.js'; @@ -35,7 +36,6 @@ import {Size} from './utils/size.js'; import * as userAgent from './utils/useragent.js'; import * as WidgetDiv from './widgetdiv.js'; import type {WorkspaceSvg} from './workspace_svg.js'; -import { getFocusManager } from './focus_manager.js'; /** * Supported types for FieldInput subclasses. @@ -338,8 +338,10 @@ export abstract class FieldInput extends Field< * Defaults to false. */ protected override showEditor_( - onEditorShown: () => void, onEditorHidden: () => void, _e?: Event, - quietInput: boolean = false + onEditorShown: () => void, + onEditorHidden: () => void, + _e?: Event, + quietInput: boolean = false, ) { this.workspace_ = (this.sourceBlock_ as BlockSvg).workspace; if ( @@ -354,8 +356,9 @@ export abstract class FieldInput extends Field< } protected override onShowEditor(): void { - this.returnFocusCallback = - getFocusManager().takeEphemeralFocus(document.body); + this.returnFocusCallback = getFocusManager().takeEphemeralFocus( + document.body, + ); } protected override onHideEditor(): void { @@ -368,7 +371,8 @@ export abstract class FieldInput extends Field< * keyboards). */ private showPromptEditor( - onEditorShown: () => void, onEditorHidden: () => void + onEditorShown: () => void, + onEditorHidden: () => void, ) { onEditorShown(); dialog.prompt( @@ -391,7 +395,9 @@ export abstract class FieldInput extends Field< * @param quietInput True if editor should be created without focus. */ private showInlineEditor( - onEditorShown: () => void, onEditorHidden: () => void, quietInput: boolean + onEditorShown: () => void, + onEditorHidden: () => void, + quietInput: boolean, ) { const block = this.getSourceBlock(); if (!block) { diff --git a/core/field_label.ts b/core/field_label.ts index d108ac499fe..b0dd6af54d2 100644 --- a/core/field_label.ts +++ b/core/field_label.ts @@ -95,7 +95,8 @@ export class FieldLabel extends Field { } protected override showEditor_( - _onEditorShown: () => void, _onEditorHidden: () => void + _onEditorShown: () => void, + _onEditorHidden: () => void, ) { // Note that an editor shouldn't be shown for this field, so the callbacks // are ignored. diff --git a/tests/mocha/field_textinput_test.js b/tests/mocha/field_textinput_test.js index 4ab40d4f506..20f05432d82 100644 --- a/tests/mocha/field_textinput_test.js +++ b/tests/mocha/field_textinput_test.js @@ -228,7 +228,10 @@ suite('Text Input Fields', function () { this.assertSpellcheck = function (field, value) { this.prepField(field); - field.showEditor_(() => {}, () => {}); + field.showEditor_( + () => {}, + () => {}, + ); assert.equal( field.htmlInput_.getAttribute('spellcheck'), value.toString(), @@ -265,7 +268,10 @@ suite('Text Input Fields', function () { test('setSpellcheck Editor Shown', function () { const field = new Blockly.FieldTextInput('test'); this.prepField(field); - field.showEditor_(() => {}, () => {}); + field.showEditor_( + () => {}, + () => {}, + ); field.setSpellcheck(false); assert.equal(field.htmlInput_.getAttribute('spellcheck'), 'false'); }); diff --git a/tests/typescript/src/field/different_user_input.ts b/tests/typescript/src/field/different_user_input.ts index afc150da48d..60cb8e00aa6 100644 --- a/tests/typescript/src/field/different_user_input.ts +++ b/tests/typescript/src/field/different_user_input.ts @@ -63,7 +63,8 @@ class FieldMitosis extends Field { } protected override showEditor_( - _onEditorShown: () => void, _onEditorHidden: () => void + _onEditorShown: () => void, + _onEditorHidden: () => void, ) { // Note that an editor shouldn't be shown for this field, so the callbacks // are ignored. From 243064676c05d38a2adc5e4da401079ff22d0676 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 22 Apr 2025 21:36:05 +0000 Subject: [PATCH 07/27] chore: Remove incorrect aria-label. --- core/field.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/core/field.ts b/core/field.ts index 8b912d237bb..7d3a5e7bf68 100644 --- a/core/field.ts +++ b/core/field.ts @@ -310,7 +310,6 @@ export abstract class Field this.fieldGroup_ = dom.createSvgElement(Svg.G, { 'tabindex': '-1', 'id': id, - 'aria-label': 'Field ' + this.name, }); if (!this.isVisible()) { this.fieldGroup_.style.display = 'none'; From 263773610804e3212a922d7d2a515a104d6fc7d6 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 23 Apr 2025 21:43:32 +0000 Subject: [PATCH 08/27] fix: Ensure Block paths are focusable. --- core/renderers/common/path_object.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index 077f80bb741..72cf2a594ce 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -62,7 +62,7 @@ export class PathObject implements IPathObject { /** The primary path of the block. */ this.svgPath = dom.createSvgElement( Svg.PATH, - {'class': 'blocklyPath'}, + {'class': 'blocklyPath', 'tabindex': '-1'}, this.svgRoot, ); From 49192ba4823c8ac466a136e6319a8b4364dc26cc Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 23 Apr 2025 22:25:53 +0000 Subject: [PATCH 09/27] chore: Fix line comment. Addresses review comment. --- core/workspace_svg.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 6d6505d0545..b1c96373771 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -771,7 +771,7 @@ export class WorkspaceSvg */ this.svgGroup_ = dom.createSvgElement(Svg.G, { 'class': 'blocklyWorkspace', - // Only the main workspace should be tabbable. + // Only the top-level workspace should be tabbable. 'tabindex': injectionDiv ? '0' : '-1', 'id': this.id, }); From d276dbcc2edfc39ea1fbc03f700fc96a2583bc2d Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 19:08:22 +0000 Subject: [PATCH 10/27] chore: reduce branching. Addresses reviewer comment. --- core/toolbox/toolbox.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/toolbox/toolbox.ts b/core/toolbox/toolbox.ts index f44ebac5348..03442e13795 100644 --- a/core/toolbox/toolbox.ts +++ b/core/toolbox/toolbox.ts @@ -1120,7 +1120,8 @@ export class Toolbox // root of the toolbox. if (!previousNode || previousNode === this) { return this.getToolboxItems().find((item) => item.isSelectable()) ?? null; - } else return null; + } + return null; } /** See IFocusableTree.getNestedTrees. */ From 90fdde21fdd51e88a90b5ed83fd40ad2bcc0f0fd Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 19:16:23 +0000 Subject: [PATCH 11/27] feat: make drop down & widget divs focusable. This is a more general purpose alternative to making fields explicitly focusable (at least making them hook up properly to ephemeral focus). --- core/dropdowndiv.ts | 14 ++++++++++++++ core/widgetdiv.ts | 15 +++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/core/dropdowndiv.ts b/core/dropdowndiv.ts index 0d259bc53d7..6d61354adf7 100644 --- a/core/dropdowndiv.ts +++ b/core/dropdowndiv.ts @@ -15,6 +15,7 @@ import type {BlockSvg} from './block_svg.js'; import * as common from './common.js'; import type {Field} from './field.js'; +import { ReturnEphemeralFocus, getFocusManager } from './focus_manager.js'; import * as dom from './utils/dom.js'; import * as math from './utils/math.js'; import {Rect} from './utils/rect.js'; @@ -82,6 +83,9 @@ let owner: Field | null = null; /** Whether the dropdown was positioned to a field or the source block. */ let positionToField: boolean | null = null; +/** Callback to FocusManager to return ephemeral focus when the div closes. */ +let returnEphemeralFocus: ReturnEphemeralFocus | null = null; + /** * Dropdown bounds info object used to encapsulate sizing information about a * bounding element (bounding box and width/height). @@ -338,6 +342,8 @@ export function show( dom.addClass(div, renderedClassName); dom.addClass(div, themeClassName); + returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div); + // When we change `translate` multiple times in close succession, // Chrome may choose to wait and apply them all at once. // Since we want the translation to initial X, Y to be immediate, @@ -623,6 +629,10 @@ export function hide() { animateOutTimer = setTimeout(function () { hideWithoutAnimation(); }, ANIMATION_TIME * 1000); + if (returnEphemeralFocus) { + returnEphemeralFocus(); + returnEphemeralFocus = null; + } if (onHide) { onHide(); onHide = null; @@ -638,6 +648,10 @@ export function hideWithoutAnimation() { clearTimeout(animateOutTimer); } + if (returnEphemeralFocus) { + returnEphemeralFocus(); + returnEphemeralFocus = null; + } if (onHide) { onHide(); onHide = null; diff --git a/core/widgetdiv.ts b/core/widgetdiv.ts index f167b6cf04d..3d2e2e9d9a7 100644 --- a/core/widgetdiv.ts +++ b/core/widgetdiv.ts @@ -8,6 +8,7 @@ import * as common from './common.js'; import {Field} from './field.js'; +import { ReturnEphemeralFocus, getFocusManager } from './focus_manager.js'; import * as dom from './utils/dom.js'; import type {Rect} from './utils/rect.js'; import type {Size} from './utils/size.js'; @@ -34,6 +35,9 @@ let themeClassName = ''; /** The HTML container for popup overlays (e.g. editor widgets). */ let containerDiv: HTMLDivElement | null; +/** Callback to FocusManager to return ephemeral focus when the div closes. */ +let returnEphemeralFocus: ReturnEphemeralFocus | null = null; + /** * Returns the HTML container for editor widgets. * @@ -110,6 +114,7 @@ export function show( if (themeClassName) { dom.addClass(div, themeClassName); } + returnEphemeralFocus = getFocusManager().takeEphemeralFocus(div); } /** @@ -126,8 +131,14 @@ export function hide() { div.style.display = 'none'; div.style.left = ''; div.style.top = ''; - if (dispose) dispose(); - dispose = null; + if (returnEphemeralFocus) { + returnEphemeralFocus(); + returnEphemeralFocus = null; + } + if (dispose) { + dispose(); + dispose = null; + } div.textContent = ''; if (rendererClassName) { From 7c2f705be956d4c7fc34206e4fa2e5021de5c671 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 19:31:13 +0000 Subject: [PATCH 12/27] chore: undo breaking field changes. With widget and drop down divs focusable directly, all field editors should automatically inherit this benefit. --- blocks/procedures.ts | 7 +-- core/dialog.ts | 7 +++ core/field.ts | 29 ++---------- core/field_checkbox.ts | 9 +--- core/field_dropdown.ts | 26 +---------- core/field_image.ts | 11 +---- core/field_input.ts | 44 +++---------------- core/field_label.ts | 12 ----- tests/mocha/field_textinput_test.js | 10 +---- .../src/field/different_user_input.ts | 12 ----- 10 files changed, 24 insertions(+), 143 deletions(-) diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 656794002b9..7a0899eacb0 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -648,12 +648,7 @@ class ProcedureArgumentField extends FieldTextInput { * * @param e The event that triggered display of the field editor. */ - protected override showEditor_( - onEditorShown: () => void, - onEditorHidden: () => void, - e?: Event, - ) { - super.showEditor_(onEditorShown, onEditorHidden, e); + protected override showEditor_(e?: Event) { this.editingInteractively = true; this.editingVariable = undefined; } diff --git a/core/dialog.ts b/core/dialog.ts index 374961323da..96631e9cbc7 100644 --- a/core/dialog.ts +++ b/core/dialog.ts @@ -33,6 +33,8 @@ const defaultPrompt = function ( defaultValue: string, callback: (result: string | null) => void, ) { + // NOTE TO DEVELOPER: Ephemeral focus doesn't need to be taken for the native + // window prompt since it prevents focus from changing while open. callback(window.prompt(message, defaultValue)); }; @@ -116,6 +118,11 @@ export function prompt( /** * Sets the function to be run when Blockly.dialog.prompt() is called. * + * **Important**: When overridding this, be aware that non-native prompt + * experiences may require managing ephemeral focus in FocusManager. This isn't + * needed for the native window prompt because it prevents focus from being + * changed while open. + * * @param promptFunction The function to be run, or undefined to restore the * default implementation. * @see Blockly.dialog.prompt diff --git a/core/field.ts b/core/field.ts index 7d3a5e7bf68..df166fbe778 100644 --- a/core/field.ts +++ b/core/field.ts @@ -782,11 +782,7 @@ export abstract class Field */ showEditor(e?: Event) { if (this.isClickable()) { - this.showEditor_( - () => this.onShowEditor(), - () => this.onHideEditor(), - e, - ); + this.showEditor_(e); } } @@ -794,28 +790,11 @@ export abstract class Field * A developer hook to create an editor for the field. This is no-op by * default, and must be overriden to create an editor. * - * @param onEditorShown Callback that must be called when the editor is shown. - * @param onEditorHidden Callback that must be called when the editor hides. - * @param e Optional mouse event that triggered the field to open, or + * @param _e Optional mouse event that triggered the field to open, or * undefined if triggered programmatically. */ - protected abstract showEditor_( - onEditorShown: () => void, - onEditorHidden: () => void, - e?: Event, - ): void; - - /** - * Called when an editor is shown. This is expected to be used for ensuring - * that the editor has proper focus. - */ - protected abstract onShowEditor(): void; - - /** - * Called when an editor is hidden. This is expected to be used for ensuring - * that the editor no longer proper focus. - */ - protected abstract onHideEditor(): void; + protected showEditor_(_e?: Event): void {} + // NOP /** * A developer hook to reposition the WidgetDiv during a window resize. You diff --git a/core/field_checkbox.ts b/core/field_checkbox.ts index 1feb65a85c3..c8307d1328a 100644 --- a/core/field_checkbox.ts +++ b/core/field_checkbox.ts @@ -136,19 +136,12 @@ export class FieldCheckbox extends Field { } /** Toggle the state of the checkbox on click. */ - protected override showEditor_( - _onEditorShown: () => void, - _onEditorHidden: () => void, - ) { + protected override showEditor_() { // There's no explicit editor for checkboxes, so let DOM focus be handled // normally. this.setValue(!this.value_); } - protected override onShowEditor(): void {} - - protected override onHideEditor(): void {} - /** * Ensure that the input value is valid ('TRUE' or 'FALSE'). * diff --git a/core/field_dropdown.ts b/core/field_dropdown.ts index 616c00ce689..ab9e4e060d6 100644 --- a/core/field_dropdown.ts +++ b/core/field_dropdown.ts @@ -89,8 +89,6 @@ export class FieldDropdown extends Field { private selectedOption!: MenuOption; override clickTarget_: SVGElement | null = null; - private returnFocusCallback: (() => void) | null = null; - /** * The y offset from the top of the field to the top of the image, if an image * is selected. @@ -272,11 +270,7 @@ export class FieldDropdown extends Field { * @param e Optional mouse event that triggered the field to open, or * undefined if triggered programmatically. */ - protected override showEditor_( - onEditorShown: () => void, - onEditorHidden: () => void, - e?: MouseEvent, - ) { + protected override showEditor_(e?: MouseEvent) { const block = this.getSourceBlock(); if (!block) { throw new UnattachedFieldError(); @@ -284,7 +278,6 @@ export class FieldDropdown extends Field { this.dropdownCreate(); if (!this.menu_) return; - onEditorShown(); if (e && typeof e.clientX === 'number') { this.menu_.openingCoords = new Coordinate(e.clientX, e.clientY); } else { @@ -303,10 +296,7 @@ export class FieldDropdown extends Field { dropDownDiv.setColour(primaryColour, borderColour); } - dropDownDiv.showPositionedByField(this, () => { - this.dropdownDispose_.bind(this); - onEditorHidden(); - }); + dropDownDiv.showPositionedByField(this, this.dropdownDispose_.bind(this)); dropDownDiv.getContentDiv().style.height = `${this.menu_.getSize().height}px`; @@ -322,18 +312,6 @@ export class FieldDropdown extends Field { this.applyColour(); } - protected override onShowEditor(): void { - const menuElement = this.menu_?.getElement(); - if (menuElement) { - this.returnFocusCallback = - getFocusManager().takeEphemeralFocus(menuElement); - } - } - - protected override onHideEditor(): void { - if (this.returnFocusCallback) this.returnFocusCallback(); - } - /** Create the dropdown editor. */ private dropdownCreate() { const block = this.getSourceBlock(); diff --git a/core/field_image.ts b/core/field_image.ts index f83fdeb5b27..650575f59a3 100644 --- a/core/field_image.ts +++ b/core/field_image.ts @@ -217,21 +217,12 @@ export class FieldImage extends Field { * If field click is called, and click handler defined, * call the handler. */ - protected override showEditor_( - _onEditorShown: () => void, - _onEditorHidden: () => void, - ) { - // Note that an editor shouldn't be shown for this field, so the callbacks - // are ignored. + protected override showEditor_() { if (this.clickHandler) { this.clickHandler(this); } } - protected override onShowEditor(): void {} - - protected override onHideEditor(): void {} - /** * Set the function that is called when this image is clicked. * diff --git a/core/field_input.ts b/core/field_input.ts index 2d8be8c089a..7bf19e217ce 100644 --- a/core/field_input.ts +++ b/core/field_input.ts @@ -101,8 +101,6 @@ export abstract class FieldInput extends Field< */ override SERIALIZABLE = true; - private returnFocusCallback: (() => void) | null = null; - /** * @param value The initial value of the field. Should cast to a string. * Defaults to an empty string if null or undefined. Also accepts @@ -330,51 +328,30 @@ export abstract class FieldInput extends Field< * Shows a prompt editor for mobile browsers if the modalInputs option is * enabled. * - * @param onEditorShown Callback that must be called when the editor is shown. - * @param onEditorHidden Callback that must be called when the editor hides. * @param _e Optional mouse event that triggered the field to open, or * undefined if triggered programmatically. * @param quietInput True if editor should be created without focus. * Defaults to false. */ - protected override showEditor_( - onEditorShown: () => void, - onEditorHidden: () => void, - _e?: Event, - quietInput: boolean = false, - ) { + protected override showEditor_(_e?: Event, quietInput: boolean = false) { this.workspace_ = (this.sourceBlock_ as BlockSvg).workspace; if ( !quietInput && this.workspace_.options.modalInputs && (userAgent.MOBILE || userAgent.ANDROID || userAgent.IPAD) ) { - this.showPromptEditor(onEditorShown, onEditorHidden); + this.showPromptEditor(); } else { - this.showInlineEditor(onEditorShown, onEditorHidden, quietInput); + this.showInlineEditor(quietInput); } } - protected override onShowEditor(): void { - this.returnFocusCallback = getFocusManager().takeEphemeralFocus( - document.body, - ); - } - - protected override onHideEditor(): void { - if (this.returnFocusCallback) this.returnFocusCallback(); - } - /** * Create and show a text input editor that is a prompt (usually a popup). * Mobile browsers may have issues with in-line textareas (focus and * keyboards). */ - private showPromptEditor( - onEditorShown: () => void, - onEditorHidden: () => void, - ) { - onEditorShown(); + private showPromptEditor() { dialog.prompt( Msg['CHANGE_VALUE_TITLE'], this.getText(), @@ -384,7 +361,6 @@ export abstract class FieldInput extends Field< this.setValue(this.getValueFromEditorText_(text)); } this.onFinishEditing_(this.value_); - onEditorHidden(); }, ); } @@ -394,11 +370,7 @@ export abstract class FieldInput extends Field< * * @param quietInput True if editor should be created without focus. */ - private showInlineEditor( - onEditorShown: () => void, - onEditorHidden: () => void, - quietInput: boolean, - ) { + private showInlineEditor(quietInput: boolean) { const block = this.getSourceBlock(); if (!block) { throw new UnattachedFieldError(); @@ -406,13 +378,9 @@ export abstract class FieldInput extends Field< WidgetDiv.show( this, block.RTL, - () => { - this.widgetDispose_(); - onEditorHidden(); - }, + this.widgetDispose_.bind(this), this.workspace_, ); - onEditorShown(); this.htmlInput_ = this.widgetCreate_() as HTMLInputElement; this.isBeingEdited_ = true; this.valueWhenEditorWasOpened_ = this.value_; diff --git a/core/field_label.ts b/core/field_label.ts index b0dd6af54d2..236154cc7b1 100644 --- a/core/field_label.ts +++ b/core/field_label.ts @@ -94,18 +94,6 @@ export class FieldLabel extends Field { return `${newValue}`; } - protected override showEditor_( - _onEditorShown: () => void, - _onEditorHidden: () => void, - ) { - // Note that an editor shouldn't be shown for this field, so the callbacks - // are ignored. - } - - protected override onShowEditor(): void {} - - protected override onHideEditor(): void {} - /** * Set the CSS class applied to the field's textElement_. * diff --git a/tests/mocha/field_textinput_test.js b/tests/mocha/field_textinput_test.js index 20f05432d82..93b8df89fd0 100644 --- a/tests/mocha/field_textinput_test.js +++ b/tests/mocha/field_textinput_test.js @@ -228,10 +228,7 @@ suite('Text Input Fields', function () { this.assertSpellcheck = function (field, value) { this.prepField(field); - field.showEditor_( - () => {}, - () => {}, - ); + field.showEditor_(); assert.equal( field.htmlInput_.getAttribute('spellcheck'), value.toString(), @@ -268,10 +265,7 @@ suite('Text Input Fields', function () { test('setSpellcheck Editor Shown', function () { const field = new Blockly.FieldTextInput('test'); this.prepField(field); - field.showEditor_( - () => {}, - () => {}, - ); + field.showEditor_(; field.setSpellcheck(false); assert.equal(field.htmlInput_.getAttribute('spellcheck'), 'false'); }); diff --git a/tests/typescript/src/field/different_user_input.ts b/tests/typescript/src/field/different_user_input.ts index 60cb8e00aa6..f91f25bbc86 100644 --- a/tests/typescript/src/field/different_user_input.ts +++ b/tests/typescript/src/field/different_user_input.ts @@ -61,18 +61,6 @@ class FieldMitosis extends Field { }); this.value_ = {cells}; } - - protected override showEditor_( - _onEditorShown: () => void, - _onEditorHidden: () => void, - ) { - // Note that an editor shouldn't be shown for this field, so the callbacks - // are ignored. - } - - protected override onShowEditor(): void {} - - protected override onHideEditor(): void {} } fieldRegistry.register('field_mitosis', FieldMitosis); From 97263894d988d645204b5f470123292f1156d6c0 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 19:36:22 +0000 Subject: [PATCH 13/27] chore: some more clean-ups after removals. --- blocks/procedures.ts | 1 + core/field_checkbox.ts | 2 -- core/field_dropdown.ts | 5 ++--- core/field_input.ts | 3 +-- tests/mocha/field_textinput_test.js | 2 +- 5 files changed, 5 insertions(+), 8 deletions(-) diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 7a0899eacb0..7284973d9cc 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -649,6 +649,7 @@ class ProcedureArgumentField extends FieldTextInput { * @param e The event that triggered display of the field editor. */ protected override showEditor_(e?: Event) { + super.showEditor_(e); this.editingInteractively = true; this.editingVariable = undefined; } diff --git a/core/field_checkbox.ts b/core/field_checkbox.ts index c8307d1328a..55ed42cbf4b 100644 --- a/core/field_checkbox.ts +++ b/core/field_checkbox.ts @@ -137,8 +137,6 @@ export class FieldCheckbox extends Field { /** Toggle the state of the checkbox on click. */ protected override showEditor_() { - // There's no explicit editor for checkboxes, so let DOM focus be handled - // normally. this.setValue(!this.value_); } diff --git a/core/field_dropdown.ts b/core/field_dropdown.ts index ab9e4e060d6..81279e2a1f5 100644 --- a/core/field_dropdown.ts +++ b/core/field_dropdown.ts @@ -22,7 +22,6 @@ import { UnattachedFieldError, } from './field.js'; import * as fieldRegistry from './field_registry.js'; -import {getFocusManager} from './focus_manager.js'; import {Menu} from './menu.js'; import {MenuSeparator} from './menu_separator.js'; import {MenuItem} from './menuitem.js'; @@ -770,7 +769,7 @@ export class FieldDropdown extends Field { } else if (typeof option[1] !== 'string') { foundError = true; console.error( - `Invalid option[${i}]: Each FieldDropdown option id must be a string. + `Invalid option[${i}]: Each FieldDropdown option id must be a string. Found ${option[1]} in: ${option}`, ); } else if ( @@ -781,7 +780,7 @@ export class FieldDropdown extends Field { ) { foundError = true; console.error( - `Invalid option[${i}]: Each FieldDropdown option must have a string + `Invalid option[${i}]: Each FieldDropdown option must have a string label, image description, or HTML element. Found ${option[0]} in: ${option}`, ); } diff --git a/core/field_input.ts b/core/field_input.ts index 7bf19e217ce..2cdd8056553 100644 --- a/core/field_input.ts +++ b/core/field_input.ts @@ -27,7 +27,6 @@ import { FieldValidator, UnattachedFieldError, } from './field.js'; -import {getFocusManager} from './focus_manager.js'; import {Msg} from './msg.js'; import * as renderManagement from './render_management.js'; import * as aria from './utils/aria.js'; @@ -333,7 +332,7 @@ export abstract class FieldInput extends Field< * @param quietInput True if editor should be created without focus. * Defaults to false. */ - protected override showEditor_(_e?: Event, quietInput: boolean = false) { + protected override showEditor_(_e?: Event, quietInput = false) { this.workspace_ = (this.sourceBlock_ as BlockSvg).workspace; if ( !quietInput && diff --git a/tests/mocha/field_textinput_test.js b/tests/mocha/field_textinput_test.js index 93b8df89fd0..82c1a645e6d 100644 --- a/tests/mocha/field_textinput_test.js +++ b/tests/mocha/field_textinput_test.js @@ -265,7 +265,7 @@ suite('Text Input Fields', function () { test('setSpellcheck Editor Shown', function () { const field = new Blockly.FieldTextInput('test'); this.prepField(field); - field.showEditor_(; + field.showEditor_(); field.setSpellcheck(false); assert.equal(field.htmlInput_.getAttribute('spellcheck'), 'false'); }); From 1094787a802b7d69cb8965121a875b1cbeedd804 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 20:27:29 +0000 Subject: [PATCH 14/27] feat: fix field node retrieval. Also, remove unnecessary casting. --- core/field.ts | 3 +-- core/workspace_svg.ts | 11 +++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/core/field.ts b/core/field.ts index df166fbe778..1da60f18b81 100644 --- a/core/field.ts +++ b/core/field.ts @@ -1427,8 +1427,7 @@ export abstract class Field if (!block) { throw new UnattachedFieldError(); } - // TODO: Remove the latter two casts once WorkspaceSvg is a focusable tree. - return block.workspace as WorkspaceSvg as unknown as IFocusableTree; + return block.workspace as WorkspaceSvg; } /** See IFocusableNode.onNodeFocus. */ diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 250d6cf43e2..b5a5b245a0a 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -2680,6 +2680,17 @@ export class WorkspaceSvg /** See IFocusableTree.lookUpFocusableNode. */ lookUpFocusableNode(id: string): IFocusableNode | null { + const fieldIndicatorIndex = id.indexOf('_field_'); + if (fieldIndicatorIndex !== -1) { + const blockId = id.substring(0, fieldIndicatorIndex); + const block = this.getBlockById(blockId); + if (block) { + for (const field of block.getFields()) { + if (field.getFocusableElement().id === id) return field; + } + } + return null; + } return this.getBlockById(id) as IFocusableNode; } From 082a6efd4b0a02cce8972fea99cfcc0045a34628 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 20:29:57 +0000 Subject: [PATCH 15/27] chore: lint fixes. --- core/dropdowndiv.ts | 2 +- core/widgetdiv.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/dropdowndiv.ts b/core/dropdowndiv.ts index 6d61354adf7..dcf8fa24ef7 100644 --- a/core/dropdowndiv.ts +++ b/core/dropdowndiv.ts @@ -15,7 +15,7 @@ import type {BlockSvg} from './block_svg.js'; import * as common from './common.js'; import type {Field} from './field.js'; -import { ReturnEphemeralFocus, getFocusManager } from './focus_manager.js'; +import {ReturnEphemeralFocus, getFocusManager} from './focus_manager.js'; import * as dom from './utils/dom.js'; import * as math from './utils/math.js'; import {Rect} from './utils/rect.js'; diff --git a/core/widgetdiv.ts b/core/widgetdiv.ts index 3d2e2e9d9a7..cb006160455 100644 --- a/core/widgetdiv.ts +++ b/core/widgetdiv.ts @@ -8,7 +8,7 @@ import * as common from './common.js'; import {Field} from './field.js'; -import { ReturnEphemeralFocus, getFocusManager } from './focus_manager.js'; +import {ReturnEphemeralFocus, getFocusManager} from './focus_manager.js'; import * as dom from './utils/dom.js'; import type {Rect} from './utils/rect.js'; import type {Size} from './utils/size.js'; From a346a920a900abd35da979391b88d868916e12f6 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 21:25:16 +0000 Subject: [PATCH 16/27] fix: remove unnecessary shadow check. --- core/block_svg.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/block_svg.ts b/core/block_svg.ts index 7b11e142148..8bc0b7af3f6 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -1836,9 +1836,7 @@ export class BlockSvg /** See IFocusableNode.onNodeFocus. */ onNodeFocus(): void { - if (!this.isShadow()) { - common.setSelected(this); - } + common.setSelected(this); } /** See IFocusableNode.onNodeBlur. */ From 80c8859de8a6af30566132f8e9189eacbfd38c12 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 21:33:56 +0000 Subject: [PATCH 17/27] chore: add braces. Addresses a reviewer comment. --- core/toolbox/toolbox.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/toolbox/toolbox.ts b/core/toolbox/toolbox.ts index 03442e13795..ceb756afbd6 100644 --- a/core/toolbox/toolbox.ts +++ b/core/toolbox/toolbox.ts @@ -1144,7 +1144,9 @@ export class Toolbox if (this.getSelectedItem() !== node) { this.setSelectedItem(node as IToolboxItem); } - } else this.clearSelection(); + } else { + this.clearSelection(); + } } /** See IFocusableTree.onTreeBlur. */ From c2384c61ced7b3b770630f869f9453d78f847965 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 21:51:26 +0000 Subject: [PATCH 18/27] chore: empty commit to make CI pass. Not really necessary, but I like seeing green CI before merging. From c75aea7a4c6cddb4bfdd7258b6cd5fb56a7eb408 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 14:48:16 -0700 Subject: [PATCH 19/27] feat: Make WorkspaceSvg and BlockSvg focusable (#8916) ## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #8913 Fixes #8914 Fixes part of #8771 ### Proposed Changes This updates `WorkspaceSvg` and `BlockSvg` to be focusable, that is, it makes the workspace a `IFocusableTree` and blocks `IFocusableNode`s. Some important details: - While this introduces focusable tree support for `Workspace` it doesn't include two other components that are obviously needed by the keyboard navigation plugin's playground: fields and connections. These will be introduced in subsequent PRs. - Blocks are set up to automatically synchronize their selection state with their focus state. This will eventually help to replace `LineCursor`'s responsibility for managing selection state itself. - The tabindex property for the workspace and its ARIA label have been moved down to the `.blocklyWorkspace` element itself rather than its wrapper. This helps address some tab stop issues that are already addressed in the plugin (via monkey patches), but also to ensure that the workspace's main SVG group interacts correctly with `FocusManager`. - `WorkspaceSvg` is being initially set up to default to its first top block when being focused for the first time. This is to match parity with the keyboard navigation plugin, however the latter also has functionality for defaulting to a position when no blocks are present. It's not clear how to actually support this under the new focus-based system (without adding an ephemeral element on which to focus), or if it's even necessary (since the workspace root can hold focus). ### Reason for Changes This is part of an ongoing effort to ensure key components of Blockly are focusable so that they can be keyboard-navigable (with other needed changes yet both in Core Blockly and the keyboard navigation plugin). ### Test Coverage No new tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915. ### Documentation No documentation changes should be needed here. ### Additional Information This includes changes that have been pulled from #8875. --- core/block_svg.ts | 28 +++++++++- core/inject.ts | 5 -- core/renderers/common/path_object.ts | 2 +- core/workspace_svg.ts | 77 +++++++++++++++++++++++++++- 4 files changed, 103 insertions(+), 9 deletions(-) diff --git a/core/block_svg.ts b/core/block_svg.ts index 4cef79df8e6..04b7d88d1f2 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -44,6 +44,8 @@ import {IContextMenu} from './interfaces/i_contextmenu.js'; import type {ICopyable} from './interfaces/i_copyable.js'; import {IDeletable} from './interfaces/i_deletable.js'; import type {IDragStrategy, IDraggable} from './interfaces/i_draggable.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import {IIcon} from './interfaces/i_icon.js'; import * as internalConstants from './internal_constants.js'; import {MarkerManager} from './marker_manager.js'; @@ -76,7 +78,8 @@ export class BlockSvg IContextMenu, ICopyable, IDraggable, - IDeletable + IDeletable, + IFocusableNode { /** * Constant for identifying rows that are to be rendered inline. @@ -210,6 +213,7 @@ export class BlockSvg // Expose this block's ID on its top-level SVG group. this.svgGroup.setAttribute('data-id', this.id); + svgPath.id = this.id; this.doInit_(); } @@ -1827,4 +1831,26 @@ export class BlockSvg ); } } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + return this.pathObject.svgPath; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.workspace; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void { + common.setSelected(this); + } + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void { + if (common.getSelected() === this) { + common.setSelected(null); + } + } } diff --git a/core/inject.ts b/core/inject.ts index de78fbfae75..34d9c1795f8 100644 --- a/core/inject.ts +++ b/core/inject.ts @@ -13,13 +13,11 @@ import * as common from './common.js'; import * as Css from './css.js'; import * as dropDownDiv from './dropdowndiv.js'; import {Grid} from './grid.js'; -import {Msg} from './msg.js'; import {Options} from './options.js'; import {ScrollbarPair} from './scrollbar_pair.js'; import {ShortcutRegistry} from './shortcut_registry.js'; import * as Tooltip from './tooltip.js'; import * as Touch from './touch.js'; -import * as aria from './utils/aria.js'; import * as dom from './utils/dom.js'; import {Svg} from './utils/svg.js'; import * as WidgetDiv from './widgetdiv.js'; @@ -56,8 +54,6 @@ export function inject( if (opt_options?.rtl) { dom.addClass(subContainer, 'blocklyRTL'); } - subContainer.tabIndex = 0; - aria.setState(subContainer, aria.State.LABEL, Msg['WORKSPACE_ARIA_LABEL']); containerElement!.appendChild(subContainer); const svg = createDom(subContainer, options); @@ -126,7 +122,6 @@ function createDom(container: HTMLElement, options: Options): SVGElement { 'xmlns:xlink': dom.XLINK_NS, 'version': '1.1', 'class': 'blocklySvg', - 'tabindex': '0', }, container, ); diff --git a/core/renderers/common/path_object.ts b/core/renderers/common/path_object.ts index 077f80bb741..72cf2a594ce 100644 --- a/core/renderers/common/path_object.ts +++ b/core/renderers/common/path_object.ts @@ -62,7 +62,7 @@ export class PathObject implements IPathObject { /** The primary path of the block. */ this.svgPath = dom.createSvgElement( Svg.PATH, - {'class': 'blocklyPath'}, + {'class': 'blocklyPath', 'tabindex': '-1'}, this.svgRoot, ); diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index e8d411651c5..2648005b257 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -37,6 +37,7 @@ import {EventType} from './events/type.js'; import * as eventUtils from './events/utils.js'; import {Flyout} from './flyout_base.js'; import type {FlyoutButton} from './flyout_button.js'; +import {getFocusManager} from './focus_manager.js'; import {Gesture} from './gesture.js'; import {Grid} from './grid.js'; import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js'; @@ -44,6 +45,8 @@ import type {IBoundedElement} from './interfaces/i_bounded_element.js'; import {IContextMenu} from './interfaces/i_contextmenu.js'; import type {IDragTarget} from './interfaces/i_drag_target.js'; import type {IFlyout} from './interfaces/i_flyout.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {IMetricsManager} from './interfaces/i_metrics_manager.js'; import type {IToolbox} from './interfaces/i_toolbox.js'; import type { @@ -54,6 +57,7 @@ import type {LineCursor} from './keyboard_nav/line_cursor.js'; import type {Marker} from './keyboard_nav/marker.js'; import {LayerManager} from './layer_manager.js'; import {MarkerManager} from './marker_manager.js'; +import {Msg} from './msg.js'; import {Options} from './options.js'; import * as Procedures from './procedures.js'; import * as registry from './registry.js'; @@ -66,6 +70,7 @@ import {Classic} from './theme/classic.js'; import {ThemeManager} from './theme_manager.js'; import * as Tooltip from './tooltip.js'; import type {Trashcan} from './trashcan.js'; +import * as aria from './utils/aria.js'; import * as arrayUtils from './utils/array.js'; import {Coordinate} from './utils/coordinate.js'; import * as dom from './utils/dom.js'; @@ -93,7 +98,7 @@ const ZOOM_TO_FIT_MARGIN = 20; */ export class WorkspaceSvg extends Workspace - implements IASTNodeLocationSvg, IContextMenu + implements IASTNodeLocationSvg, IContextMenu, IFocusableNode, IFocusableTree { /** * A wrapper function called when a resize event occurs. @@ -764,7 +769,19 @@ export class WorkspaceSvg * * */ - this.svgGroup_ = dom.createSvgElement(Svg.G, {'class': 'blocklyWorkspace'}); + this.svgGroup_ = dom.createSvgElement(Svg.G, { + 'class': 'blocklyWorkspace', + // Only the top-level workspace should be tabbable. + 'tabindex': injectionDiv ? '0' : '-1', + 'id': this.id, + }); + if (injectionDiv) { + aria.setState( + this.svgGroup_, + aria.State.LABEL, + Msg['WORKSPACE_ARIA_LABEL'], + ); + } // Note that a alone does not receive mouse events--it must have a // valid target inside it. If no background class is specified, as in the @@ -840,6 +857,9 @@ export class WorkspaceSvg this.getTheme(), isParentWorkspace ? this.getInjectionDiv() : undefined, ); + + getFocusManager().registerTree(this); + return this.svgGroup_; } @@ -924,6 +944,10 @@ export class WorkspaceSvg document.body.removeEventListener('wheel', this.dummyWheelListener); this.dummyWheelListener = null; } + + if (getFocusManager().isRegistered(this)) { + getFocusManager().unregisterTree(this); + } } /** @@ -2631,6 +2655,55 @@ export class WorkspaceSvg deltaY *= scale; this.scroll(this.scrollX + deltaX, this.scrollY + deltaY); } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + return this.svgGroup_; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} + + /** See IFocusableTree.getRootFocusableNode. */ + getRootFocusableNode(): IFocusableNode { + return this; + } + + /** See IFocusableTree.getRestoredFocusableNode. */ + getRestoredFocusableNode( + previousNode: IFocusableNode | null, + ): IFocusableNode | null { + if (!previousNode) { + return this.getTopBlocks(true)[0] ?? null; + } else return null; + } + + /** See IFocusableTree.getNestedTrees. */ + getNestedTrees(): Array { + return []; + } + + /** See IFocusableTree.lookUpFocusableNode. */ + lookUpFocusableNode(id: string): IFocusableNode | null { + return this.getBlockById(id) as IFocusableNode; + } + + /** See IFocusableTree.onTreeFocus. */ + onTreeFocus( + _node: IFocusableNode, + _previousTree: IFocusableTree | null, + ): void {} + + /** See IFocusableTree.onTreeBlur. */ + onTreeBlur(_nextTree: IFocusableTree | null): void {} } /** From d4883f57d18ac92da2752942b15ae90cffb7cee5 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Thu, 24 Apr 2025 15:08:18 -0700 Subject: [PATCH 20/27] feat: Make toolbox and flyout focusable (#8920) ## The basics - [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change) ## The details ### Resolves Fixes #8918 Fixes #8919 Fixes part of #8771 ### Proposed Changes This updates several classes in order to make toolboxes and flyouts focusable: - `IFlyout` is now an `IFocusableTree` with corresponding implementations in `FlyoutBase`. - `IToolbox` is now an `IFocusableTree` with corresponding implementations in `Toolbox`. - `IToolboxItem` is now an `IFocusableNode` with corresponding implementations in `ToolboxItem`. - As the primary toolbox items, `ToolboxCategory` and `ToolboxSeparator` were updated to have -1 tab indexes and defined IDs to help `ToolboxItem` fulfill its contracted for `IFocusableNode.getFocusableElement`. Each of these two new focusable trees have specific noteworthy behaviors behaviors: - `Toolbox` will automatically indicate that its first item should be focused (if one is present), even overriding the ability to focus the toolbox's root (however there are some cases where that can still happen). - `Toolbox` will automatically synchronize its selection state with its item nodes being focused. - `FlyoutBase`, now being a focusable tree, has had a tab index of 0 added. Normally a tab index of -1 is all that's needed, but the keyboard navigation plugin specifically uses 0 for flyout so that the flyout is tabbable. This is a **new** tab stop being introduced. - `FlyoutBase` holds a workspace (for rendering blocks) and, since `WorkspaceSvg` is already set up to be a focusable tree, it's represented as a subtree to `FlyoutBase`. This does introduce some wonky behaviors: the flyout's root will have passive focus while its contents have active focus. This could be manually disabled with some CSS if it ends up being a confusing user experience. - Both `FlyoutBase` and `WorkspaceSvg` have built-in behaviors for detecting when a user tries navigating away from an open flyout to ensure that the flyout is closed when it's supposed to be. That is, the flyout is auto-hideable and a non-flyout, non-toolbox node has then been focused. This matches parity with the `T`/`Esc` flows supported in the keyboard navigation plugin playground. One other thing to note: `Toolbox` had a few tests to update that were trying to reinit a toolbox without first disposing of it (which was caught by one of `FocusManager`'s state guardrails). ### Reason for Changes This is part of an ongoing effort to ensure key components of Blockly are focusable so that they can be keyboard-navigable (with other needed changes yet both in Core Blockly and the keyboard navigation plugin). ### Test Coverage No new tests have been added. It's certainly possible to add unit tests for the focusable configurations being introduced in this PR, but it may not be highly beneficial. It's largely assumed that the individual implementations should work due to a highly tested FocusManager, and it may be the case that the interactions of the components working together is far more important to verify (that is, the end user flows). The latter is planned to be tackled as part of #8915. ### Documentation No documentation changes should be needed here. ### Additional Information This includes changes that have been pulled from #8875. --- core/flyout_base.ts | 69 ++++++++++++++++++++++++++- core/interfaces/i_flyout.ts | 3 +- core/interfaces/i_toolbox.ts | 3 +- core/interfaces/i_toolbox_item.ts | 4 +- core/toolbox/category.ts | 2 + core/toolbox/separator.ts | 2 + core/toolbox/toolbox.ts | 77 ++++++++++++++++++++++++++++++- core/toolbox/toolbox_item.ts | 24 ++++++++++ core/workspace_svg.ts | 14 +++++- tests/mocha/toolbox_test.js | 3 ++ 10 files changed, 194 insertions(+), 7 deletions(-) diff --git a/core/flyout_base.ts b/core/flyout_base.ts index e738470a606..d24ea2758a0 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -21,9 +21,12 @@ import * as eventUtils from './events/utils.js'; import {FlyoutItem} from './flyout_item.js'; import {FlyoutMetricsManager} from './flyout_metrics_manager.js'; import {FlyoutSeparator, SeparatorAxis} from './flyout_separator.js'; +import {getFocusManager} from './focus_manager.js'; import {IAutoHideable} from './interfaces/i_autohideable.js'; import type {IFlyout} from './interfaces/i_flyout.js'; import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js'; +import {IFocusableNode} from './interfaces/i_focusable_node.js'; +import {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {Options} from './options.js'; import * as registry from './registry.js'; import * as renderManagement from './render_management.js'; @@ -43,7 +46,7 @@ import {WorkspaceSvg} from './workspace_svg.js'; */ export abstract class Flyout extends DeleteArea - implements IAutoHideable, IFlyout + implements IAutoHideable, IFlyout, IFocusableNode { /** * Position the flyout. @@ -303,6 +306,7 @@ export abstract class Flyout // hide/show code will set up proper visibility and size later. this.svgGroup_ = dom.createSvgElement(tagName, { 'class': 'blocklyFlyout', + 'tabindex': '0', }); this.svgGroup_.style.display = 'none'; this.svgBackground_ = dom.createSvgElement( @@ -317,6 +321,9 @@ export abstract class Flyout this.workspace_ .getThemeManager() .subscribe(this.svgBackground_, 'flyoutOpacity', 'fill-opacity'); + + getFocusManager().registerTree(this); + return this.svgGroup_; } @@ -398,6 +405,7 @@ export abstract class Flyout if (this.svgGroup_) { dom.removeNode(this.svgGroup_); } + getFocusManager().unregisterTree(this); } /** @@ -961,4 +969,63 @@ export abstract class Flyout return null; } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + if (!this.svgGroup_) throw new Error('Flyout DOM is not yet created.'); + return this.svgGroup_; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} + + /** See IFocusableTree.getRootFocusableNode. */ + getRootFocusableNode(): IFocusableNode { + return this; + } + + /** See IFocusableTree.getRestoredFocusableNode. */ + getRestoredFocusableNode( + _previousNode: IFocusableNode | null, + ): IFocusableNode | null { + return null; + } + + /** See IFocusableTree.getNestedTrees. */ + getNestedTrees(): Array { + return [this.workspace_]; + } + + /** See IFocusableTree.lookUpFocusableNode. */ + lookUpFocusableNode(_id: string): IFocusableNode | null { + // No focusable node needs to be returned since the flyout's subtree is a + // workspace that will manage its own focusable state. + return null; + } + + /** See IFocusableTree.onTreeFocus. */ + onTreeFocus( + _node: IFocusableNode, + _previousTree: IFocusableTree | null, + ): void {} + + /** See IFocusableTree.onTreeBlur. */ + onTreeBlur(nextTree: IFocusableTree | null): void { + const toolbox = this.targetWorkspace.getToolbox(); + // If focus is moving to either the toolbox or the flyout's workspace, do + // not close the flyout. For anything else, do close it since the flyout is + // no longer focused. + if (toolbox && nextTree === toolbox) return; + if (nextTree == this.workspace_) return; + if (toolbox) toolbox.clearSelection(); + this.autoHide(false); + } } diff --git a/core/interfaces/i_flyout.ts b/core/interfaces/i_flyout.ts index 42204775ece..067cd5ef20d 100644 --- a/core/interfaces/i_flyout.ts +++ b/core/interfaces/i_flyout.ts @@ -12,12 +12,13 @@ import type {Coordinate} from '../utils/coordinate.js'; import type {Svg} from '../utils/svg.js'; import type {FlyoutDefinition} from '../utils/toolbox.js'; import type {WorkspaceSvg} from '../workspace_svg.js'; +import {IFocusableTree} from './i_focusable_tree.js'; import type {IRegistrable} from './i_registrable.js'; /** * Interface for a flyout. */ -export interface IFlyout extends IRegistrable { +export interface IFlyout extends IRegistrable, IFocusableTree { /** Whether the flyout is laid out horizontally or not. */ horizontalLayout: boolean; diff --git a/core/interfaces/i_toolbox.ts b/core/interfaces/i_toolbox.ts index 1780af94d8a..f5d9c9fd7c6 100644 --- a/core/interfaces/i_toolbox.ts +++ b/core/interfaces/i_toolbox.ts @@ -9,13 +9,14 @@ import type {ToolboxInfo} from '../utils/toolbox.js'; import type {WorkspaceSvg} from '../workspace_svg.js'; import type {IFlyout} from './i_flyout.js'; +import type {IFocusableTree} from './i_focusable_tree.js'; import type {IRegistrable} from './i_registrable.js'; import type {IToolboxItem} from './i_toolbox_item.js'; /** * Interface for a toolbox. */ -export interface IToolbox extends IRegistrable { +export interface IToolbox extends IRegistrable, IFocusableTree { /** Initializes the toolbox. */ init(): void; diff --git a/core/interfaces/i_toolbox_item.ts b/core/interfaces/i_toolbox_item.ts index e3c9864f0c0..661624fd7e8 100644 --- a/core/interfaces/i_toolbox_item.ts +++ b/core/interfaces/i_toolbox_item.ts @@ -6,10 +6,12 @@ // Former goog.module ID: Blockly.IToolboxItem +import type {IFocusableNode} from './i_focusable_node.js'; + /** * Interface for an item in the toolbox. */ -export interface IToolboxItem { +export interface IToolboxItem extends IFocusableNode { /** * Initializes the toolbox item. * This includes creating the DOM and updating the state of any items based diff --git a/core/toolbox/category.ts b/core/toolbox/category.ts index d8ee8736ea6..fc7d1aa03cf 100644 --- a/core/toolbox/category.ts +++ b/core/toolbox/category.ts @@ -225,6 +225,8 @@ export class ToolboxCategory */ protected createContainer_(): HTMLDivElement { const container = document.createElement('div'); + container.tabIndex = -1; + container.id = this.getId(); const className = this.cssConfig_['container']; if (className) { dom.addClass(container, className); diff --git a/core/toolbox/separator.ts b/core/toolbox/separator.ts index 31ccb7e42f3..44ae358cf53 100644 --- a/core/toolbox/separator.ts +++ b/core/toolbox/separator.ts @@ -54,6 +54,8 @@ export class ToolboxSeparator extends ToolboxItem { */ protected createDom_(): HTMLDivElement { const container = document.createElement('div'); + container.tabIndex = -1; + container.id = this.getId(); const className = this.cssConfig_['container']; if (className) { dom.addClass(container, className); diff --git a/core/toolbox/toolbox.ts b/core/toolbox/toolbox.ts index b0fd82e97f2..ceb756afbd6 100644 --- a/core/toolbox/toolbox.ts +++ b/core/toolbox/toolbox.ts @@ -22,11 +22,14 @@ import {DeleteArea} from '../delete_area.js'; import '../events/events_toolbox_item_select.js'; import {EventType} from '../events/type.js'; import * as eventUtils from '../events/utils.js'; +import {getFocusManager} from '../focus_manager.js'; import type {IAutoHideable} from '../interfaces/i_autohideable.js'; import type {ICollapsibleToolboxItem} from '../interfaces/i_collapsible_toolbox_item.js'; import {isDeletable} from '../interfaces/i_deletable.js'; 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 {IFocusableTree} from '../interfaces/i_focusable_tree.js'; import type {IKeyboardAccessible} from '../interfaces/i_keyboard_accessible.js'; import type {ISelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.js'; import {isSelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.js'; @@ -51,7 +54,12 @@ import {CollapsibleToolboxCategory} from './collapsible_category.js'; */ export class Toolbox extends DeleteArea - implements IAutoHideable, IKeyboardAccessible, IStyleable, IToolbox + implements + IAutoHideable, + IKeyboardAccessible, + IStyleable, + IToolbox, + IFocusableNode { /** * The unique ID for this component that is used to register with the @@ -163,6 +171,7 @@ export class Toolbox ComponentManager.Capability.DRAG_TARGET, ], }); + getFocusManager().registerTree(this); } /** @@ -177,7 +186,6 @@ export class Toolbox const container = this.createContainer_(); this.contentsDiv_ = this.createContentsContainer_(); - this.contentsDiv_.tabIndex = 0; aria.setRole(this.contentsDiv_, aria.Role.TREE); container.appendChild(this.contentsDiv_); @@ -194,6 +202,7 @@ export class Toolbox */ protected createContainer_(): HTMLDivElement { const toolboxContainer = document.createElement('div'); + toolboxContainer.tabIndex = 0; toolboxContainer.setAttribute('layout', this.isHorizontal() ? 'h' : 'v'); dom.addClass(toolboxContainer, 'blocklyToolbox'); toolboxContainer.setAttribute('dir', this.RTL ? 'RTL' : 'LTR'); @@ -1077,7 +1086,71 @@ export class Toolbox this.workspace_.getThemeManager().unsubscribe(this.HtmlDiv); dom.removeNode(this.HtmlDiv); } + + getFocusManager().unregisterTree(this); + } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + if (!this.HtmlDiv) throw Error('Toolbox DOM has not yet been created.'); + return this.HtmlDiv; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} + + /** See IFocusableTree.getRootFocusableNode. */ + getRootFocusableNode(): IFocusableNode { + return this; + } + + /** See IFocusableTree.getRestoredFocusableNode. */ + getRestoredFocusableNode( + previousNode: IFocusableNode | null, + ): IFocusableNode | null { + // Always try to select the first selectable toolbox item rather than the + // root of the toolbox. + if (!previousNode || previousNode === this) { + return this.getToolboxItems().find((item) => item.isSelectable()) ?? null; + } + return null; } + + /** See IFocusableTree.getNestedTrees. */ + getNestedTrees(): Array { + return []; + } + + /** See IFocusableTree.lookUpFocusableNode. */ + lookUpFocusableNode(id: string): IFocusableNode | null { + return this.getToolboxItemById(id) as IFocusableNode; + } + + /** See IFocusableTree.onTreeFocus. */ + onTreeFocus( + node: IFocusableNode, + _previousTree: IFocusableTree | null, + ): void { + if (node !== this) { + // Only select the item if it isn't already selected so as to not toggle. + if (this.getSelectedItem() !== node) { + this.setSelectedItem(node as IToolboxItem); + } + } else { + this.clearSelection(); + } + } + + /** See IFocusableTree.onTreeBlur. */ + onTreeBlur(_nextTree: IFocusableTree | null): void {} } /** CSS for Toolbox. See css.js for use. */ diff --git a/core/toolbox/toolbox_item.ts b/core/toolbox/toolbox_item.ts index ef9d979ab43..0d46a5eadfd 100644 --- a/core/toolbox/toolbox_item.ts +++ b/core/toolbox/toolbox_item.ts @@ -12,6 +12,7 @@ // Former goog.module ID: Blockly.ToolboxItem import type {ICollapsibleToolboxItem} from '../interfaces/i_collapsible_toolbox_item.js'; +import type {IFocusableTree} from '../interfaces/i_focusable_tree.js'; import type {IToolbox} from '../interfaces/i_toolbox.js'; import type {IToolboxItem} from '../interfaces/i_toolbox_item.js'; import * as idGenerator from '../utils/idgenerator.js'; @@ -148,5 +149,28 @@ export class ToolboxItem implements IToolboxItem { * @param _isVisible True if category should be visible. */ setVisible_(_isVisible: boolean) {} + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + const div = this.getDiv(); + if (!div) { + throw Error('Trying to access toolbox item before DOM is initialized.'); + } + if (!(div instanceof HTMLElement)) { + throw Error('Toolbox item div is unexpectedly not an HTML element.'); + } + return div as HTMLElement; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.parentToolbox_; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} } // nop by default diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 2648005b257..5f4c57a5774 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -2703,7 +2703,19 @@ export class WorkspaceSvg ): void {} /** See IFocusableTree.onTreeBlur. */ - onTreeBlur(_nextTree: IFocusableTree | null): void {} + onTreeBlur(nextTree: IFocusableTree | null): void { + // If the flyout loses focus, make sure to close it. + if (this.isFlyout && this.targetWorkspace) { + // Only hide the flyout if the flyout's workspace is losing focus and that + // focus isn't returning to the flyout itself or the toolbox. + const flyout = this.targetWorkspace.getFlyout(); + const toolbox = this.targetWorkspace.getToolbox(); + if (flyout && nextTree === flyout) return; + if (toolbox && nextTree === toolbox) return; + if (toolbox) toolbox.clearSelection(); + if (flyout && flyout instanceof Flyout) flyout.autoHide(false); + } + } } /** diff --git a/tests/mocha/toolbox_test.js b/tests/mocha/toolbox_test.js index 10bfd335223..f32319c6779 100644 --- a/tests/mocha/toolbox_test.js +++ b/tests/mocha/toolbox_test.js @@ -54,6 +54,7 @@ suite('Toolbox', function () { const themeManagerSpy = sinon.spy(themeManager, 'subscribe'); const componentManager = this.toolbox.workspace_.getComponentManager(); sinon.stub(componentManager, 'addComponent'); + this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited. this.toolbox.init(); sinon.assert.calledWith( themeManagerSpy, @@ -72,12 +73,14 @@ suite('Toolbox', function () { const renderSpy = sinon.spy(this.toolbox, 'render'); const componentManager = this.toolbox.workspace_.getComponentManager(); sinon.stub(componentManager, 'addComponent'); + this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited. this.toolbox.init(); sinon.assert.calledOnce(renderSpy); }); test('Init called -> Flyout is initialized', function () { const componentManager = this.toolbox.workspace_.getComponentManager(); sinon.stub(componentManager, 'addComponent'); + this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited. this.toolbox.init(); assert.isDefined(this.toolbox.getFlyout()); }); From e5abf720c887c05ac93c6855199b1a85e547bcff Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 00:21:52 +0000 Subject: [PATCH 21/27] fix: Remove CSS for active/passive focus. The CSS work here is far more extensive than should go in this or any of the structural branches. Instead, proper rendering will be done via the plugin and eventually merged into Core once finalized. --- core/css.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/core/css.ts b/core/css.ts index 6ca262f3b25..ed47e8037ec 100644 --- a/core/css.ts +++ b/core/css.ts @@ -463,8 +463,8 @@ input[type=number] { } .blocklyMenuSeparator { - background-color: #ccc; - height: 1px; + background-color: #ccc; + height: 1px; border: 0; margin-left: 4px; margin-right: 4px; @@ -494,12 +494,4 @@ input[type=number] { cursor: grabbing; } -.blocklyActiveFocus { - outline-color: #2ae; - outline-width: 2px; -} -.blocklyPassiveFocus { - outline-color: #3fdfff; - outline-width: 1.5px; -} `; From 585c950f38428612b551761f92ca2acfb661da53 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 00:41:26 +0000 Subject: [PATCH 22/27] feat: Make FlyoutButton focusable. --- core/flyout_button.ts | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/core/flyout_button.ts b/core/flyout_button.ts index 3b9b2fe0735..f24a3d3bc5f 100644 --- a/core/flyout_button.ts +++ b/core/flyout_button.ts @@ -15,6 +15,8 @@ import type {IASTNodeLocationSvg} from './blockly.js'; import * as browserEvents from './browser_events.js'; import * as Css from './css.js'; import type {IBoundedElement} from './interfaces/i_bounded_element.js'; +import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {IRenderedElement} from './interfaces/i_rendered_element.js'; import {Coordinate} from './utils/coordinate.js'; import * as dom from './utils/dom.js'; @@ -29,7 +31,11 @@ import type {WorkspaceSvg} from './workspace_svg.js'; * Class for a button or label in the flyout. */ export class FlyoutButton - implements IASTNodeLocationSvg, IBoundedElement, IRenderedElement + implements + IASTNodeLocationSvg, + IBoundedElement, + IRenderedElement, + IFocusableNode { /** The horizontal margin around the text in the button. */ static TEXT_MARGIN_X = 5; @@ -107,7 +113,7 @@ export class FlyoutButton this.svgGroup = dom.createSvgElement( Svg.G, - {'class': cssClass}, + {'class': cssClass, 'tabindex': '-1'}, this.workspace.getCanvas(), ); @@ -389,6 +395,22 @@ export class FlyoutButton getSvgRoot() { return this.svgGroup; } + + /** See IFocusableNode.getFocusableElement. */ + getFocusableElement(): HTMLElement | SVGElement { + return this.svgGroup; + } + + /** See IFocusableNode.getFocusableTree. */ + getFocusableTree(): IFocusableTree { + return this.workspace; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} } /** CSS for buttons and labels. See css.js for use. */ From d6dcc4b42da30de1c03ef7fc1954587811b63b05 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 01:24:32 +0000 Subject: [PATCH 23/27] fix: Actually make FlyoutButton focusable. It needed a unique ID and to be properly hooked up to node retrieval. --- core/flyout_button.ts | 7 ++++++- core/workspace_svg.ts | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/core/flyout_button.ts b/core/flyout_button.ts index f24a3d3bc5f..21aa408e543 100644 --- a/core/flyout_button.ts +++ b/core/flyout_button.ts @@ -26,6 +26,7 @@ import * as style from './utils/style.js'; import {Svg} from './utils/svg.js'; import type * as toolbox from './utils/toolbox.js'; import type {WorkspaceSvg} from './workspace_svg.js'; +import {idGenerator} from './utils.js'; /** * Class for a button or label in the flyout. @@ -74,6 +75,9 @@ export class FlyoutButton */ cursorSvg: SVGElement | null = null; + /** The unique ID for this FlyoutButton. */ + private id: string; + /** * @param workspace The workspace in which to place this button. * @param targetWorkspace The flyout's target workspace. @@ -111,9 +115,10 @@ export class FlyoutButton cssClass += ' ' + this.cssClass; } + this.id = idGenerator.getNextUniqueId(); this.svgGroup = dom.createSvgElement( Svg.G, - {'class': cssClass, 'tabindex': '-1'}, + {'id': this.id, 'class': cssClass, 'tabindex': '-1'}, this.workspace.getCanvas(), ); diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 5f4c57a5774..b99679db3fd 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -45,7 +45,7 @@ import type {IBoundedElement} from './interfaces/i_bounded_element.js'; import {IContextMenu} from './interfaces/i_contextmenu.js'; import type {IDragTarget} from './interfaces/i_drag_target.js'; import type {IFlyout} from './interfaces/i_flyout.js'; -import type {IFocusableNode} from './interfaces/i_focusable_node.js'; +import {isFocusableNode, type IFocusableNode} from './interfaces/i_focusable_node.js'; import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {IMetricsManager} from './interfaces/i_metrics_manager.js'; import type {IToolbox} from './interfaces/i_toolbox.js'; @@ -2693,6 +2693,19 @@ export class WorkspaceSvg /** See IFocusableTree.lookUpFocusableNode. */ lookUpFocusableNode(id: string): IFocusableNode | null { + // Check against flyout items if this workspace is part of a flyout. Note + // that blocks may match against this pass before reaching getBlockById() + // below (but only for a flyout workspace). + const flyout = this.targetWorkspace?.getFlyout(); + if (this.isFlyout && flyout) { + for (const flyoutItem of flyout.getContents()) { + const elem = flyoutItem.getElement(); + if (isFocusableNode(elem) && elem.getFocusableElement().id === id) { + return elem; + } + } + } + return this.getBlockById(id) as IFocusableNode; } From a9cf3d73990bd4e29ab24fc920057472d58f2f5c Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 01:27:25 +0000 Subject: [PATCH 24/27] chore: lint fixes. --- core/flyout_button.ts | 2 +- core/workspace_svg.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/core/flyout_button.ts b/core/flyout_button.ts index 21aa408e543..666a2e081bf 100644 --- a/core/flyout_button.ts +++ b/core/flyout_button.ts @@ -18,6 +18,7 @@ import type {IBoundedElement} from './interfaces/i_bounded_element.js'; import type {IFocusableNode} from './interfaces/i_focusable_node.js'; import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {IRenderedElement} from './interfaces/i_rendered_element.js'; +import {idGenerator} from './utils.js'; import {Coordinate} from './utils/coordinate.js'; import * as dom from './utils/dom.js'; import * as parsing from './utils/parsing.js'; @@ -26,7 +27,6 @@ import * as style from './utils/style.js'; import {Svg} from './utils/svg.js'; import type * as toolbox from './utils/toolbox.js'; import type {WorkspaceSvg} from './workspace_svg.js'; -import {idGenerator} from './utils.js'; /** * Class for a button or label in the flyout. diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index b99679db3fd..921fb72ecc7 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -45,7 +45,10 @@ import type {IBoundedElement} from './interfaces/i_bounded_element.js'; import {IContextMenu} from './interfaces/i_contextmenu.js'; import type {IDragTarget} from './interfaces/i_drag_target.js'; import type {IFlyout} from './interfaces/i_flyout.js'; -import {isFocusableNode, type IFocusableNode} from './interfaces/i_focusable_node.js'; +import { + isFocusableNode, + type IFocusableNode, +} from './interfaces/i_focusable_node.js'; import type {IFocusableTree} from './interfaces/i_focusable_tree.js'; import type {IMetricsManager} from './interfaces/i_metrics_manager.js'; import type {IToolbox} from './interfaces/i_toolbox.js'; From f18670afd714850c5b437c477bc3a0a0c0e1fe8c Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 01:33:07 +0000 Subject: [PATCH 25/27] chore: Use strict equals. Addresses self-review comment. --- core/flyout_base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/flyout_base.ts b/core/flyout_base.ts index d24ea2758a0..a0ba75eb9fe 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -1024,7 +1024,7 @@ export abstract class Flyout // not close the flyout. For anything else, do close it since the flyout is // no longer focused. if (toolbox && nextTree === toolbox) return; - if (nextTree == this.workspace_) return; + if (nextTree === this.workspace_) return; if (toolbox) toolbox.clearSelection(); this.autoHide(false); } From 34970cc5b7fdd222e2b3b75076d88bd5555a6874 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 01:40:25 +0000 Subject: [PATCH 26/27] chore: Empty commit to re-trigger CI. From 3cf8fb8eed1295068708d4143878cf7f1229df70 Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Wed, 30 Apr 2025 01:56:22 +0000 Subject: [PATCH 27/27] chore: Add field doc. Addresses self-review comment. --- core/field.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/core/field.ts b/core/field.ts index 1da60f18b81..a5e43a27665 100644 --- a/core/field.ts +++ b/core/field.ts @@ -195,6 +195,7 @@ export abstract class Field */ SERIALIZABLE = false; + /** The unique ID of this field. */ private id_: string | null = null; /**