Skip to content

Commit 7fcad2a

Browse files
authored
Revert "feat: Make toolbox and flyout focusable (#8920)"
This reverts commit 5bc8380.
1 parent 8f59649 commit 7fcad2a

File tree

10 files changed

+7
-194
lines changed

10 files changed

+7
-194
lines changed

core/flyout_base.ts

Lines changed: 1 addition & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,9 @@ 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';
2524
import {IAutoHideable} from './interfaces/i_autohideable.js';
2625
import type {IFlyout} from './interfaces/i_flyout.js';
2726
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';
3027
import type {Options} from './options.js';
3128
import * as registry from './registry.js';
3229
import * as renderManagement from './render_management.js';
@@ -46,7 +43,7 @@ import {WorkspaceSvg} from './workspace_svg.js';
4643
*/
4744
export abstract class Flyout
4845
extends DeleteArea
49-
implements IAutoHideable, IFlyout, IFocusableNode
46+
implements IAutoHideable, IFlyout
5047
{
5148
/**
5249
* Position the flyout.
@@ -306,7 +303,6 @@ export abstract class Flyout
306303
// hide/show code will set up proper visibility and size later.
307304
this.svgGroup_ = dom.createSvgElement(tagName, {
308305
'class': 'blocklyFlyout',
309-
'tabindex': '0',
310306
});
311307
this.svgGroup_.style.display = 'none';
312308
this.svgBackground_ = dom.createSvgElement(
@@ -321,9 +317,6 @@ export abstract class Flyout
321317
this.workspace_
322318
.getThemeManager()
323319
.subscribe(this.svgBackground_, 'flyoutOpacity', 'fill-opacity');
324-
325-
getFocusManager().registerTree(this);
326-
327320
return this.svgGroup_;
328321
}
329322

@@ -405,7 +398,6 @@ export abstract class Flyout
405398
if (this.svgGroup_) {
406399
dom.removeNode(this.svgGroup_);
407400
}
408-
getFocusManager().unregisterTree(this);
409401
}
410402

411403
/**
@@ -969,63 +961,4 @@ export abstract class Flyout
969961

970962
return null;
971963
}
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-
}
1031964
}

core/interfaces/i_flyout.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,12 @@ 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';
1615
import type {IRegistrable} from './i_registrable.js';
1716

1817
/**
1918
* Interface for a flyout.
2019
*/
21-
export interface IFlyout extends IRegistrable, IFocusableTree {
20+
export interface IFlyout extends IRegistrable {
2221
/** Whether the flyout is laid out horizontally or not. */
2322
horizontalLayout: boolean;
2423

core/interfaces/i_toolbox.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@
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';
1312
import type {IRegistrable} from './i_registrable.js';
1413
import type {IToolboxItem} from './i_toolbox_item.js';
1514

1615
/**
1716
* Interface for a toolbox.
1817
*/
19-
export interface IToolbox extends IRegistrable, IFocusableTree {
18+
export interface IToolbox extends IRegistrable {
2019
/** Initializes the toolbox. */
2120
init(): void;
2221

core/interfaces/i_toolbox_item.ts

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

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

9-
import type {IFocusableNode} from './i_focusable_node.js';
10-
119
/**
1210
* Interface for an item in the toolbox.
1311
*/
14-
export interface IToolboxItem extends IFocusableNode {
12+
export interface IToolboxItem {
1513
/**
1614
* Initializes the toolbox item.
1715
* This includes creating the DOM and updating the state of any items based

core/toolbox/category.ts

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

core/toolbox/separator.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,6 @@ 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();
5957
const className = this.cssConfig_['container'];
6058
if (className) {
6159
dom.addClass(container, className);

core/toolbox/toolbox.ts

Lines changed: 2 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,11 @@ 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';
2625
import type {IAutoHideable} from '../interfaces/i_autohideable.js';
2726
import type {ICollapsibleToolboxItem} from '../interfaces/i_collapsible_toolbox_item.js';
2827
import {isDeletable} from '../interfaces/i_deletable.js';
2928
import type {IDraggable} from '../interfaces/i_draggable.js';
3029
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';
3330
import type {IKeyboardAccessible} from '../interfaces/i_keyboard_accessible.js';
3431
import type {ISelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.js';
3532
import {isSelectableToolboxItem} from '../interfaces/i_selectable_toolbox_item.js';
@@ -54,12 +51,7 @@ import {CollapsibleToolboxCategory} from './collapsible_category.js';
5451
*/
5552
export class Toolbox
5653
extends DeleteArea
57-
implements
58-
IAutoHideable,
59-
IKeyboardAccessible,
60-
IStyleable,
61-
IToolbox,
62-
IFocusableNode
54+
implements IAutoHideable, IKeyboardAccessible, IStyleable, IToolbox
6355
{
6456
/**
6557
* The unique ID for this component that is used to register with the
@@ -171,7 +163,6 @@ export class Toolbox
171163
ComponentManager.Capability.DRAG_TARGET,
172164
],
173165
});
174-
getFocusManager().registerTree(this);
175166
}
176167

177168
/**
@@ -186,6 +177,7 @@ export class Toolbox
186177
const container = this.createContainer_();
187178

188179
this.contentsDiv_ = this.createContentsContainer_();
180+
this.contentsDiv_.tabIndex = 0;
189181
aria.setRole(this.contentsDiv_, aria.Role.TREE);
190182
container.appendChild(this.contentsDiv_);
191183

@@ -202,7 +194,6 @@ export class Toolbox
202194
*/
203195
protected createContainer_(): HTMLDivElement {
204196
const toolboxContainer = document.createElement('div');
205-
toolboxContainer.tabIndex = 0;
206197
toolboxContainer.setAttribute('layout', this.isHorizontal() ? 'h' : 'v');
207198
dom.addClass(toolboxContainer, 'blocklyToolbox');
208199
toolboxContainer.setAttribute('dir', this.RTL ? 'RTL' : 'LTR');
@@ -1086,71 +1077,7 @@ export class Toolbox
10861077
this.workspace_.getThemeManager().unsubscribe(this.HtmlDiv);
10871078
dom.removeNode(this.HtmlDiv);
10881079
}
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;
11251080
}
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 {}
11541081
}
11551082

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

core/toolbox/toolbox_item.ts

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
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';
1615
import type {IToolbox} from '../interfaces/i_toolbox.js';
1716
import type {IToolboxItem} from '../interfaces/i_toolbox_item.js';
1817
import * as idGenerator from '../utils/idgenerator.js';
@@ -149,28 +148,5 @@ export class ToolboxItem implements IToolboxItem {
149148
* @param _isVisible True if category should be visible.
150149
*/
151150
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 {}
175151
}
176152
// nop by default

core/workspace_svg.ts

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

26922692
/** See IFocusableTree.onTreeBlur. */
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-
}
2693+
onTreeBlur(_nextTree: IFocusableTree | null): void {}
27062694
}
27072695

27082696
/**

tests/mocha/toolbox_test.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ 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.
5857
this.toolbox.init();
5958
sinon.assert.calledWith(
6059
themeManagerSpy,
@@ -73,14 +72,12 @@ suite('Toolbox', function () {
7372
const renderSpy = sinon.spy(this.toolbox, 'render');
7473
const componentManager = this.toolbox.workspace_.getComponentManager();
7574
sinon.stub(componentManager, 'addComponent');
76-
this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited.
7775
this.toolbox.init();
7876
sinon.assert.calledOnce(renderSpy);
7977
});
8078
test('Init called -> Flyout is initialized', function () {
8179
const componentManager = this.toolbox.workspace_.getComponentManager();
8280
sinon.stub(componentManager, 'addComponent');
83-
this.toolbox.dispose(); // Dispose of the old toolbox so that it can be reinited.
8481
this.toolbox.init();
8582
assert.isDefined(this.toolbox.getFlyout());
8683
});

0 commit comments

Comments
 (0)