Skip to content

Commit 3806982

Browse files
feat: allow for toolbox with multiple tab stops
- Switch to focusin/out for toolbox (maybe in future elsewhere) - Drop the tab index fiddling as it's not practical to get it right in the face of more than one tab stop in the toolbox, especially if that toolbox has a DOM structure not known to the plugin. Given that we're reinstating the ability to shift-tab flyout->toolbox it makes sense to allow tab to the flyout. Shift-tab to the flyout remains not possible in the auto-hide case but that's not unreasonable. This version can be made to work with MakeCode without further modifications.
1 parent 7067e1c commit 3806982

File tree

2 files changed

+27
-24
lines changed

2 files changed

+27
-24
lines changed

src/index.ts

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ export class KeyboardNavigation {
3131
private blurListener: (e: Event) => void;
3232

3333
/** Event handler run when the toolbox gains focus. */
34-
private toolboxFocusListener: () => void;
34+
private toolboxFocusInListener: (e: Event) => void;
3535

3636
/** Event handler run when the toolbox loses focus. */
37-
private toolboxBlurListener: (e: Event) => void;
37+
private toolboxFocusOutListener: (e: Event) => void;
3838

3939
/** Event handler run when the flyout gains focus. */
4040
private flyoutFocusListener: () => void;
@@ -99,10 +99,6 @@ export class KeyboardNavigation {
9999
flyoutElement,
100100
workspace.getParentSvg(),
101101
);
102-
// Allow tab to the flyout only when there's no toolbox.
103-
if (workspace.getToolbox() && flyoutElement) {
104-
flyoutElement.tabIndex = -1;
105-
}
106102

107103
this.focusListener = (e: Event) => {
108104
if (e.currentTarget === this.workspace.getParentSvg()) {
@@ -143,10 +139,26 @@ export class KeyboardNavigation {
143139
workspace.getSvgGroup().addEventListener('blur', this.blurListener);
144140

145141
const toolboxElement = getToolboxElement(workspace);
146-
this.toolboxFocusListener = () => {
142+
this.toolboxFocusInListener = (e: Event) => {
143+
if (
144+
(e.currentTarget as Element).contains(
145+
(e as FocusEvent).relatedTarget as Node,
146+
)
147+
) {
148+
return;
149+
}
150+
147151
this.navigationController.handleFocusToolbox(workspace);
148152
};
149-
this.toolboxBlurListener = (e: Event) => {
153+
this.toolboxFocusOutListener = (e: Event) => {
154+
if (
155+
(e.currentTarget as Element).contains(
156+
(e as FocusEvent).relatedTarget as Node,
157+
)
158+
) {
159+
return;
160+
}
161+
150162
this.navigationController.handleBlurToolbox(
151163
workspace,
152164
this.shouldCloseFlyoutOnBlur(
@@ -155,8 +167,8 @@ export class KeyboardNavigation {
155167
),
156168
);
157169
};
158-
toolboxElement?.addEventListener('focus', this.toolboxFocusListener);
159-
toolboxElement?.addEventListener('blur', this.toolboxBlurListener);
170+
toolboxElement?.addEventListener('focusin', this.toolboxFocusInListener);
171+
toolboxElement?.addEventListener('focusout', this.toolboxFocusOutListener);
160172

161173
this.flyoutFocusListener = () => {
162174
this.navigationController.handleFocusFlyout(workspace);
@@ -198,8 +210,11 @@ export class KeyboardNavigation {
198210
.removeEventListener('focus', this.focusListener);
199211

200212
const toolboxElement = getToolboxElement(this.workspace);
201-
toolboxElement?.removeEventListener('focus', this.toolboxFocusListener);
202-
toolboxElement?.removeEventListener('blur', this.toolboxBlurListener);
213+
toolboxElement?.removeEventListener('focusin', this.toolboxFocusInListener);
214+
toolboxElement?.removeEventListener(
215+
'focusout',
216+
this.toolboxFocusOutListener,
217+
);
203218

204219
const flyoutElement = getFlyoutElement(this.workspace);
205220
flyoutElement?.removeEventListener('focus', this.flyoutFocusListener);

src/navigation.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -503,12 +503,6 @@ export class Navigation {
503503
this.setState(workspace, Constants.STATE.FLYOUT);
504504
this.getFlyoutCursor(workspace)?.draw();
505505
this.resetFlyoutCursorPosition(workspace);
506-
507-
// Prevent shift-tab to the toolbox while the flyout has focus.
508-
const toolboxElement = getToolboxElement(workspace);
509-
if (toolboxElement) {
510-
toolboxElement.tabIndex = -1;
511-
}
512506
}
513507

514508
/**
@@ -524,12 +518,6 @@ export class Navigation {
524518
workspace.hideChaff();
525519
}
526520
this.getFlyoutCursor(workspace)?.hide();
527-
528-
// Reinstate tab to toolbox.
529-
const toolboxElement = getToolboxElement(workspace);
530-
if (toolboxElement) {
531-
toolboxElement.tabIndex = 0;
532-
}
533521
}
534522

535523
/**

0 commit comments

Comments
 (0)