From 49f8c7617fec0b054eac855e3e9431018bf5e4f8 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Thu, 17 Sep 2026 13:48:06 -0400 Subject: [PATCH 1/2] feat(focus): let useFocusManager listen on a given event target The focus manager bound keydown and keyup on `document`. A host without a DOM, such as a NativeScript app that reports remote presses through its own bridge, had nowhere to hand those events. `useFocusManager` now takes the target as a second argument, typed as the two listener methods it uses, and still defaults to `document`, so browser apps are unchanged. Co-Authored-By: Claude Fable 5.1 --- docs/primitives/useFocusManager.md | 16 ++++++ src/core/focusManager.ts | 50 ++++++++++++++---- src/index.ts | 7 ++- src/primitives/useFocusManager.ts | 2 + tests/focusManagerTarget.test.tsx | 83 ++++++++++++++++++++++++++++++ 5 files changed, 148 insertions(+), 10 deletions(-) create mode 100644 tests/focusManagerTarget.test.tsx diff --git a/docs/primitives/useFocusManager.md b/docs/primitives/useFocusManager.md index 8943b9fc..79f5f5e4 100644 --- a/docs/primitives/useFocusManager.md +++ b/docs/primitives/useFocusManager.md @@ -27,6 +27,22 @@ const App = () => { }; ``` +### Event Target + +`useFocusManager` binds `keydown` and `keyup` on `document`. A host without a +document, such as a native runtime that reports remote presses through its own +bridge, passes the object to listen on as the second argument. It needs only +`addEventListener` and `removeEventListener` for those two event types (the +`KeyEventTarget` type), and the events it delivers need only the fields the +focus manager reads: `key`, `keyCode` and `repeat` (`KeyEventLike`). Key +handlers then receive the host's event object in place of a `KeyboardEvent`. + +```jsx +useFocusManager(undefined, keyBridge); +``` + +Browser apps are unaffected: leave the argument out and `document` is used. + ### Focus Path Tracking `focusPath` is a signal holding the array of elements that currently have focus, from the focused leaf up to the root. It is imported separately — `useFocusManager` itself returns nothing: diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index eb5187fa..21599424 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -558,7 +558,37 @@ const handleKeyEvents = (keydown?: KeyboardEvent, keyup?: KeyboardEvent) => { } }; -export const useFocusManager = (userKeyMap?: Partial) => { +/** + * What the focus manager reads off a key event. A browser's `KeyboardEvent` + * has these; a host without a DOM raises objects that carry at least them, + * and key handlers receive whichever object the host raised. + */ +export interface KeyEventLike { + readonly key: string; + readonly keyCode: number; + readonly repeat: boolean; +} + +/** + * Where {@link useFocusManager} listens for `keydown` and `keyup`: `document` + * in a browser, or anything with the same two methods on a host without one, + * such as a native key bridge. + */ +export interface KeyEventTarget { + addEventListener( + type: 'keydown' | 'keyup', + listener: (event: KeyEventLike) => void, + ): void; + removeEventListener( + type: 'keydown' | 'keyup', + listener: (event: KeyEventLike) => void, + ): void; +} + +export const useFocusManager = ( + userKeyMap?: Partial, + target: KeyEventTarget = document, +) => { if (userKeyMap) { flattenKeyMap(userKeyMap, keyMapEntries); } @@ -577,17 +607,19 @@ export const useFocusManager = (userKeyMap?: Partial) => { Config.setActiveElement = (elm) => ownerContext(() => setActiveElementSignal(elm)); - const keyPressHandler = (event: KeyboardEvent) => - ownerContext(() => handleKeyEvents(event, undefined)); - const keyUpHandler = (event: KeyboardEvent) => - ownerContext(() => handleKeyEvents(undefined, event)); + // Handlers are typed as KeyboardEvent throughout; on a host that raises + // its own objects they see those, which carry the fields read here. + const keyPressHandler = (event: KeyEventLike) => + ownerContext(() => handleKeyEvents(event as KeyboardEvent, undefined)); + const keyUpHandler = (event: KeyEventLike) => + ownerContext(() => handleKeyEvents(undefined, event as KeyboardEvent)); - document.addEventListener('keydown', keyPressHandler); - document.addEventListener('keyup', keyUpHandler); + target.addEventListener('keydown', keyPressHandler); + target.addEventListener('keyup', keyUpHandler); onCleanup(() => { - document.removeEventListener('keydown', keyPressHandler); - document.removeEventListener('keyup', keyUpHandler); + target.removeEventListener('keydown', keyPressHandler); + target.removeEventListener('keyup', keyUpHandler); suppressedKeys.clear(); }); }; diff --git a/src/index.ts b/src/index.ts index 61013588..3915330e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,12 @@ export type * from '@solidtv/solid/jsx-runtime'; export * from './core/index.js'; export type * from './core/index.js'; -export type { KeyHandler, KeyMap } from './core/focusManager.js'; +export type { + KeyEventLike, + KeyEventTarget, + KeyHandler, + KeyMap, +} from './core/focusManager.js'; export { activeElement, setActiveElement } from './core/activeElement.js'; export { setActiveElementCore } from './core/focusManager.js'; export * from './utils.js'; diff --git a/src/primitives/useFocusManager.ts b/src/primitives/useFocusManager.ts index 10e192c4..560208d1 100644 --- a/src/primitives/useFocusManager.ts +++ b/src/primitives/useFocusManager.ts @@ -7,6 +7,8 @@ export { printFocusHistory, getFocusHistory, type FocusHistoryEntry, + type KeyEventLike, + type KeyEventTarget, type KeyMap, } from '../core/focusManager.js'; export { activeElement, setActiveElement } from '../core/activeElement.js'; diff --git a/tests/focusManagerTarget.test.tsx b/tests/focusManagerTarget.test.tsx new file mode 100644 index 00000000..b53f056a --- /dev/null +++ b/tests/focusManagerTarget.test.tsx @@ -0,0 +1,83 @@ +import * as v from 'vitest'; +import { + useFocusManager, + type KeyEventLike, + type KeyEventTarget, +} from '@solidtv/solid/primitives'; +import { renderer, waitForUpdate } from './setup.js'; + +type Listener = (event: KeyEventLike) => void; + +// A host without a document: records the listeners it is handed so the test +// can fire them and check they are gone after cleanup. +class FakeTarget implements KeyEventTarget { + listeners: Record<'keydown' | 'keyup', Listener[]> = { + keydown: [], + keyup: [], + }; + + addEventListener(type: 'keydown' | 'keyup', listener: Listener) { + this.listeners[type].push(listener); + } + + removeEventListener(type: 'keydown' | 'keyup', listener: Listener) { + const list = this.listeners[type]; + const idx = list.indexOf(listener); + if (idx !== -1) list.splice(idx, 1); + } + + press(type: 'keydown' | 'keyup', key: string) { + const event: KeyEventLike = { key, keyCode: 0, repeat: false }; + for (const listener of this.listeners[type].slice()) listener(event); + } +} + +async function setup(target: FakeTarget) { + const onEnter = v.vi.fn(); + const onEnterRelease = v.vi.fn(); + const dispose = renderer.render(() => { + useFocusManager(undefined, target); + return ; + }) as unknown as () => void; + await waitForUpdate(); + return { onEnter, onEnterRelease, dispose }; +} + +v.describe('useFocusManager event target', () => { + v.test('binds keydown and keyup on the given target', async () => { + const target = new FakeTarget(); + const { onEnter, onEnterRelease, dispose } = await setup(target); + + v.assert.equal(target.listeners.keydown.length, 1); + v.assert.equal(target.listeners.keyup.length, 1); + + target.press('keydown', 'Enter'); + v.assert.equal(onEnter.mock.calls.length, 1); + target.press('keyup', 'Enter'); + v.assert.equal(onEnterRelease.mock.calls.length, 1); + + dispose(); + }); + + v.test('leaves document alone when a target is given', async () => { + const target = new FakeTarget(); + const { onEnter, dispose } = await setup(target); + + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); + v.assert.equal(onEnter.mock.calls.length, 0); + + dispose(); + }); + + v.test('removes its listeners from the target on cleanup', async () => { + const target = new FakeTarget(); + const { onEnter, dispose } = await setup(target); + + dispose(); + v.assert.equal(target.listeners.keydown.length, 0); + v.assert.equal(target.listeners.keyup.length, 0); + + target.press('keydown', 'Enter'); + v.assert.equal(onEnter.mock.calls.length, 0); + }); +}); From 4f265e16cd6a12f1799bb5c91798f8173669f4df Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Thu, 17 Sep 2026 16:08:25 -0400 Subject: [PATCH 2/2] test(focus): check the target binding without dispatching on document The suite runs its files without isolation, so a document key listener left by another file in the same worker reached the focused element and failed the assertion on CI. Assert that useFocusManager registers no key listener on document instead, which is the property under test. Co-Authored-By: Claude Fable 5.1 --- tests/focusManagerTarget.test.tsx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/focusManagerTarget.test.tsx b/tests/focusManagerTarget.test.tsx index b53f056a..d64a0fdd 100644 --- a/tests/focusManagerTarget.test.tsx +++ b/tests/focusManagerTarget.test.tsx @@ -60,12 +60,21 @@ v.describe('useFocusManager event target', () => { }); v.test('leaves document alone when a target is given', async () => { + // Test files share this worker's module state (`isolate: false`), and + // other files bind the focus manager to document, so a key dispatched on + // document is not a clean signal here. Check the registration itself. + const addEventListener = v.vi.spyOn(document, 'addEventListener'); const target = new FakeTarget(); - const { onEnter, dispose } = await setup(target); + const { dispose } = await setup(target); - document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); - v.assert.equal(onEnter.mock.calls.length, 0); + const keyRegistrations = addEventListener.mock.calls.filter( + ([type]) => type === 'keydown' || type === 'keyup', + ); + v.assert.equal(keyRegistrations.length, 0); + v.assert.equal(target.listeners.keydown.length, 1); + v.assert.equal(target.listeners.keyup.length, 1); + addEventListener.mockRestore(); dispose(); });