From cd18eafa7f4a3d18d49b6851e9c8b026cd0a806a Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Thu, 17 Sep 2026 23:40:15 -0400 Subject: [PATCH] feat(focus): preventDefault for a consumed key, behind a config flag A host with no browser between the remote and the app decides for itself what an unhandled press does: on tvOS a Menu press nothing handled returns to the Home screen. It needs to know which presses the app consumed, and until now only a handler calling `preventDefault()` itself could say so. `Config.preventDefaultOnHandledKeys`, off by default, makes the focus manager call it on every key event the app consumed: a handler returned true, or the focus manager dropped the press itself, a throttled press or a suppressed repeat. `KeyEventLike` gains an optional `preventDefault`. Also a compatibility fix: 1.6.3 reads the second `useFocusManager` argument as the event target, where earlier releases ignored it. An app still passing the removed hold options there threw on `target.addEventListener`. An argument that cannot listen is ignored again, in favour of `document`. Co-Authored-By: Claude Fable 5.1 --- docs/essentials/render.md | 3 + docs/primitives/useFocusManager.md | 23 ++++ src/core/config.ts | 10 ++ src/core/focusManager.ts | 63 ++++++++--- tests/focusManagerPreventDefault.test.tsx | 132 ++++++++++++++++++++++ tests/focusManagerTarget.test.tsx | 23 ++++ 6 files changed, 241 insertions(+), 13 deletions(-) create mode 100644 tests/focusManagerPreventDefault.test.tsx diff --git a/docs/essentials/render.md b/docs/essentials/render.md index b50ce13b..c11b2469 100644 --- a/docs/essentials/render.md +++ b/docs/essentials/render.md @@ -146,6 +146,9 @@ Besides `rendererOptions`, the `Config` object exposes several properties specif Allows simple CSS-like transition properties without full engine overhead. - **throttleInput**: `number` Rate-limiting for key handling in milliseconds. +- **preventDefaultOnHandledKeys**: `boolean` (Default: `false`) + Calls `preventDefault()` on every key event the app consumed, for a host that + passes unhandled presses to the system. See [useFocusManager](/primitives/useFocusManager.md#consumed-keys-configpreventdefaultonhandledkeys). - **lockStyles**: `boolean` (Default: `true`) Enables locking on styles to prevent unintended overrides. - **convertToShader**: `(node: ElementNode, v: StyleEffects) => IRendererShader` diff --git a/docs/primitives/useFocusManager.md b/docs/primitives/useFocusManager.md index 79f5f5e4..41b954f3 100644 --- a/docs/primitives/useFocusManager.md +++ b/docs/primitives/useFocusManager.md @@ -42,6 +42,9 @@ useFocusManager(undefined, keyBridge); ``` Browser apps are unaffected: leave the argument out and `document` is used. +Anything passed there that cannot listen is ignored in favour of `document` +too, so an app still passing the removed hold options object (see below) +keeps working. ### Focus Path Tracking @@ -114,6 +117,26 @@ For more granular control, you can add a `throttleInput` property directly to an ... ``` +### Consumed Keys (`Config.preventDefaultOnHandledKeys`) + +A host with no browser between the remote and the app decides for itself what +an unhandled press does: on tvOS, a Menu press nothing handled returns to the +Home screen. Such a host needs to know which presses the app consumed. Set +`Config.preventDefaultOnHandledKeys` and the focus manager calls +`preventDefault()` on every key event the app consumed: a handler returned +`true`, or the focus manager dropped the press itself (a throttled press, a +repeat suppressed by `useHold`). An event without a `preventDefault` method is +left alone. + +```javascript +import { Config } from '@solidtv/solid'; + +Config.preventDefaultOnHandledKeys = true; +``` + +It is off by default, since in a browser it changes what handled keys do: a +handled arrow no longer scrolls the page, say. + ### Focus History Logging Focus history logging records each focus change — whether triggered by a key press or programmatically — into a ring buffer of up to 50 entries. This is a **dev-only** feature: recording and printing are both gated behind the `isDev` flag and do nothing in production builds. diff --git a/src/core/config.ts b/src/core/config.ts index 4970f0b4..b8173c53 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -73,6 +73,15 @@ export interface Config { lockStyles?: boolean; fontWeightAlias?: Record; throttleInput?: number; + /** + * Call `preventDefault()` on every key event the app consumed: a handler + * returned `true`, or the focus manager dropped the press itself (a + * throttled press, a suppressed repeat). A host that hands the system + * whatever the app leaves unhandled, such as the Menu button on tvOS, + * reads it to tell the two apart. Off by default: in a browser it would + * also stop the default action of every handled key. + */ + preventDefaultOnHandledKeys: boolean; taskDelay?: number; convertToShader: (_node: ElementNode, v: StyleEffects) => IRendererShader; stateOrder?: DollarString[]; @@ -106,6 +115,7 @@ export const Config: Config = { }, focusStateKey: '$focus', lockStyles: true, + preventDefaultOnHandledKeys: false, rendererOptions: {}, stateOrder: [], }; diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index 21599424..27bb4372 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -397,7 +397,10 @@ const propagateKeyPress = ( `Keypress throttled by global Config.throttleInput: ${Config.throttleInput}ms`, ); } - return false; + // Dropped on purpose, so consumed: the same answer an element's own + // throttleInput gives below, and what a host asking through + // Config.preventDefaultOnHandledKeys needs to hear. + return true; } lastGlobalKeyPressTime = currentTime; } @@ -529,33 +532,40 @@ export const releaseKeySuppression = ( liftSuppression(keyIdentities(keyOrEvent)); }; -const handleKeyEvents = (keydown?: KeyboardEvent, keyup?: KeyboardEvent) => { +// Returns whether the app consumed the event: a handler took it, or the focus +// manager dropped it on purpose (a suppressed repeat, a throttled press). +const handleKeyEvents = ( + keydown?: KeyboardEvent, + keyup?: KeyboardEvent, +): boolean => { if (keydown) { const ids = keyIdentities(keydown); if (keydown.repeat) { - if (findSuppression(ids)) return; + if (findSuppression(ids)) return true; } else { // A fresh press starts a new gesture, so the previous one is over even // though its key-up never arrived. Settle it before handling this press. liftSuppression(ids); } - propagateKeyPress( + return propagateKeyPress( keydown, keyMapEntries[keydown.key] || keyMapEntries[keydown.keyCode], ); - } else if (keyup) { + } + if (keyup) { // The key is up: whatever was suppressing its repeats is done. Settle it // before propagating, so a suppressor that is still in the focus path sees // its own release callback rather than a second one via the key-up below. liftSuppression(keyIdentities(keyup)); - propagateKeyPress( + return propagateKeyPress( keyup, keyMapEntries[keyup.key] || keyMapEntries[keyup.keyCode], true, ); } + return false; }; /** @@ -567,6 +577,12 @@ export interface KeyEventLike { readonly key: string; readonly keyCode: number; readonly repeat: boolean; + /** + * Called on an event the app consumed when + * `Config.preventDefaultOnHandledKeys` is set; optional, since a host's + * own event objects need not have it. + */ + preventDefault?(): void; } /** @@ -587,12 +603,19 @@ export interface KeyEventTarget { export const useFocusManager = ( userKeyMap?: Partial, - target: KeyEventTarget = document, + target?: KeyEventTarget, ) => { if (userKeyMap) { flattenKeyMap(userKeyMap, keyMapEntries); } + // Before the target parameter existed the second argument was ignored, and + // an object that cannot listen (the removed hold options, say) still is. + const eventTarget: KeyEventTarget = + target !== undefined && isFunction(target.addEventListener) + ? target + : document; + // Capture the calling owner so signal updates and key-event reactions // can run inside it — needed for programmatic .setFocus(), post-mutation // focus, and any effect subscribers that rely on onCleanup. @@ -610,16 +633,30 @@ export const useFocusManager = ( // 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)); + ownerContext(() => { + if ( + handleKeyEvents(event as KeyboardEvent, undefined) && + Config.preventDefaultOnHandledKeys + ) { + event.preventDefault?.(); + } + }); const keyUpHandler = (event: KeyEventLike) => - ownerContext(() => handleKeyEvents(undefined, event as KeyboardEvent)); + ownerContext(() => { + if ( + handleKeyEvents(undefined, event as KeyboardEvent) && + Config.preventDefaultOnHandledKeys + ) { + event.preventDefault?.(); + } + }); - target.addEventListener('keydown', keyPressHandler); - target.addEventListener('keyup', keyUpHandler); + eventTarget.addEventListener('keydown', keyPressHandler); + eventTarget.addEventListener('keyup', keyUpHandler); onCleanup(() => { - target.removeEventListener('keydown', keyPressHandler); - target.removeEventListener('keyup', keyUpHandler); + eventTarget.removeEventListener('keydown', keyPressHandler); + eventTarget.removeEventListener('keyup', keyUpHandler); suppressedKeys.clear(); }); }; diff --git a/tests/focusManagerPreventDefault.test.tsx b/tests/focusManagerPreventDefault.test.tsx new file mode 100644 index 00000000..c6e48fe5 --- /dev/null +++ b/tests/focusManagerPreventDefault.test.tsx @@ -0,0 +1,132 @@ +import * as v from 'vitest'; +import { Config } from '@solidtv/solid'; +import { + useFocusManager, + type KeyEventLike, + type KeyEventTarget, +} from '@solidtv/solid/primitives'; +import { renderer, waitForUpdate } from './setup.js'; + +type Listener = (event: KeyEventLike) => void; + +// A host's event target whose events carry preventDefault, so a test can see +// whether the focus manager called it. +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 preventDefault = v.vi.fn(); + const event: KeyEventLike = { + key, + keyCode: 0, + repeat: false, + preventDefault, + }; + for (const listener of this.listeners[type].slice()) listener(event); + return preventDefault; + } +} + +interface Handlers { + onEnter?: () => boolean | undefined; + onEnterRelease?: () => boolean | undefined; + onRight?: () => boolean | undefined; +} + +async function setup(target: FakeTarget, handlers: Handlers) { + const dispose = renderer.render(() => { + useFocusManager(undefined, target); + return ( + + ); + }) as unknown as () => void; + await waitForUpdate(); + return dispose; +} + +v.describe('Config.preventDefaultOnHandledKeys', () => { + v.afterEach(() => { + Config.preventDefaultOnHandledKeys = false; + Config.throttleInput = undefined; + }); + + v.test('calls preventDefault on a press a handler consumed', async () => { + Config.preventDefaultOnHandledKeys = true; + const target = new FakeTarget(); + const dispose = await setup(target, { onEnter: () => true }); + + const preventDefault = target.press('keydown', 'Enter'); + v.assert.equal(preventDefault.mock.calls.length, 1); + + dispose(); + }); + + v.test('leaves a press no handler took alone', async () => { + Config.preventDefaultOnHandledKeys = true; + const target = new FakeTarget(); + const dispose = await setup(target, { onEnter: () => undefined }); + + const preventDefault = target.press('keydown', 'Enter'); + v.assert.equal(preventDefault.mock.calls.length, 0); + + dispose(); + }); + + v.test('does nothing while the flag is off', async () => { + const target = new FakeTarget(); + const dispose = await setup(target, { onEnter: () => true }); + + const preventDefault = target.press('keydown', 'Enter'); + v.assert.equal(preventDefault.mock.calls.length, 0); + + dispose(); + }); + + v.test('covers a release a handler consumed', async () => { + Config.preventDefaultOnHandledKeys = true; + const target = new FakeTarget(); + const dispose = await setup(target, { onEnterRelease: () => true }); + + target.press('keydown', 'Enter'); + const preventDefault = target.press('keyup', 'Enter'); + v.assert.equal(preventDefault.mock.calls.length, 1); + + dispose(); + }); + + v.test('treats a press the global throttle dropped as consumed', async () => { + const onRight = v.vi.fn(() => true); + const target = new FakeTarget(); + const dispose = await setup(target, { onRight }); + // The throttle compares against the previous key, whatever an earlier + // test left there: make it a different one before turning it on. + target.press('keydown', 'Enter'); + Config.preventDefaultOnHandledKeys = true; + Config.throttleInput = 10000; + + target.press('keydown', 'ArrowRight'); + const preventDefault = target.press('keydown', 'ArrowRight'); + v.assert.equal(onRight.mock.calls.length, 1); + v.assert.equal(preventDefault.mock.calls.length, 1); + + dispose(); + }); +}); diff --git a/tests/focusManagerTarget.test.tsx b/tests/focusManagerTarget.test.tsx index d64a0fdd..30217383 100644 --- a/tests/focusManagerTarget.test.tsx +++ b/tests/focusManagerTarget.test.tsx @@ -89,4 +89,27 @@ v.describe('useFocusManager event target', () => { target.press('keydown', 'Enter'); v.assert.equal(onEnter.mock.calls.length, 0); }); + + v.test( + 'listens on document when the second argument cannot listen', + async () => { + // Before the target parameter existed the argument was ignored; an app + // still passing the removed hold options there must keep working. + const addEventListener = v.vi.spyOn(document, 'addEventListener'); + const legacyOptions = { userKeyHoldMap: {}, holdThreshold: 1000 }; + const dispose = renderer.render(() => { + useFocusManager(undefined, legacyOptions as unknown as KeyEventTarget); + return ; + }) as unknown as () => void; + await waitForUpdate(); + + const keyRegistrations = addEventListener.mock.calls.filter( + ([type]) => type === 'keydown' || type === 'keyup', + ); + v.assert.equal(keyRegistrations.length, 2); + + addEventListener.mockRestore(); + dispose(); + }, + ); });