From 6fffc3af2883bda35c1c170639a4c31905eb6495 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Dec 2023 16:08:10 +0000 Subject: [PATCH 1/7] fix: Tab inside an editable field should trigger keyboard mode Tab key should still trigger keyboard navigation when inside an editable field. There is still an unknown edge case where rich text fields could use Tab key to indent - however we should handle that case if it ends up resulting in any bugs. Also cleans up the conditions to trigger/dismiss keyboard navigation mode. --- src/Keyborg.ts | 62 ++++++++++++++++++++++++++++++-------------------- src/utils.ts | 18 +++++++++++++++ 2 files changed, 55 insertions(+), 25 deletions(-) create mode 100644 src/utils.ts diff --git a/src/Keyborg.ts b/src/Keyborg.ts index 20a186877..b6f1f3e21 100644 --- a/src/Keyborg.ts +++ b/src/Keyborg.ts @@ -10,6 +10,7 @@ import { setupFocusEvent, } from "./FocusEvent"; import { Disposable, WeakRefInstance } from "./WeakRefInstance"; +import { getActiveElement } from "./utils"; interface WindowWithKeyborg extends Window { __keyborg?: { @@ -229,35 +230,46 @@ class KeyborgCore implements Disposable { private _onKeyDown = (e: KeyboardEvent): void => { const isNavigatingWithKeyboard = _state.getVal(); - const keyCode = e.keyCode; - const triggerKeys = this._triggerKeys; - - if ( - !isNavigatingWithKeyboard && - (!triggerKeys || triggerKeys.has(keyCode)) - ) { - const activeElement = this._win?.document.activeElement as - | HTMLElement - | null - | undefined; - - if ( - activeElement && - (activeElement.tagName === "INPUT" || - activeElement.tagName === "TEXTAREA" || - activeElement.contentEditable === "true") - ) { - // We're inside an input, textarea or contenteditable, it's not - // keyboard navigation, it is text editing scenario. - return; + if (isNavigatingWithKeyboard) { + if (this._shouldDismissKeyboardNavigation(e)) { + this._scheduleDismiss(); + } + } else { + if (this._shouldTriggerKeyboardNavigation(e)) { + _state.setVal(true); } - - _state.setVal(true); - } else if (isNavigatingWithKeyboard && this._dismissKeys?.has(keyCode)) { - this._scheduleDismiss(); } }; + /** + * @returns whether the keyboard event should trigger keyboard navigation mode + */ + private _shouldTriggerKeyboardNavigation(e: KeyboardEvent) { + // TODO Some rich text fields can allow Tab key for indentation so it doesn't + // need to be a navigation key. If there is a bug regarding that we should revisit + if (e.key === "Tab") { + return true; + } + + const activeElement = getActiveElement(this._win); + const isTriggerKey = !this._triggerKeys || this._triggerKeys.has(e.keyCode); + + const isEditable = + activeElement && + (activeElement.tagName === "INPUT" || + activeElement.tagName === "TEXTAREA" || + activeElement.contentEditable === "true"); + + return isTriggerKey && !isEditable; + } + + /** + * @returns whether the keyboard event should dismiss keyboard navigation mode + */ + private _shouldDismissKeyboardNavigation(e: KeyboardEvent) { + return this._dismissKeys?.has(e.keyCode); + } + private _scheduleDismiss(): void { const win = this._win; diff --git a/src/utils.ts b/src/utils.ts new file mode 100644 index 000000000..c276bdc79 --- /dev/null +++ b/src/utils.ts @@ -0,0 +1,18 @@ +/*! + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * Also drills into open shadow roots + * @returns The active element of a document + */ +export function getActiveElement(win: Window | undefined): HTMLElement | null { + const activeElement = win?.document.activeElement as HTMLElement | null; + + if (!activeElement?.shadowRoot) { + return activeElement; + } + + return activeElement.shadowRoot.activeElement as HTMLElement | null; +} From 157a5cc39fd3599b7b0d3abe6717a870005da2fc Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Wed, 20 Dec 2023 16:15:57 +0000 Subject: [PATCH 2/7] remove shadow support for now --- src/Keyborg.ts | 4 ++-- src/utils.ts | 18 ------------------ 2 files changed, 2 insertions(+), 20 deletions(-) delete mode 100644 src/utils.ts diff --git a/src/Keyborg.ts b/src/Keyborg.ts index b6f1f3e21..8966a5451 100644 --- a/src/Keyborg.ts +++ b/src/Keyborg.ts @@ -10,7 +10,6 @@ import { setupFocusEvent, } from "./FocusEvent"; import { Disposable, WeakRefInstance } from "./WeakRefInstance"; -import { getActiveElement } from "./utils"; interface WindowWithKeyborg extends Window { __keyborg?: { @@ -251,7 +250,8 @@ class KeyborgCore implements Disposable { return true; } - const activeElement = getActiveElement(this._win); + const activeElement = this._win?.document + .activeElement as HTMLElement | null; const isTriggerKey = !this._triggerKeys || this._triggerKeys.has(e.keyCode); const isEditable = diff --git a/src/utils.ts b/src/utils.ts deleted file mode 100644 index c276bdc79..000000000 --- a/src/utils.ts +++ /dev/null @@ -1,18 +0,0 @@ -/*! - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -/** - * Also drills into open shadow roots - * @returns The active element of a document - */ -export function getActiveElement(win: Window | undefined): HTMLElement | null { - const activeElement = win?.document.activeElement as HTMLElement | null; - - if (!activeElement?.shadowRoot) { - return activeElement; - } - - return activeElement.shadowRoot.activeElement as HTMLElement | null; -} From 95af8357297389c3aeed9b344800d4765a7fa6a2 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Thu, 28 Dec 2023 14:01:29 +0100 Subject: [PATCH 3/7] add test --- tests/focus-behavior.spec.ts | 9 +++++++++ tests/focus-behavior.stories.tsx | 15 +++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/tests/focus-behavior.spec.ts b/tests/focus-behavior.spec.ts index f253f8a6f..5ca35bb39 100644 --- a/tests/focus-behavior.spec.ts +++ b/tests/focus-behavior.spec.ts @@ -30,3 +30,12 @@ test("Buttons scenario", async ({ page }) => { await page.getByText("Button B").click(); await expect(page.getByTestId("keyboard-mode")).toHaveText("false"); }); + +test("Input scenario", async ({ page }) => { + await page.goto("/?mode=preview&story=focus-behavior--input"); + + await expect(page.getByTestId("keyboard-mode")).toHaveText("false"); + await page.focus("input"); + await page.keyboard.press("Tab"); + await expect(page.getByTestId("keyboard-mode")).toHaveText("true"); +}); diff --git a/tests/focus-behavior.stories.tsx b/tests/focus-behavior.stories.tsx index 9c3c88ef8..7ed628c88 100644 --- a/tests/focus-behavior.stories.tsx +++ b/tests/focus-behavior.stories.tsx @@ -21,3 +21,18 @@ export const Buttons = () => ( ); + +export const Input = () => ( +
+

Focus Behavior with Buttons

+ +
+ + +
+ + +
+); From a657863957baae2b8dc67e973f19aa4284d82dbc Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Thu, 28 Dec 2023 14:02:12 +0100 Subject: [PATCH 4/7] isContentEditable --- src/Keyborg.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Keyborg.ts b/src/Keyborg.ts index ab6c31611..47ff082ed 100644 --- a/src/Keyborg.ts +++ b/src/Keyborg.ts @@ -258,7 +258,7 @@ class KeyborgCore implements Disposable { activeElement && (activeElement.tagName === "INPUT" || activeElement.tagName === "TEXTAREA" || - activeElement.contentEditable === "true"); + activeElement.isContentEditable); return isTriggerKey && !isEditable; } From 2eb8542557079921e8e22716df75858e3a223def Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Thu, 28 Dec 2023 18:11:18 +0100 Subject: [PATCH 5/7] feat: Support multi bundle --- src/Keyborg.ts | 230 ++++++++++++++++----------------------- src/WeakRefInstance.ts | 9 +- src/constants.ts | 8 ++ src/createEventTarget.ts | 7 ++ src/types.ts | 21 ++++ 5 files changed, 133 insertions(+), 142 deletions(-) create mode 100644 src/constants.ts create mode 100644 src/createEventTarget.ts create mode 100644 src/types.ts diff --git a/src/Keyborg.ts b/src/Keyborg.ts index 47ff082ed..74c1f2f06 100644 --- a/src/Keyborg.ts +++ b/src/Keyborg.ts @@ -9,95 +9,34 @@ import { KEYBORG_FOCUSIN, setupFocusEvent, } from "./FocusEvent"; -import { Disposable, WeakRefInstance } from "./WeakRefInstance"; +import { DIMISS_TIMEOUT, KEYBORG_KEYBOARDNAVIGATION } from "./constants"; +import { + Disposable, + KeyboardNavigationEventData, + KeyborgCallback, + KeyborgProps, +} from "./types"; interface WindowWithKeyborg extends Window { - __keyborg?: { + __keyborg_v2?: { + refs: Set; core: KeyborgCore; - refs: { [id: string]: Keyborg }; }; } -const _dismissTimeout = 500; // When a key from dismissKeys is pressed and the focus is not moved -// during _dismissTimeout time, dismiss the keyboard navigation mode. - -let _lastId = 0; - -export interface KeyborgProps { - // Keys to be used to trigger keyboard navigation mode. By default, any key will trigger - // it. Could be limited to, for example, just Tab (or Tab and arrow keys). - triggerKeys?: number[]; - // Keys to be used to dismiss keyboard navigation mode using keyboard (in addition to - // mouse clicks which dismiss it). For example, Esc could be used to dismiss. - dismissKeys?: number[]; -} - -export type KeyborgCallback = (isNavigatingWithKeyboard: boolean) => void; - -/** - * Source of truth for all the keyborg core instances and the current keyboard navigation state - */ -export class KeyborgState { - private __keyborgCoreRefs: { [id: string]: WeakRefInstance } = - {}; - private _isNavigatingWithKeyboard = false; - - add(keyborg: KeyborgCore): void { - const id = keyborg.id; - - if (!(id in this.__keyborgCoreRefs)) { - this.__keyborgCoreRefs[id] = new WeakRefInstance(keyborg); - } - } - - remove(id: string): void { - delete this.__keyborgCoreRefs[id]; - - if (Object.keys(this.__keyborgCoreRefs).length === 0) { - this._isNavigatingWithKeyboard = false; - } - } - - setVal(isNavigatingWithKeyboard: boolean): void { - if (this._isNavigatingWithKeyboard === isNavigatingWithKeyboard) { - return; - } - - this._isNavigatingWithKeyboard = isNavigatingWithKeyboard; - - for (const id of Object.keys(this.__keyborgCoreRefs)) { - const ref = this.__keyborgCoreRefs[id]; - const keyborg = ref.deref(); - - if (keyborg) { - keyborg.update(isNavigatingWithKeyboard); - } else { - this.remove(id); - } - } - } - - getVal(): boolean { - return this._isNavigatingWithKeyboard; - } -} - -const _state = new KeyborgState(); - /** * Manages a collection of Keyborg instances in a window/document and updates keyborg state */ class KeyborgCore implements Disposable { - readonly id: string; - private _win?: WindowWithKeyborg; private _isMouseUsedTimer: number | undefined; private _dismissTimer: number | undefined; private _triggerKeys?: Set; private _dismissKeys?: Set; + private _isNavigatingWithKeyboard_DO_NOT_USE_DIRECTLY = false; + constructor(win: WindowWithKeyborg, props?: KeyborgProps) { - this.id = "c" + ++_lastId; this._win = win; const doc = win.document; @@ -119,8 +58,23 @@ class KeyborgCore implements Disposable { win.addEventListener("keydown", this._onKeyDown, true); // Capture! setupFocusEvent(win); + } + + get isNavigatingWithKeyboard() { + return this._isNavigatingWithKeyboard_DO_NOT_USE_DIRECTLY; + } - _state.add(this); + set isNavigatingWithKeyboard(val: boolean) { + if (this._isNavigatingWithKeyboard_DO_NOT_USE_DIRECTLY === val || !this._win) { + return; + } + + this._isNavigatingWithKeyboard_DO_NOT_USE_DIRECTLY = val; + this._win.dispatchEvent( + new CustomEvent(KEYBORG_KEYBOARDNAVIGATION, { + detail: { isNavigatingWithKeyboard: val }, + }), + ); } dispose(): void { @@ -146,8 +100,6 @@ class KeyborgCore implements Disposable { win.removeEventListener("keydown", this._onKeyDown, true); // Capture! delete this._win; - - _state.remove(this.id); } } @@ -159,13 +111,11 @@ class KeyborgCore implements Disposable { * Updates all keyborg instances with the keyboard navigation state */ update(isNavigatingWithKeyboard: boolean): void { - const keyborgs = this._win?.__keyborg?.refs; - - if (keyborgs) { - for (const id of Object.keys(keyborgs)) { - Keyborg.update(keyborgs[id], isNavigatingWithKeyboard); - } - } + this._win?.dispatchEvent( + new CustomEvent(KEYBORG_KEYBOARDNAVIGATION, { + detail: { isNavigatingWithKeyboard }, + }), + ); } private _onFocusIn = (e: KeyborgFocusInEvent) => { @@ -179,7 +129,7 @@ class KeyborgCore implements Disposable { return; } - if (_state.getVal()) { + if (this.isNavigatingWithKeyboard) { return; } @@ -198,7 +148,7 @@ class KeyborgCore implements Disposable { return; } - _state.setVal(true); + this.isNavigatingWithKeyboard = true; }; private _onMouseDown = (e: MouseEvent): void => { @@ -223,19 +173,17 @@ class KeyborgCore implements Disposable { }, 1000); // Keeping the indication of the mouse usage for some time. } - _state.setVal(false); + this.isNavigatingWithKeyboard = false; }; private _onKeyDown = (e: KeyboardEvent): void => { - const isNavigatingWithKeyboard = _state.getVal(); - - if (isNavigatingWithKeyboard) { + if (this.isNavigatingWithKeyboard) { if (this._shouldDismissKeyboardNavigation(e)) { this._scheduleDismiss(); } } else { if (this._shouldTriggerKeyboardNavigation(e)) { - _state.setVal(true); + this.isNavigatingWithKeyboard = true; } } }; @@ -289,9 +237,9 @@ class KeyborgCore implements Disposable { if (was && cur && was === cur) { // Esc was pressed, currently focused element hasn't changed. // Just dismiss the keyboard navigation mode. - _state.setVal(false); + this.isNavigatingWithKeyboard = false; } - }, _dismissTimeout); + }, DIMISS_TIMEOUT); } } } @@ -300,13 +248,17 @@ class KeyborgCore implements Disposable { * Used to determine the keyboard navigation state */ export class Keyborg { - private _id: string; private _win?: WindowWithKeyborg; - private _core?: KeyborgCore; - private _cb: KeyborgCallback[] = []; + private _cb = new Map< + KeyborgCallback, + (e: CustomEvent) => void + >(); - static create(win: WindowWithKeyborg, props?: KeyborgProps): Keyborg { - return new Keyborg(win, props); + /** + * @deprecated + */ + static create(win: WindowWithKeyborg): Keyborg { + return new Keyborg(win); } static dispose(instance: Keyborg): void { @@ -314,49 +266,29 @@ export class Keyborg { } /** - * Updates all subscribed callbacks with the keyboard navigation state + * @deprecated no longer used internally */ - static update(instance: Keyborg, isNavigatingWithKeyboard: boolean): void { - instance._cb.forEach((callback) => callback(isNavigatingWithKeyboard)); - } + static update(instance: Keyborg, isNavigatingWithKeyboard: boolean): void {} - private constructor(win: WindowWithKeyborg, props?: KeyborgProps) { - this._id = "k" + ++_lastId; + constructor(win: WindowWithKeyborg) { this._win = win; - - const current = win.__keyborg; - - if (current) { - this._core = current.core; - current.refs[this._id] = this; - } else { - this._core = new KeyborgCore(win, props); - win.__keyborg = { - core: this._core, - refs: { [this._id]: this }, - }; - } } private dispose(): void { - const current = this._win?.__keyborg; + const current = this._win?.__keyborg_v2; - if (current?.refs[this._id]) { - delete current.refs[this._id]; + if (current?.refs.has(this)) { + current.refs.delete(this); - if (Object.keys(current.refs).length === 0) { - current.core.dispose(); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - delete this._win!.__keyborg; + if (current.refs.size === 0) { + this._win?.__keyborg_v2?.core.dispose(); + delete this._win?.__keyborg_v2; } } else if (process.env.NODE_ENV !== "production") { - console.error( - `Keyborg instance ${this._id} is being disposed incorrectly.`, - ); + console.error(`Keyborg instance is being disposed incorrectly.`); } - this._cb = []; - delete this._core; + this._cb = new Map(); delete this._win; } @@ -364,37 +296,65 @@ export class Keyborg { * @returns Whether the user is navigating with keyboard */ isNavigatingWithKeyboard(): boolean { - return _state.getVal(); + return !!this._win?.__keyborg_v2?.core.isNavigatingWithKeyboard; } /** * @param callback - Called when the keyboard navigation state changes */ subscribe(callback: KeyborgCallback): void { - this._cb.push(callback); + if (this._cb.has(callback)) { + return; + } + + const handler = (e: CustomEvent) => { + callback(e.detail.isNavigatingWithKeyboard); + }; + this._win?.addEventListener(KEYBORG_KEYBOARDNAVIGATION, handler); + this._cb.set(callback, handler); } /** * @param callback - Registered with subscribe */ unsubscribe(callback: KeyborgCallback): void { - const index = this._cb.indexOf(callback); - - if (index >= 0) { - this._cb.splice(index, 1); + if (!this._cb.has(callback)) { + return; } + + this._win?.removeEventListener( + KEYBORG_KEYBOARDNAVIGATION, + this._cb.get(callback)!, + ); + this._cb.delete(callback); } /** * Manually set the keyboard navigtion state */ setVal(isNavigatingWithKeyboard: boolean): void { - _state.setVal(isNavigatingWithKeyboard); + if (!this._win?.__keyborg_v2) { + return; + } + + this._win.__keyborg_v2.core.isNavigatingWithKeyboard = + isNavigatingWithKeyboard; } } export function createKeyborg(win: Window, props?: KeyborgProps): Keyborg { - return Keyborg.create(win, props); + const keyborgWin = win as WindowWithKeyborg; + if (!keyborgWin.__keyborg_v2) { + keyborgWin.__keyborg_v2 = { + core: new KeyborgCore(win, props), + refs: new Set(), + }; + } + + const keyborg = new Keyborg(win); + keyborgWin.__keyborg_v2.refs.add(keyborg); + + return keyborg; } export function disposeKeyborg(instance: Keyborg) { diff --git a/src/WeakRefInstance.ts b/src/WeakRefInstance.ts index 26e940dde..6644a16ba 100644 --- a/src/WeakRefInstance.ts +++ b/src/WeakRefInstance.ts @@ -3,16 +3,11 @@ * Licensed under the MIT License. */ +import { Disposable } from "./types"; + // IE11 compat, checks if WeakRef is supported export const _canUseWeakRef = typeof WeakRef !== "undefined"; -/** - * Allows disposable instances to be used - */ -export interface Disposable { - isDisposed?(): boolean; -} - /** * WeakRef wrapper around a HTMLElement that also supports IE11 * @see {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef} diff --git a/src/constants.ts b/src/constants.ts new file mode 100644 index 000000000..b2a61c939 --- /dev/null +++ b/src/constants.ts @@ -0,0 +1,8 @@ +export const KEYBORG_FOCUSIN = "keyborg:focusin"; +export const KEYBORG_KEYBOARDNAVIGATION = "keyborg:keyboardnavigation"; + +/** + * When a key from dismissKeys is pressed and the focus is not moved + * during _dismissTimeout time, dismiss the keyboard navigation mode. + */ +export const DIMISS_TIMEOUT = 500; diff --git a/src/createEventTarget.ts b/src/createEventTarget.ts new file mode 100644 index 000000000..0fd85ef64 --- /dev/null +++ b/src/createEventTarget.ts @@ -0,0 +1,7 @@ +export function createEventTarget(win: Window) { + if (typeof EventTarget === "undefined") { + return win.document.createElement("div"); + } + + return new EventTarget(); +} diff --git a/src/types.ts b/src/types.ts new file mode 100644 index 000000000..5c7104545 --- /dev/null +++ b/src/types.ts @@ -0,0 +1,21 @@ +export interface KeyboardNavigationEventData { + isNavigatingWithKeyboard: boolean; +} + +export interface KeyborgProps { + // Keys to be used to trigger keyboard navigation mode. By default, any key will trigger + // it. Could be limited to, for example, just Tab (or Tab and arrow keys). + triggerKeys?: number[]; + // Keys to be used to dismiss keyboard navigation mode using keyboard (in addition to + // mouse clicks which dismiss it). For example, Esc could be used to dismiss. + dismissKeys?: number[]; +} + +export type KeyborgCallback = (isNavigatingWithKeyboard: boolean) => void; + +/** + * Allows disposable instances to be used + */ +export interface Disposable { + isDisposed?(): boolean; +} From 817dde83077616d9f69d868e11c3cbfedcf91f82 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Thu, 28 Dec 2023 18:19:26 +0100 Subject: [PATCH 6/7] fix formatting --- src/Keyborg.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Keyborg.ts b/src/Keyborg.ts index 74c1f2f06..73d48bff1 100644 --- a/src/Keyborg.ts +++ b/src/Keyborg.ts @@ -65,7 +65,10 @@ class KeyborgCore implements Disposable { } set isNavigatingWithKeyboard(val: boolean) { - if (this._isNavigatingWithKeyboard_DO_NOT_USE_DIRECTLY === val || !this._win) { + if ( + this._isNavigatingWithKeyboard_DO_NOT_USE_DIRECTLY === val || + !this._win + ) { return; } From 988c7bfa476b496ae68c37028c5d6e1a4d4ab848 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Thu, 28 Dec 2023 18:20:16 +0100 Subject: [PATCH 7/7] remove file --- src/createEventTarget.ts | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 src/createEventTarget.ts diff --git a/src/createEventTarget.ts b/src/createEventTarget.ts deleted file mode 100644 index 0fd85ef64..000000000 --- a/src/createEventTarget.ts +++ /dev/null @@ -1,7 +0,0 @@ -export function createEventTarget(win: Window) { - if (typeof EventTarget === "undefined") { - return win.document.createElement("div"); - } - - return new EventTarget(); -}