Skip to content

Commit 5bc8380

Browse files
authored
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.
1 parent d7680cf commit 5bc8380

File tree

10 files changed

+194
-7
lines changed

10 files changed

+194
-7
lines changed

core/flyout_base.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,12 @@ import * as eventUtils from './events/utils.js';
2121
import {FlyoutItem} from './flyout_item.js';
2222
import {FlyoutMetricsManager} from './flyout_metrics_manager.js';
2323
import {FlyoutSeparator, SeparatorAxis} from './flyout_separator.js';
24+
import {getFocusManager} from './focus_manager.js';
2425
import {IAutoHideable} from './interfaces/i_autohideable.js';
2526
import type {IFlyout} from './interfaces/i_flyout.js';
2627
import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js';
28+
import {IFocusableNode} from './interfaces/i_focusable_node.js';
29+
import {IFocusableTree} from './interfaces/i_focusable_tree.js';
2730
import type {Options} from './options.js';
2831
import * as registry from './registry.js';
2932
import * as renderManagement from './render_management.js';
@@ -43,7 +46,7 @@ import {WorkspaceSvg} from './workspace_svg.js';
4346
*/
4447
export abstract class Flyout
4548
extends DeleteArea
46-
implements IAutoHideable, IFlyout
49+
implements IAutoHideable, IFlyout, IFocusableNode
4750
{
4851
/**
4952
* Position the flyout.
@@ -303,6 +306,7 @@ export abstract class Flyout
303306
// hide/show code will set up proper visibility and size later.
304307
this.svgGroup_ = dom.createSvgElement(tagName, {
305308
'class': 'blocklyFlyout',
309+
'tabindex': '0',
306310
});
307311
this.svgGroup_.style.display = 'none';
308312
this.svgBackground_ = dom.createSvgElement(
@@ -317,6 +321,9 @@ export abstract class Flyout
317321
this.workspace_
318322
.getThemeManager()
319323
.subscribe(this.svgBackground_, 'flyoutOpacity', 'fill-opacity');
324+
325+
getFocusManager().registerTree(this);
326+
320327
return this.svgGroup_;
321328
}
322329

@@ -398,6 +405,7 @@ export abstract class Flyout
398405
if (this.svgGroup_) {
399406
dom.removeNode(this.svgGroup_);
400407
}
408+
getFocusManager().unregisterTree(this);
401409
}
402410

403411
/**
@@ -961,4 +969,63 @@ export abstract class Flyout
961969

962970
return null;
963971
}
972+
973+
/** See IFocusableNode.getFocusableElement. */
974+
getFocusableElement(): HTMLElement | SVGElement {
975+
if (!this.svgGroup_) throw new Error('Flyout DOM is not yet created.');
976+
return this.svgGroup_;
977+
}
978+
979+
/** See IFocusableNode.getFocusableTree. */
980+
getFocusableTree(): IFocusableTree {
981+
return this;
982+
}
983+
984+
/** See IFocusableNode.onNodeFocus. */
985+
onNodeFocus(): void {}
986+
987+
/** See IFocusableNode.onNodeBlur. */
988+
onNodeBlur(): void {}
989+
990+
/** See IFocusableTree.getRootFocusableNode. */
991+
getRootFocusableNode(): IFocusableNode {
992+
return this;
993+
}
994+
995+
/** See IFocusableTree.getRestoredFocusableNode. */
996+
getRestoredFocusableNode(
997+
_previousNode: IFocusableNode | null,
998+
): IFocusableNode | null {
999+
return null;
1000+
}
1001+
1002+
/** See IFocusableTree.getNestedTrees. */
1003+
getNestedTrees(): Array<IFocusableTree> {
1004+
return [this.workspace_];
1005+
}
1006+
1007+
/** See IFocusableTree.lookUpFocusableNode. */
1008+
lookUpFocusableNode(_id: string): IFocusableNode | null {
1009+
// No focusable node needs to be returned since the flyout's subtree is a
1010+
// workspace that will manage its own focusable state.
1011+
return null;
1012+
}
1013+
1014+
/** See IFocusableTree.onTreeFocus. */
1015+
onTreeFocus(
1016+
_node: IFocusableNode,
1017+
_previousTree: IFocusableTree | null,
1018+
): void {}
1019+
1020+
/** See IFocusableTree.onTreeBlur. */
1021+
onTreeBlur(nextTree: IFocusableTree | null): void {
1022+
const toolbox = this.targetWorkspace.getToolbox();
1023+
// If focus is moving to either the toolbox or the flyout's workspace, do
1024+
// not close the flyout. For anything else, do close it since the flyout is
1025+
// no longer focused.
1026+
if (toolbox && nextTree === toolbox) return;
1027+
if (nextTree == this.workspace_) return;
1028+
if (toolbox) toolbox.clearSelection();
1029+
this.autoHide(false);
1030+
}
9641031
}

core/interfaces/i_flyout.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,13 @@ import type {Coordinate} from '../utils/coordinate.js';
1212
import type {Svg} from '../utils/svg.js';
1313
import type {FlyoutDefinition} from '../utils/toolbox.js';
1414
import type {WorkspaceSvg} from '../workspace_svg.js';
15+
import {IFocusableTree} from './i_focusable_tree.js';
1516
import type {IRegistrable} from './i_registrable.js';
1617

1718
/**
1819
* Interface for a flyout.
1920
*/
20-
export interface IFlyout extends IRegistrable {
21+
export interface IFlyout extends IRegistrable, IFocusableTree {
2122
/** Whether the flyout is laid out horizontally or not. */
2223
horizontalLayout: boolean;
2324

core/interfaces/i_toolbox.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@
99
import type {ToolboxInfo} from '../utils/toolbox.js';
1010
import type {WorkspaceSvg} from '../workspace_svg.js';
1111
import type {IFlyout} from './i_flyout.js';
12+
import type {IFocusableTree} from './i_focusable_tree.js';
1213
import type {IRegistrable} from './i_registrable.js';
1314
import type {IToolboxItem} from './i_toolbox_item.js';
1415

1516
/**
1617
* Interface for a toolbox.
1718
*/
18-
export interface IToolbox extends IRegistrable {
19+
export interface IToolbox extends IRegistrable, IFocusableTree {
1920
/** Initializes the toolbox. */
2021
init(): void;
2122

core/interfaces/i_toolbox_item.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66

77
// Former goog.module ID: Blockly.IToolboxItem
88

9+
import type {IFocusableNode} from './i_focusable_node.js';
10+
911
/**
1012
* Interface for an item in the toolbox.
1113
*/
12-
export interface IToolboxItem {
14+
export interface IToolboxItem extends IFocusableNode {
1315
/**
1416
* Initializes the toolbox item.
1517
* This includes creating the DOM and updating the state of any items based

core/toolbox/category.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@ export class ToolboxCategory
225225
*/
226226
protected createContainer_(): HTMLDivElement {
227227
const container = document.createElement('div');
228+
container.tabIndex = -1;
229+
container.id = this.getId();
228230
const className = this.cssConfig_['container'];
229231
if (className) {
230232
dom.addClass(container, className);

core/toolbox/separator.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ export class ToolboxSeparator extends ToolboxItem {
5454
*/
5555
protected createDom_(): HTMLDivElement {
5656
const container = document.createElement('div');
57+
container.tabIndex = -1;
58+
container.id = this.getId();
5759
const className = this.cssConfig_['container'];
5860
if (className) {
5961
dom.addClass(container, className);

core/toolbox/toolbox.ts

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,14 @@ import {DeleteArea} from '../delete_area.js';
2222
import '../events/events_toolbox_item_select.js';
2323
import {EventType} from '../events/type.js';
2424
import * as eventUtils from '../events/utils.js';
25+
import {getFocusManager} from '../focus_manager.js';
2526
import type {IAutoHideable} from '../interfaces/i_autohideable.js';
2627
import type {ICollapsibleToolboxItem} from '../interfaces/i_collapsible_toolbox_item.js';
2728
import {isDeletable} from '../interfaces/i_deletable.js';
2829
import type {IDraggable} from '../interfaces/i_draggable.js';
2930
import type {IFlyout} from '../interfaces/i_flyout.js';
31+
import type {IFocusableNode} from '../interfaces/i_focusable_node.js';
32+
import type {IFocusableTree} from '../interfaces/i_focusable_tree.js';
3033
import type {IKeyboardAccessible} from '../interfaces/i_keyboard_accessible.js';
3134
import type {ISelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.js';
3235
import {isSelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.js';
@@ -51,7 +54,12 @@ import {CollapsibleToolboxCategory} from './collapsible_category.js';
5154
*/
5255
export class Toolbox
5356
extends DeleteArea
54-
implements IAutoHideable, IKeyboardAccessible, IStyleable, IToolbox
57+
implements
58+
IAutoHideable,
59+
IKeyboardAccessible,
60+
IStyleable,
61+
IToolbox,
62+
IFocusableNode
5563
{
5664
/**
5765
* The unique ID for this component that is used to register with the
@@ -163,6 +171,7 @@ export class Toolbox
163171
ComponentManager.Capability.DRAG_TARGET,
164172
],
165173
});
174+
getFocusManager().registerTree(this);
166175
}
167176

168177
/**
@@ -177,7 +186,6 @@ export class Toolbox
177186
const container = this.createContainer_();
178187

179188
this.contentsDiv_ = this.createContentsContainer_();
180-
this.contentsDiv_.tabIndex = 0;
181189
aria.setRole(this.contentsDiv_, aria.Role.TREE);
182190
container.appendChild(this.contentsDiv_);
183191

@@ -194,6 +202,7 @@ export class Toolbox
194202
*/
195203
protected createContainer_(): HTMLDivElement {
196204
const toolboxContainer = document.createElement('div');
205+
toolboxContainer.tabIndex = 0;
197206
toolboxContainer.setAttribute('layout', this.isHorizontal() ? 'h' : 'v');
198207
dom.addClass(toolboxContainer, 'blocklyToolbox');
199208
toolboxContainer.setAttribute('dir', this.RTL ? 'RTL' : 'LTR');
@@ -1077,7 +1086,71 @@ export class Toolbox
10771086
this.workspace_.getThemeManager().unsubscribe(this.HtmlDiv);
10781087
dom.removeNode(this.HtmlDiv);
10791088
}
1089+
1090+
getFocusManager().unregisterTree(this);
1091+
}
1092+
1093+
/** See IFocusableNode.getFocusableElement. */
1094+
getFocusableElement(): HTMLElement | SVGElement {
1095+
if (!this.HtmlDiv) throw Error('Toolbox DOM has not yet been created.');
1096+
return this.HtmlDiv;
1097+
}
1098+
1099+
/** See IFocusableNode.getFocusableTree. */
1100+
getFocusableTree(): IFocusableTree {
1101+
return this;
1102+
}
1103+
1104+
/** See IFocusableNode.onNodeFocus. */
1105+
onNodeFocus(): void {}
1106+
1107+
/** See IFocusableNode.onNodeBlur. */
1108+
onNodeBlur(): void {}
1109+
1110+
/** See IFocusableTree.getRootFocusableNode. */
1111+
getRootFocusableNode(): IFocusableNode {
1112+
return this;
1113+
}
1114+
1115+
/** See IFocusableTree.getRestoredFocusableNode. */
1116+
getRestoredFocusableNode(
1117+
previousNode: IFocusableNode | null,
1118+
): IFocusableNode | null {
1119+
// Always try to select the first selectable toolbox item rather than the
1120+
// root of the toolbox.
1121+
if (!previousNode || previousNode === this) {
1122+
return this.getToolboxItems().find((item) => item.isSelectable()) ?? null;
1123+
}
1124+
return null;
10801125
}
1126+
1127+
/** See IFocusableTree.getNestedTrees. */
1128+
getNestedTrees(): Array<IFocusableTree> {
1129+
return [];
1130+
}
1131+
1132+
/** See IFocusableTree.lookUpFocusableNode. */
1133+
lookUpFocusableNode(id: string): IFocusableNode | null {
1134+
return this.getToolboxItemById(id) as IFocusableNode;
1135+
}
1136+
1137+
/** See IFocusableTree.onTreeFocus. */
1138+
onTreeFocus(
1139+
node: IFocusableNode,
1140+
_previousTree: IFocusableTree | null,
1141+
): void {
1142+
if (node !== this) {
1143+
// Only select the item if it isn't already selected so as to not toggle.
1144+
if (this.getSelectedItem() !== node) {
1145+
this.setSelectedItem(node as IToolboxItem);
1146+
}
1147+
} else {
1148+
this.clearSelection();
1149+
}
1150+
}
1151+
1152+
/** See IFocusableTree.onTreeBlur. */
1153+
onTreeBlur(_nextTree: IFocusableTree | null): void {}
10811154
}
10821155

10831156
/** CSS for Toolbox. See css.js for use. */

core/toolbox/toolbox_item.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// Former goog.module ID: Blockly.ToolboxItem
1313

1414
import type {ICollapsibleToolboxItem} from '../interfaces/i_collapsible_toolbox_item.js';
15+
import type {IFocusableTree} from '../interfaces/i_focusable_tree.js';
1516
import type {IToolbox} from '../interfaces/i_toolbox.js';
1617
import type {IToolboxItem} from '../interfaces/i_toolbox_item.js';
1718
import * as idGenerator from '../utils/idgenerator.js';
@@ -148,5 +149,28 @@ export class ToolboxItem implements IToolboxItem {
148149
* @param _isVisible True if category should be visible.
149150
*/
150151
setVisible_(_isVisible: boolean) {}
152+
153+
/** See IFocusableNode.getFocusableElement. */
154+
getFocusableElement(): HTMLElement | SVGElement {
155+
const div = this.getDiv();
156+
if (!div) {
157+
throw Error('Trying to access toolbox item before DOM is initialized.');
158+
}
159+
if (!(div instanceof HTMLElement)) {
160+
throw Error('Toolbox item div is unexpectedly not an HTML element.');
161+
}
162+
return div as HTMLElement;
163+
}
164+
165+
/** See IFocusableNode.getFocusableTree. */
166+
getFocusableTree(): IFocusableTree {
167+
return this.parentToolbox_;
168+
}
169+
170+
/** See IFocusableNode.onNodeFocus. */
171+
onNodeFocus(): void {}
172+
173+
/** See IFocusableNode.onNodeBlur. */
174+
onNodeBlur(): void {}
151175
}
152176
// nop by default

core/workspace_svg.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2690,7 +2690,19 @@ export class WorkspaceSvg
26902690
): void {}
26912691

26922692
/** See IFocusableTree.onTreeBlur. */
2693-
onTreeBlur(_nextTree: IFocusableTree | null): void {}
2693+
onTreeBlur(nextTree: IFocusableTree | null): void {
2694+
// If the flyout loses focus, make sure to close it.
2695+
if (this.isFlyout && this.targetWorkspace) {
2696+
// Only hide the flyout if the flyout's workspace is losing focus and that
2697+
// focus isn't returning to the flyout itself or the toolbox.
2698+
const flyout = this.targetWorkspace.getFlyout();
2699+
const toolbox = this.targetWorkspace.getToolbox();
2700+
if (flyout && nextTree === flyout) return;
2701+
if (toolbox && nextTree === toolbox) return;
2702+
if (toolbox) toolbox.clearSelection();
2703+
if (flyout && flyout instanceof Flyout) flyout.autoHide(false);
2704+
}
2705+
}
26942706
}
26952707

26962708
/**

tests/mocha/toolbox_test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ suite('Toolbox', function () {
5454
const themeManagerSpy = sinon.spy(themeManager, 'subscribe');
5555
const componentManager = this.toolbox.workspace_.getComponentManager();
5656
sinon.stub(componentManager, 'addComponent');
57+
this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited.
5758
this.toolbox.init();
5859
sinon.assert.calledWith(
5960
themeManagerSpy,
@@ -72,12 +73,14 @@ suite('Toolbox', function () {
7273
const renderSpy = sinon.spy(this.toolbox, 'render');
7374
const componentManager = this.toolbox.workspace_.getComponentManager();
7475
sinon.stub(componentManager, 'addComponent');
76+
this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited.
7577
this.toolbox.init();
7678
sinon.assert.calledOnce(renderSpy);
7779
});
7880
test('Init called -> Flyout is initialized', function () {
7981
const componentManager = this.toolbox.workspace_.getComponentManager();
8082
sinon.stub(componentManager, 'addComponent');
83+
this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited.
8184
this.toolbox.init();
8285
assert.isDefined(this.toolbox.getFlyout());
8386
});

0 commit comments

Comments
 (0)