From 6f29b65fbd9f8cda043f08d08e2736cb9a6025e9 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 20:20:06 -0400 Subject: [PATCH 01/10] fix(useHold): suppress trailing repeats and deliver onRelease after a hold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hold action typically moves focus while the key is still physically down (hold OK to open a context menu). Two things then went wrong, and neither could be fixed by the element owning the hold, since it is no longer in the focus path: the platform's trailing auto-repeat key-downs propagated down the *new* focus path and fired whatever just took focus, and the key-up went there too, so onRelease never ran. Both are regressions from the legacy `userKeyHoldMap` path, which got the same protection for free by returning early from every key-down for a hold-mapped key. Add a suppression latch to the focus manager, which owns propagation: `suppressKeyUntilRelease(keyOrEvent, onRelease?)` drops auto-repeat key-downs for a key and delivers a callback when the key is finally released, wherever focus has moved. Suppression lifts on key-up or on the next non-repeat key-down, so a swallowed key-up (webOS) cannot wedge a key; non-repeat key-downs are never suppressed. useHold latches when its hold fires. Also: - Add `holdRequiresRepeat` (default true, preserving current behavior). Hold detection reads `e.repeat`, so on a platform delivering neither key-up nor auto-repeat a long press resolved as a tap and a hold was unreachable — the opposite of what `keyHoldOptions` did. Setting it false resolves that ambiguous case as a hold. - Pass the originating KeyboardEvent and elements through to onHold, onEnter and onRelease, so call sites no longer stash KeyHandler args in a closure. - Annotate the return as a tuple rather than letting it infer an array, and document the KeyboardEvent parameter that hold detection depends on. - Document that startHold ends the bubble phase, so ancestor handlers for the key do not run and the deferred tap cannot be handed back to them. Co-Authored-By: Claude Opus 5 --- docs/primitives/useHold.md | 123 +++++++++++++++++++++++++++-- src/core/focusManager.ts | 73 +++++++++++++++++- src/primitives/useFocusManager.ts | 2 + src/primitives/useHold.ts | 103 +++++++++++++++++++++---- tests/keySuppression.test.tsx | 91 ++++++++++++++++++++++ tests/useHold.spec.ts | 124 +++++++++++++++++++++++++++++- 6 files changed, 488 insertions(+), 28 deletions(-) create mode 100644 tests/keySuppression.test.tsx diff --git a/docs/primitives/useHold.md b/docs/primitives/useHold.md index 628d739..f6772ff 100644 --- a/docs/primitives/useHold.md +++ b/docs/primitives/useHold.md @@ -26,25 +26,52 @@ const [holdRight, releaseRight] = useHold({ #### `UseHoldProps` -| Prop | Type | Description | Default | -| --------------------------- | ------------ | --------------------------------------------------------------------- | ------------ | -| `onHold` | `() => void` | Called once the hold threshold is exceeded. | **Required** | -| `onEnter` | `() => void` | Called on press or key entry. May be delayed depending on config. | **Required** | -| `onRelease` | `() => void` | Called after a successful hold is released. | `undefined` | -| `holdThreshold` | `number` | Time in milliseconds to wait before triggering `onHold`. | `500` | -| `performOnEnterImmediately` | `boolean` | Whether `onEnter` is triggered immediately or only if released early. | `false` | +| Prop | Type | Description | Default | +| --------------------------- | -------------- | --------------------------------------------------------------------------------------------------------------------------------- | ------------ | +| `onHold` | `HoldCallback` | Called once the hold threshold is exceeded. | **Required** | +| `onEnter` | `HoldCallback` | Called on press or key entry. May be delayed depending on config. | **Required** | +| `onRelease` | `HoldCallback` | Called after a successful hold is released. | `undefined` | +| `holdThreshold` | `number` | Time in milliseconds to wait before triggering `onHold`. | `500` | +| `performOnEnterImmediately` | `boolean` | Whether `onEnter` is triggered immediately or only if released early. | `false` | +| `holdRequiresRepeat` | `boolean` | Whether a hold must be confirmed by an auto-repeat key-down. See [Platforms without auto-repeat](#platforms-without-auto-repeat). | `true` | + +Each callback receives the same context a `KeyHandler` gets — the `KeyboardEvent` +that began the press, the element whose handler ran, and the focused element: + +```ts +type HoldCallback = ( + e?: KeyboardEvent, + target?: ElementNode, + handlerElm?: ElementNode, +) => void; +``` + +For `onHold` and for timer-resolved `onEnter`, this is the context captured from +the originating key-down, since those fire from a timer with no event of their +own. --- ### Returns ```ts -[startHold, releaseHold]: [() => boolean, () => boolean] +[startHold, releaseHold]: [HoldHandler, HoldHandler] + +type HoldHandler = ( + e?: KeyboardEvent, + target?: ElementNode, + handlerElm?: ElementNode, +) => boolean; ``` - `startHold`: Call this on a key/button press. Starts the hold timer and conditionally calls `onEnter`. - `releaseHold`: Call this on a key/button release. Stops the timer and calls `onEnter` or `onRelease` depending on how long it was held. +> **`startHold` needs the event.** It reads `e.repeat` to detect a hold. Passed +> directly as a key handler (`onRight={holdRight}`) it receives one. If you wrap +> it, forward all of the arguments — a wrapper that drops the event yields a +> primitive that can never detect a hold, silently, and typically only on device. + --- ### Behavior Summary @@ -71,3 +98,83 @@ const [onHoldEnter, onHoldRelease] = useHold({ ; ``` + +--- + +### Holds that move focus + +The canonical hold — hold OK to open a context menu — moves focus while the key +is still physically down. The platform keeps emitting auto-repeat key-downs after +`onHold` fires, and those would otherwise propagate down the **new** focus path +and fire whatever just took focus. + +`useHold` handles this by latching the key in the focus manager once `onHold` +fires. Remaining auto-repeats are dropped until the key is released, and the +latch delivers `onRelease` even though the key-up now propagates somewhere the +holding element can no longer see. Suppression lifts on key-up, or on the next +fresh (non-repeat) key-down so a swallowed key-up cannot wedge a key. + +Non-repeat key-downs are never suppressed, so a real press always gets through. + +If you implement hold behavior without this primitive, the same latch is +available directly: + +```ts +import { + suppressKeyUntilRelease, + releaseKeySuppression, +} from '@solidtv/solid/primitives'; + +suppressKeyUntilRelease(event, () => console.log('key released')); +``` + +--- + +### Platforms without auto-repeat + +A hold is confirmed by an auto-repeat key-down. If a press delivers neither a +key-up nor an auto-repeat by the threshold, the press is ambiguous, and by +default it resolves as a **tap** — the behavior that keeps taps working on +remotes that swallow key-up. + +On a platform whose remote input layer delivers no auto-repeat at all, that makes +a hold unreachable. Set `holdRequiresRepeat: false` to resolve the ambiguous case +as a hold instead, matching the legacy `keyHoldOptions` behavior: + +```tsx +const [holdEnter, releaseEnter] = useHold({ + onHold: openContextMenu, + onEnter: openTile, + holdRequiresRepeat: false, // no auto-repeat on this platform +}); +``` + +An early key-up still resolves as a tap either way, so this only changes the +no-key-up **and** no-repeat case. + +--- + +### Caveat: `startHold` stops propagation + +`startHold` returns `true`, which ends the focus manager's bubble phase — +**ancestor handlers for that key will not run.** Attaching `useHold` to a row +removes that row's subtree from every ancestor `onEnter`. + +This is structural rather than incidental: whether the press was a tap isn't +known until key-up or until the timer fires, by which point the propagation pass +is long over, so a deferred tap cannot be handed back to ancestors. + +If an ancestor performs work on that key — a root-level handler resolving an +`href`, or a container doing analytics — invoke it from `onEnter` yourself: + +```ts +const onEnter: HoldCallback = (e, target, focused) => { + for (let elm = target?.parent; elm; elm = elm.parent) { + if ( + typeof elm.onEnter === 'function' && + elm.onEnter(e, elm, focused) === true + ) + return; + } +}; +``` diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index e151c19..935083a 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -448,13 +448,77 @@ const propagateKeyPress = ( const DEFAULT_KEY_HOLD_THRESHOLD = 500; // ms const keyHoldTimeouts: { [key: KeyNameOrKeyCode]: number | true } = {}; +const keyOf = (e: KeyboardEvent): KeyNameOrKeyCode => e.key || e.keyCode; + +// Keys whose auto-repeat key-downs are dropped before propagation, mapped to an +// optional callback run when the key is finally released. +// +// A hold action typically moves focus (opening a context menu, say) while the +// key is still physically down. Two things then go wrong, and neither can be +// fixed by the element that owns the hold, because it is no longer in the focus +// path: the trailing auto-repeats propagate down the *new* focus path and fire +// whatever just took focus, and the key-up goes there too, so the owner's +// release handler never runs. +// +// Both are propagation concerns, so the latch lives here. The release callback +// is what lets a suppressor still learn about a key-up it can no longer receive +// through the focus path. +const suppressedKeys = new Map void) | undefined>(); + +const liftSuppression = (key: KeyNameOrKeyCode): void => { + const onRelease = suppressedKeys.get(key); + suppressedKeys.delete(key); + onRelease?.(); +}; + +/** + * Drop auto-repeat key-downs for `keyOrEvent` until the key is released. + * + * Suppression is lifted by the key's key-up, or by the next fresh (non-repeat) + * key-down — the latter so platforms that swallow key-up (webOS) can't wedge a + * key permanently. Non-repeat key-downs are never suppressed. `onRelease` runs + * when suppression lifts, whichever way it lifts, and is delivered regardless of + * where focus has moved in the meantime. + * + * `useHold` calls this itself when a hold fires; call it directly only when + * implementing hold behavior outside that primitive. + */ +export const suppressKeyUntilRelease = ( + keyOrEvent: KeyboardEvent | KeyNameOrKeyCode, + onRelease?: () => void, +): void => { + suppressedKeys.set( + typeof keyOrEvent === 'object' ? keyOf(keyOrEvent) : keyOrEvent, + onRelease, + ); +}; + +/** + * Lift suppression added by {@link suppressKeyUntilRelease} early, running its + * release callback. + */ +export const releaseKeySuppression = ( + keyOrEvent: KeyboardEvent | KeyNameOrKeyCode, +): void => { + liftSuppression( + typeof keyOrEvent === 'object' ? keyOf(keyOrEvent) : keyOrEvent, + ); +}; + const handleKeyEvents = ( delay: number, keydown?: KeyboardEvent, keyup?: KeyboardEvent, ) => { if (keydown) { - const key: KeyNameOrKeyCode = keydown.key || keydown.keyCode; + const key: KeyNameOrKeyCode = keyOf(keydown); + if (keydown.repeat) { + if (suppressedKeys.has(key)) return; + } else if (suppressedKeys.has(key)) { + // 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(key); + } const mappedKeyHoldEvent = keyHoldMapEntries[keydown.key] || keyHoldMapEntries[keydown.keyCode]; const mappedKeyEvent = @@ -471,7 +535,11 @@ const handleKeyEvents = ( propagateKeyPress(keydown, mappedKeyEvent, false); } else if (keyup) { - const key: KeyNameOrKeyCode = keyup.key || keyup.keyCode; + const key: KeyNameOrKeyCode = keyOf(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. + if (suppressedKeys.has(key)) liftSuppression(key); const mappedKeyEvent = keyMapEntries[keyup.key] || keyMapEntries[keyup.keyCode]; if (keyHoldTimeouts[key] === true) { @@ -529,5 +597,6 @@ export const useFocusManager = ( for (const timeout of Object.values(keyHoldTimeouts)) { if (timeout && timeout !== true) clearTimeout(timeout); } + suppressedKeys.clear(); }); }; diff --git a/src/primitives/useFocusManager.ts b/src/primitives/useFocusManager.ts index 6e10221..f1c7298 100644 --- a/src/primitives/useFocusManager.ts +++ b/src/primitives/useFocusManager.ts @@ -2,6 +2,8 @@ export { focusPath, useFocusManager, setActiveElementCore, + suppressKeyUntilRelease, + releaseKeySuppression, type KeyMap, type KeyHoldOptions, } from '../core/focusManager.js'; diff --git a/src/primitives/useHold.ts b/src/primitives/useHold.ts index 947ddce..9442f20 100644 --- a/src/primitives/useHold.ts +++ b/src/primitives/useHold.ts @@ -1,11 +1,41 @@ import { createMemo } from 'solid-js'; +import { suppressKeyUntilRelease } from '../core/focusManager.js'; +import type { ElementNode } from '../core/elementNode.js'; + +/** + * A hold callback. Receives the `KeyboardEvent` that began the press, the + * element whose handler was invoked, and the focused (leaf) element at that + * moment — the same arguments a `KeyHandler` gets. All three are `undefined` + * when `startHold` was called without them. + */ +export type HoldCallback = ( + e?: KeyboardEvent, + target?: ElementNode, + handlerElm?: ElementNode, +) => void; + +export type HoldHandler = ( + e?: KeyboardEvent, + target?: ElementNode, + handlerElm?: ElementNode, +) => boolean; export type UseHoldProps = { - onHold: () => void; - onEnter: () => void; - onRelease?: () => void; + onHold: HoldCallback; + onEnter: HoldCallback; + onRelease?: HoldCallback; holdThreshold?: number; performOnEnterImmediately?: boolean; + /** + * Whether a hold must be confirmed by an auto-repeat key-down. + * + * `true` (default): a press with neither key-up nor auto-repeat by the + * threshold resolves as a tap. `false`: it resolves as a hold, matching the + * legacy `keyHoldOptions` behavior. Set this to `false` on platforms whose + * remote input layer delivers no auto-repeat, where a hold gesture would + * otherwise be unreachable. + */ + holdRequiresRepeat?: boolean; }; /** @@ -23,10 +53,23 @@ export type UseHoldProps = { * - if neither key-up nor auto-repeat arrives, the timer resolves to a tap → * `onEnter` after `holdThreshold` ms. This is the key-up-independent path that * keeps taps working on webOS, at the cost of ~`holdThreshold` ms latency. + * Set `holdRequiresRepeat: false` to resolve this ambiguous case as a hold + * instead, on platforms that deliver no auto-repeat at all. + * + * Once `onHold` fires the key is still physically down, so the focus manager + * drops its remaining auto-repeats until it is released. Without that, a hold + * that moves focus — the canonical case, opening a context menu — would have + * its trailing repeats fire whatever it just focused. The same latch delivers + * `onRelease` even though the key-up now propagates somewhere else entirely. * * `performOnEnterImmediately` keeps the legacy behavior of firing `onEnter` on * key-down; a long-press then fires both `onEnter` and `onHold`. * + * Note that `startHold` returns `true`, which ends the bubble phase: ancestor + * handlers for that key will not run, and whether the press was a tap isn't + * known until after propagation is over. If an ancestor performs navigation, + * invoke it from `onEnter` yourself. See the caveat in the docs. + * * @example * const [holdRight, releaseRight] = useHold({ * onHold: handleHoldRight, @@ -40,21 +83,31 @@ export type UseHoldProps = { * onRightRelease={releaseRight} * /> * + * `startHold` reads `e.repeat` to detect a hold. Used directly as a `KeyHandler` + * (above) it gets the event; if you wrap it, forward all of the arguments, or + * hold detection silently never triggers. + * * @param {UseHoldProps} props - The properties for configuring the hold behavior. - * @returns {[(e?: KeyboardEvent) => boolean, () => boolean]} A tuple containing `startHold` and `releaseHold` functions. + * @returns {[HoldHandler, HoldHandler]} A tuple of `startHold` and `releaseHold`. */ -export function useHold(props: UseHoldProps) { +export function useHold(props: UseHoldProps): [HoldHandler, HoldHandler] { const holdThreshold = createMemo(() => props.holdThreshold ?? 500); const performOnEnterImmediately = createMemo( () => props.performOnEnterImmediately ?? false, ); + const holdRequiresRepeat = createMemo(() => props.holdRequiresRepeat ?? true); let holdTimeout = -1; let enterFired = false; // onEnter already fired for this press let holdFired = false; // onHold already fired for this press let repeated = false; // an auto-repeat key-down was seen (key still held) + // Context from the key-down that began the press, replayed to whichever + // callback resolves it — the timer path has no event of its own, and after a + // focus-moving hold neither does the release. + let press: Parameters = []; + const reset = () => { if (holdTimeout !== -1) { clearTimeout(holdTimeout); @@ -63,9 +116,17 @@ export function useHold(props: UseHoldProps) { enterFired = false; holdFired = false; repeated = false; + press = []; }; - const startHold = (e?: KeyboardEvent) => { + const fireRelease = () => { + if (!holdFired) return; + const args = press; + reset(); // before the callback, so a re-entrant press isn't clobbered + props.onRelease?.(...args); + }; + + const startHold: HoldHandler = (e, target, handlerElm) => { // Auto-repeat key-down: the key is still held. Record it so the timer // resolves to a hold even if the key-up event never arrives (webOS). if (e?.repeat) { @@ -76,30 +137,35 @@ export function useHold(props: UseHoldProps) { // Fresh key-down begins a new press. Reset first so a previous press whose // key-up was never delivered doesn't leave us wedged for this one. reset(); + press = [e, target, handlerElm]; if (performOnEnterImmediately()) { enterFired = true; - props.onEnter(); + props.onEnter(...press); } holdTimeout = setTimeout(() => { holdTimeout = -1; - if (repeated) { + if (repeated || !holdRequiresRepeat()) { // Held past the threshold → hold gesture. holdFired = true; - props.onHold(); + // The key is still down and will keep repeating. Drop those repeats so + // they can't reach whatever onHold focuses, and route the key-up back + // here even if this element is no longer in the focus path. + if (e) suppressKeyUntilRelease(e, fireRelease); + props.onHold(...press); } else if (!enterFired) { // No key-up and no auto-repeat arrived: resolve as a tap so the // primary action still fires on remotes that swallow key-up. enterFired = true; - props.onEnter(); + props.onEnter(...press); } }, holdThreshold()) as unknown as number; return true; }; - const releaseHold = () => { + const releaseHold: HoldHandler = (e, target, handlerElm) => { if (holdTimeout !== -1) { // Released before the threshold → tap. Fires immediately where key-up is // delivered, avoiding the timer latency. @@ -107,12 +173,19 @@ export function useHold(props: UseHoldProps) { holdTimeout = -1; if (!enterFired) { enterFired = true; - props.onEnter(); + props.onEnter( + e ?? press[0], + target ?? press[1], + handlerElm ?? press[2], + ); } - } else if (holdFired) { - props.onRelease?.(); + reset(); + } else { + // After a hold, the suppression latch has usually already fired onRelease + // and reset — `holdFired` is false and this is a no-op. + fireRelease(); + reset(); } - reset(); return true; }; diff --git a/tests/keySuppression.test.tsx b/tests/keySuppression.test.tsx new file mode 100644 index 0000000..2a55b2b --- /dev/null +++ b/tests/keySuppression.test.tsx @@ -0,0 +1,91 @@ +import * as v from 'vitest'; +import * as lng from '@solidtv/solid'; +import { + useFocusManager, + suppressKeyUntilRelease, + releaseKeySuppression, +} from '@solidtv/solid/primitives'; +import { renderer, waitForUpdate } from './setup.js'; + +const keydown = (repeat = false) => + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', repeat })); +const keyup = () => + document.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); + +async function setup() { + const onEnter = v.vi.fn(); + let dispose!: () => void; + dispose = renderer.render(() => { + useFocusManager(); + return ; + }) as unknown as () => void; + await waitForUpdate(); + return { onEnter, dispose }; +} + +v.describe('key suppression', () => { + v.afterEach(() => releaseKeySuppression('Enter')); + + v.test('drops auto-repeat key-downs while a key is suppressed', async () => { + const { onEnter, dispose } = await setup(); + + keydown(); + v.assert.equal(onEnter.mock.calls.length, 1); + + // A hold fired and latched the key — the repeats that follow must not + // propagate, even though this element is still focused. + suppressKeyUntilRelease('Enter'); + keydown(true); + keydown(true); + v.assert.equal(onEnter.mock.calls.length, 1); + + dispose(); + }); + + v.test('key-up lifts suppression and runs the release callback', async () => { + const { onEnter, dispose } = await setup(); + const onRelease = v.vi.fn(); + + keydown(); + suppressKeyUntilRelease('Enter', onRelease); + keydown(true); + v.assert.equal(onRelease.mock.calls.length, 0); + + keyup(); + v.assert.equal(onRelease.mock.calls.length, 1); + + // Suppression is gone, so repeats propagate again. + keydown(true); + v.assert.equal(onEnter.mock.calls.length, 2); + + dispose(); + }); + + v.test('a fresh key-down lifts suppression when key-up never arrives', async () => { + const { onEnter, dispose } = await setup(); + const onRelease = v.vi.fn(); + + keydown(); + suppressKeyUntilRelease('Enter', onRelease); + keydown(true); + v.assert.equal(onEnter.mock.calls.length, 1); + + // webOS: no key-up is ever delivered. The next real press must not be + // swallowed, and must settle the abandoned one. + keydown(); + v.assert.equal(onRelease.mock.calls.length, 1); + v.assert.equal(onEnter.mock.calls.length, 2); + + dispose(); + }); + + v.test('never suppresses non-repeat key-downs', async () => { + const { onEnter, dispose } = await setup(); + + suppressKeyUntilRelease('Enter'); + keydown(); + v.assert.equal(onEnter.mock.calls.length, 1); + + dispose(); + }); +}); diff --git a/tests/useHold.spec.ts b/tests/useHold.spec.ts index 7c11625..67d224c 100644 --- a/tests/useHold.spec.ts +++ b/tests/useHold.spec.ts @@ -1,8 +1,20 @@ import { createRoot } from 'solid-js'; import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { useHold } from '../src/primitives/useHold.ts'; +import type { ElementNode } from '../src/core/elementNode.ts'; -const downRepeat = { repeat: true } as KeyboardEvent; +const suppressKeyUntilRelease = + vi.fn<(e: KeyboardEvent | string | number, onRelease?: () => void) => void>(); +vi.mock('../src/core/focusManager.ts', () => ({ + suppressKeyUntilRelease: (...args: unknown[]) => + (suppressKeyUntilRelease as (...a: unknown[]) => void)(...args), +})); + +const { useHold } = await import('../src/primitives/useHold.ts'); + +const down = { key: 'Enter', repeat: false } as KeyboardEvent; +const downRepeat = { key: 'Enter', repeat: true } as KeyboardEvent; +const target = { id: 'target' } as unknown as ElementNode; +const handlerElm = { id: 'handlerElm' } as unknown as ElementNode; function setup(props: Partial[0]> = {}) { const onEnter = vi.fn(); @@ -18,7 +30,10 @@ function setup(props: Partial[0]> = {}) { } describe('useHold', () => { - beforeEach(() => vi.useFakeTimers()); + beforeEach(() => { + vi.useFakeTimers(); + suppressKeyUntilRelease.mockClear(); + }); afterEach(() => vi.useRealTimers()); it('fires onEnter immediately on key-up before the threshold (tap)', () => { @@ -90,4 +105,107 @@ describe('useHold', () => { expect(onEnter).toHaveBeenCalledTimes(1); // not double-fired dispose(); }); + + describe('holdRequiresRepeat', () => { + it('resolves a repeat-less press as a tap by default', () => { + const { startHold, onEnter, onHold, dispose } = setup(); + startHold(down); // no repeat, no key-up + vi.advanceTimersByTime(200); + expect(onEnter).toHaveBeenCalledTimes(1); + expect(onHold).not.toHaveBeenCalled(); + dispose(); + }); + + it('resolves a repeat-less press as a hold when false', () => { + const { startHold, onEnter, onHold, dispose } = setup({ + holdRequiresRepeat: false, + }); + startHold(down); // platform delivers neither key-up nor auto-repeat + vi.advanceTimersByTime(200); + expect(onHold).toHaveBeenCalledTimes(1); + expect(onEnter).not.toHaveBeenCalled(); + dispose(); + }); + + it('still resolves an early key-up as a tap when false', () => { + const { startHold, releaseHold, onEnter, onHold, dispose } = setup({ + holdRequiresRepeat: false, + }); + startHold(down); + releaseHold(); + expect(onEnter).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(200); + expect(onHold).not.toHaveBeenCalled(); + dispose(); + }); + }); + + describe('key suppression after a hold', () => { + it('suppresses the held key and fires onRelease via the latch', () => { + const { startHold, onHold, onRelease, dispose } = setup(); + startHold(down); + startHold(downRepeat); + vi.advanceTimersByTime(200); + expect(onHold).toHaveBeenCalledTimes(1); + expect(suppressKeyUntilRelease).toHaveBeenCalledTimes(1); + expect(suppressKeyUntilRelease.mock.calls[0]![0]).toBe(down); + + // The element that owns the hold is no longer in the focus path, so its + // key-up never arrives. The latch delivers the release instead. + expect(onRelease).not.toHaveBeenCalled(); + suppressKeyUntilRelease.mock.calls[0]![1]!(); + expect(onRelease).toHaveBeenCalledTimes(1); + dispose(); + }); + + it('does not fire onRelease twice when key-up also reaches the element', () => { + const { startHold, releaseHold, onRelease, dispose } = setup(); + startHold(down); + startHold(downRepeat); + vi.advanceTimersByTime(200); + // focusManager lifts suppression first, then propagates the key-up. + suppressKeyUntilRelease.mock.calls[0]![1]!(); + releaseHold(); + expect(onRelease).toHaveBeenCalledTimes(1); + dispose(); + }); + + it('does not suppress when the press resolves as a tap', () => { + const { startHold, releaseHold, dispose } = setup(); + startHold(down); + releaseHold(); + vi.advanceTimersByTime(200); + expect(suppressKeyUntilRelease).not.toHaveBeenCalled(); + dispose(); + }); + }); + + describe('callback context', () => { + it('passes the key-down event and elements through to onEnter', () => { + const { startHold, releaseHold, onEnter, dispose } = setup(); + startHold(down, target, handlerElm); + releaseHold(); + expect(onEnter).toHaveBeenCalledWith(down, target, handlerElm); + dispose(); + }); + + it('replays the key-down context to a timer-resolved onEnter', () => { + const { startHold, onEnter, dispose } = setup(); + startHold(down, target, handlerElm); // no key-up + vi.advanceTimersByTime(200); + expect(onEnter).toHaveBeenCalledWith(down, target, handlerElm); + dispose(); + }); + + it('replays the key-down context to onHold and onRelease', () => { + const { startHold, onHold, onRelease, dispose } = setup(); + startHold(down, target, handlerElm); + startHold(downRepeat); + vi.advanceTimersByTime(200); + expect(onHold).toHaveBeenCalledWith(down, target, handlerElm); + suppressKeyUntilRelease.mock.calls[0]![1]!(); + expect(onRelease).toHaveBeenCalledWith(down, target, handlerElm); + dispose(); + }); + }); }); From 97f7c573eb2ad72f3a840023d18d2e45cd5f854c Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 20:31:01 -0400 Subject: [PATCH 02/10] Release 1.4.1-0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 423921e..a1d944e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@solidtv/solid", - "version": "1.4.0", + "version": "1.4.1-0", "description": "SolidTV", "type": "module", "exports": { From 4a858c0700e241030d423c9378737b99e1e357c4 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 20:58:44 -0400 Subject: [PATCH 03/10] fix(useHold): use auto-repeat where a key has it, timer where it doesn't MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit webOS emits no OS-level `repeat === true` for the Back key, so requiring an auto-repeat to confirm a hold made Back holds unreachable there. Whether a key repeats is a per-key property, not a per-platform one — webOS emits repeat for OK but not Back, and swallows key-up for OK but not Back — so a static flag per useHold instance was the wrong shape. Make `holdRequiresRepeat` three-state, defaulting to a new `'auto'` that infers per key. The discriminator is key-up delivery, which is the only thing that separates "still held" from "already released, key-up swallowed" at the threshold: - a key seen delivering key-up must still be down if none arrived, so no auto-repeat is needed and the hold resolves by timer (webOS Back); - a key that swallows key-up is indistinguishable either way, so an auto-repeat is still required and a repeat-less press stays a tap (webOS OK). The focus manager records key-up delivery per key as it observes it, exposed as `keyDeliversKeyUp` / `noteKeyUpDelivered`. Inference needs one prior press of a key, so the first press of a session resolves as a tap; `holdRequiresRepeat: false` skips the warm-up and always resolves by timer for a key already known to lack auto-repeat. `true` keeps the strict repeat-only behavior. Co-Authored-By: Claude Opus 5 --- docs/primitives/useHold.md | 67 +++++++++++++++------- src/core/focusManager.ts | 40 +++++++++++++ src/primitives/useFocusManager.ts | 2 + src/primitives/useHold.ts | 61 +++++++++++++++----- tests/keySuppression.test.tsx | 29 ++++++++-- tests/useHold.spec.ts | 95 ++++++++++++++++++++++++++++++- 6 files changed, 251 insertions(+), 43 deletions(-) diff --git a/docs/primitives/useHold.md b/docs/primitives/useHold.md index f6772ff..6e4ea8d 100644 --- a/docs/primitives/useHold.md +++ b/docs/primitives/useHold.md @@ -26,14 +26,14 @@ const [holdRight, releaseRight] = useHold({ #### `UseHoldProps` -| Prop | Type | Description | Default | -| --------------------------- | -------------- | --------------------------------------------------------------------------------------------------------------------------------- | ------------ | -| `onHold` | `HoldCallback` | Called once the hold threshold is exceeded. | **Required** | -| `onEnter` | `HoldCallback` | Called on press or key entry. May be delayed depending on config. | **Required** | -| `onRelease` | `HoldCallback` | Called after a successful hold is released. | `undefined` | -| `holdThreshold` | `number` | Time in milliseconds to wait before triggering `onHold`. | `500` | -| `performOnEnterImmediately` | `boolean` | Whether `onEnter` is triggered immediately or only if released early. | `false` | -| `holdRequiresRepeat` | `boolean` | Whether a hold must be confirmed by an auto-repeat key-down. See [Platforms without auto-repeat](#platforms-without-auto-repeat). | `true` | +| Prop | Type | Description | Default | +| --------------------------- | ------------------- | ----------------------------------------------------------------------------------------------------------------------- | ------------ | +| `onHold` | `HoldCallback` | Called once the hold threshold is exceeded. | **Required** | +| `onEnter` | `HoldCallback` | Called on press or key entry. May be delayed depending on config. | **Required** | +| `onRelease` | `HoldCallback` | Called after a successful hold is released. | `undefined` | +| `holdThreshold` | `number` | Time in milliseconds to wait before triggering `onHold`. | `500` | +| `performOnEnterImmediately` | `boolean` | Whether `onEnter` is triggered immediately or only if released early. | `false` | +| `holdRequiresRepeat` | `boolean \| 'auto'` | Whether a hold must be confirmed by an auto-repeat key-down. See [Keys without auto-repeat](#keys-without-auto-repeat). | `'auto'` | Each callback receives the same context a `KeyHandler` gets — the `KeyboardEvent` that began the press, the element whose handler ran, and the focused element: @@ -130,27 +130,50 @@ suppressKeyUntilRelease(event, () => console.log('key released')); --- -### Platforms without auto-repeat +### Keys without auto-repeat -A hold is confirmed by an auto-repeat key-down. If a press delivers neither a -key-up nor an auto-repeat by the threshold, the press is ambiguous, and by -default it resolves as a **tap** — the behavior that keeps taps working on -remotes that swallow key-up. +When the timer fires having seen neither a key-up nor an auto-repeat, the press +is ambiguous: the key is either still held, or already released with its key-up +swallowed. `holdRequiresRepeat` decides how that resolves. -On a platform whose remote input layer delivers no auto-repeat at all, that makes -a hold unreachable. Set `holdRequiresRepeat: false` to resolve the ambiguous case -as a hold instead, matching the legacy `keyHoldOptions` behavior: +This is a **per-key** property, not a per-platform one. On webOS, OK emits +auto-repeat but swallows key-up; Back is the reverse — it emits key-up but no +OS-level `repeat === true`. A hold on Back is therefore unreachable if +auto-repeat is required, and a timer is the only way to detect it. + +| Value | Behavior | +| ------------------ | -------------------------------------------------------------------- | +| `'auto'` (default) | Use auto-repeat where the key needs it, fall back to timer where not | +| `false` | Always resolve by timer | +| `true` | Require an auto-repeat; never resolve a repeat-less press as a hold | + +`'auto'` keys off whether the key delivers key-up, which is the only thing that +disambiguates the case: + +- **Key delivers key-up** (webOS Back): reaching the threshold without one means + it is genuinely still down. No auto-repeat needed — resolve by timer → `onHold`. +- **Key swallows key-up** (webOS OK): a finished press looks identical to one + still held, so only an auto-repeat can confirm a hold. Without one → `onEnter`. + +The focus manager records key-up delivery per key as it observes it, so this +needs no configuration. It does mean **the first press of a key resolves as a +tap**, since nothing has been observed yet. For a key whose behavior you already +know, say so and skip the warm-up: ```tsx -const [holdEnter, releaseEnter] = useHold({ - onHold: openContextMenu, - onEnter: openTile, - holdRequiresRepeat: false, // no auto-repeat on this platform +// webOS Back: emits key-up, but never emits auto-repeat. +const [holdBack, releaseBack] = useHold({ + onHold: exitApp, + onEnter: goBack, + holdThreshold: 1000, + holdRequiresRepeat: false, // resolve by timer from the very first press }); + +; ``` -An early key-up still resolves as a tap either way, so this only changes the -no-key-up **and** no-repeat case. +An early key-up still resolves as a tap under every setting — this only governs +the no-key-up **and** no-repeat case. --- diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index 935083a..fd28af9 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -450,6 +450,45 @@ const keyHoldTimeouts: { [key: KeyNameOrKeyCode]: number | true } = {}; const keyOf = (e: KeyboardEvent): KeyNameOrKeyCode => e.key || e.keyCode; +// Keys this platform has actually been seen delivering a key-up for. +// +// Whether a hold can be detected by timer alone comes down to this. If a key +// delivers key-up, reaching the hold threshold without one means the key is +// genuinely still down, and no auto-repeat is needed to confirm the hold. If it +// does not (webOS OK), a press that is already over is indistinguishable from +// one still held, so only an auto-repeat can confirm it. +// +// This is per-key, not per-platform: webOS emits auto-repeat for OK but not for +// Back, and swallows key-up for OK but not for Back. +const keysSeenReleasing = new Set(); + +/** + * Record that `keyOrEvent` delivered a key-up. The focus manager calls this for + * every key-up it sees; call it directly only when driving key events without + * {@link useFocusManager}. + */ +export const noteKeyUpDelivered = ( + keyOrEvent: KeyboardEvent | KeyNameOrKeyCode, +): void => { + keysSeenReleasing.add( + typeof keyOrEvent === 'object' ? keyOf(keyOrEvent) : keyOrEvent, + ); +}; + +/** + * Whether this key has been observed delivering a key-up in this session. + * + * `false` for a key not yet pressed — absence of evidence, not evidence that the + * key never releases. Callers that need a decision before any press has been + * observed should be told the answer explicitly rather than inferring it. + */ +export const keyDeliversKeyUp = ( + keyOrEvent: KeyboardEvent | KeyNameOrKeyCode, +): boolean => + keysSeenReleasing.has( + typeof keyOrEvent === 'object' ? keyOf(keyOrEvent) : keyOrEvent, + ); + // Keys whose auto-repeat key-downs are dropped before propagation, mapped to an // optional callback run when the key is finally released. // @@ -536,6 +575,7 @@ const handleKeyEvents = ( propagateKeyPress(keydown, mappedKeyEvent, false); } else if (keyup) { const key: KeyNameOrKeyCode = keyOf(keyup); + keysSeenReleasing.add(key); // 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. diff --git a/src/primitives/useFocusManager.ts b/src/primitives/useFocusManager.ts index f1c7298..ca185af 100644 --- a/src/primitives/useFocusManager.ts +++ b/src/primitives/useFocusManager.ts @@ -4,6 +4,8 @@ export { setActiveElementCore, suppressKeyUntilRelease, releaseKeySuppression, + keyDeliversKeyUp, + noteKeyUpDelivered, type KeyMap, type KeyHoldOptions, } from '../core/focusManager.js'; diff --git a/src/primitives/useHold.ts b/src/primitives/useHold.ts index 9442f20..17c9f3b 100644 --- a/src/primitives/useHold.ts +++ b/src/primitives/useHold.ts @@ -1,5 +1,9 @@ import { createMemo } from 'solid-js'; -import { suppressKeyUntilRelease } from '../core/focusManager.js'; +import { + suppressKeyUntilRelease, + keyDeliversKeyUp, + noteKeyUpDelivered, +} from '../core/focusManager.js'; import type { ElementNode } from '../core/elementNode.js'; /** @@ -29,13 +33,21 @@ export type UseHoldProps = { /** * Whether a hold must be confirmed by an auto-repeat key-down. * - * `true` (default): a press with neither key-up nor auto-repeat by the - * threshold resolves as a tap. `false`: it resolves as a hold, matching the - * legacy `keyHoldOptions` behavior. Set this to `false` on platforms whose - * remote input layer delivers no auto-repeat, where a hold gesture would - * otherwise be unreachable. + * - `'auto'` (default): use auto-repeat if the key needs it, otherwise resolve + * by timer. A key that has been seen delivering key-up needs no auto-repeat — + * reaching the threshold without a key-up means it is still down. A key that + * swallows key-up does need one, since a finished press is otherwise + * indistinguishable from one still held. + * - `false`: always resolve by timer, matching the legacy `keyHoldOptions` + * behavior. Deterministic from the very first press. + * - `true`: require an auto-repeat, never resolving a repeat-less press as a + * hold. + * + * `'auto'` infers from keys seen earlier in the session, so the first press of + * a key resolves as a tap. Set this explicitly for a key you already know — + * e.g. `false` for Back on webOS, which emits no auto-repeat. */ - holdRequiresRepeat?: boolean; + holdRequiresRepeat?: boolean | 'auto'; }; /** @@ -50,11 +62,16 @@ export type UseHoldProps = { * when the timer fires it resolves to a hold → `onHold`. * - if key-up arrives before the timer, it's a tap → `onEnter` (fires * immediately, no latency, on platforms that deliver key-up). - * - if neither key-up nor auto-repeat arrives, the timer resolves to a tap → - * `onEnter` after `holdThreshold` ms. This is the key-up-independent path that - * keeps taps working on webOS, at the cost of ~`holdThreshold` ms latency. - * Set `holdRequiresRepeat: false` to resolve this ambiguous case as a hold - * instead, on platforms that deliver no auto-repeat at all. + * - if the timer fires with neither, the key is either still held or already + * released with its key-up swallowed. `holdRequiresRepeat` decides which, and + * by default (`'auto'`) infers it: a key known to deliver key-up must still be + * down, so it resolves to a hold by timer alone; a key that swallows key-up + * resolves to a tap, keeping the primary action working on webOS OK at the + * cost of ~`holdThreshold` ms latency. + * + * Auto-repeat is therefore used where it exists and not required where it does + * not — webOS emits it for OK but not for Back, so Back holds resolve by timer + * while OK holds are confirmed by repeat. * * Once `onHold` fires the key is still physically down, so the focus manager * drops its remaining auto-repeats until it is released. Without that, a hold @@ -96,7 +113,18 @@ export function useHold(props: UseHoldProps): [HoldHandler, HoldHandler] { const performOnEnterImmediately = createMemo( () => props.performOnEnterImmediately ?? false, ); - const holdRequiresRepeat = createMemo(() => props.holdRequiresRepeat ?? true); + const holdRequiresRepeat = createMemo( + () => props.holdRequiresRepeat ?? 'auto', + ); + + // At the threshold with no auto-repeat seen, is this press still held (a hold) + // or already over with its key-up swallowed (a tap)? Only a key that delivers + // key-up can answer that on its own. + const requiresRepeat = (e: KeyboardEvent | undefined) => { + const mode = holdRequiresRepeat(); + if (mode !== 'auto') return mode; + return !(e !== undefined && keyDeliversKeyUp(e)); + }; let holdTimeout = -1; let enterFired = false; // onEnter already fired for this press @@ -146,7 +174,7 @@ export function useHold(props: UseHoldProps): [HoldHandler, HoldHandler] { holdTimeout = setTimeout(() => { holdTimeout = -1; - if (repeated || !holdRequiresRepeat()) { + if (repeated || !requiresRepeat(e)) { // Held past the threshold → hold gesture. holdFired = true; // The key is still down and will keep repeating. Drop those repeats so @@ -166,6 +194,11 @@ export function useHold(props: UseHoldProps): [HoldHandler, HoldHandler] { }; const releaseHold: HoldHandler = (e, target, handlerElm) => { + // Reaching here at all is proof this key delivers key-up, which is what + // `'auto'` reads. The focus manager records this too; doing it here as well + // keeps `'auto'` working behind a custom focus manager. + if (e) noteKeyUpDelivered(e); + if (holdTimeout !== -1) { // Released before the threshold → tap. Fires immediately where key-up is // delivered, avoiding the timer latency. diff --git a/tests/keySuppression.test.tsx b/tests/keySuppression.test.tsx index 2a55b2b..0006a51 100644 --- a/tests/keySuppression.test.tsx +++ b/tests/keySuppression.test.tsx @@ -4,13 +4,14 @@ import { useFocusManager, suppressKeyUntilRelease, releaseKeySuppression, + keyDeliversKeyUp, } from '@solidtv/solid/primitives'; import { renderer, waitForUpdate } from './setup.js'; -const keydown = (repeat = false) => - document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', repeat })); -const keyup = () => - document.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); +const keydown = (repeat = false, key = 'Enter') => + document.dispatchEvent(new KeyboardEvent('keydown', { key, repeat })); +const keyup = (key = 'Enter') => + document.dispatchEvent(new KeyboardEvent('keyup', { key })); async function setup() { const onEnter = v.vi.fn(); @@ -88,4 +89,24 @@ v.describe('key suppression', () => { dispose(); }); + + // Uses keys no other test in this file touches: the registry is a + // session-long capability probe and deliberately has no reset. + v.test('records which keys deliver key-up', async () => { + const { dispose } = await setup(); + + // Absence of evidence, not evidence the key never releases. + v.assert.equal(keyDeliversKeyUp('Back'), false); + + keydown(false, 'Back'); + v.assert.equal(keyDeliversKeyUp('Back'), false); // key-down proves nothing + + keyup('Back'); + v.assert.equal(keyDeliversKeyUp('Back'), true); + + // Per-key: a key that never released stays unknown, in the same session. + v.assert.equal(keyDeliversKeyUp('ColorF0Red'), false); + + dispose(); + }); }); diff --git a/tests/useHold.spec.ts b/tests/useHold.spec.ts index 67d224c..7f8c955 100644 --- a/tests/useHold.spec.ts +++ b/tests/useHold.spec.ts @@ -4,15 +4,29 @@ import type { ElementNode } from '../src/core/elementNode.ts'; const suppressKeyUntilRelease = vi.fn<(e: KeyboardEvent | string | number, onRelease?: () => void) => void>(); + +// Stands in for the focus manager's key-up registry, which `'auto'` reads. +const seenReleasing = new Set(); +const keyOf = (e: KeyboardEvent | string | number) => + typeof e === 'object' ? e.key : e; + vi.mock('../src/core/focusManager.ts', () => ({ suppressKeyUntilRelease: (...args: unknown[]) => (suppressKeyUntilRelease as (...a: unknown[]) => void)(...args), + keyDeliversKeyUp: (e: KeyboardEvent | string | number) => + seenReleasing.has(keyOf(e)), + noteKeyUpDelivered: (e: KeyboardEvent | string | number) => { + seenReleasing.add(keyOf(e)); + }, })); const { useHold } = await import('../src/primitives/useHold.ts'); +// webOS: OK emits auto-repeat but swallows key-up; Back is the reverse. const down = { key: 'Enter', repeat: false } as KeyboardEvent; const downRepeat = { key: 'Enter', repeat: true } as KeyboardEvent; +const backDown = { key: 'Back', repeat: false } as KeyboardEvent; +const backUp = { key: 'Back' } as KeyboardEvent; const target = { id: 'target' } as unknown as ElementNode; const handlerElm = { id: 'handlerElm' } as unknown as ElementNode; @@ -33,6 +47,7 @@ describe('useHold', () => { beforeEach(() => { vi.useFakeTimers(); suppressKeyUntilRelease.mockClear(); + seenReleasing.clear(); }); afterEach(() => vi.useRealTimers()); @@ -106,16 +121,73 @@ describe('useHold', () => { dispose(); }); - describe('holdRequiresRepeat', () => { - it('resolves a repeat-less press as a tap by default', () => { + describe("holdRequiresRepeat: 'auto' (default)", () => { + it('requires a repeat for a key that swallows key-up', () => { + // webOS OK: no key-up has ever been observed for this key, so a press + // that is already over is indistinguishable from one still held. const { startHold, onEnter, onHold, dispose } = setup(); - startHold(down); // no repeat, no key-up + startHold(down); vi.advanceTimersByTime(200); expect(onEnter).toHaveBeenCalledTimes(1); expect(onHold).not.toHaveBeenCalled(); dispose(); }); + it('resolves by timer for a key known to deliver key-up', () => { + // webOS Back: emits no auto-repeat, but does emit key-up. One prior press + // is what teaches 'auto' that. + const { startHold, releaseHold, onEnter, onHold, dispose } = setup(); + startHold(backDown); + releaseHold(backUp); + expect(onEnter).toHaveBeenCalledTimes(1); + + // Now hold it. No auto-repeat will ever arrive, and none is needed. + startHold(backDown); + vi.advanceTimersByTime(200); + expect(onHold).toHaveBeenCalledTimes(1); + expect(onEnter).toHaveBeenCalledTimes(1); + dispose(); + }); + + it('still resolves an early key-up as a tap for such a key', () => { + const { startHold, releaseHold, onEnter, onHold, dispose } = setup(); + startHold(backDown); + releaseHold(backUp); // warms the registry + startHold(backDown); + releaseHold(backUp); // released before the threshold → tap + vi.advanceTimersByTime(200); + expect(onEnter).toHaveBeenCalledTimes(2); + expect(onHold).not.toHaveBeenCalled(); + dispose(); + }); + + it('tracks key-up delivery per key, not per platform', () => { + // Back has released before; Enter never has. The same session must + // resolve them differently. + const back = setup(); + back.startHold(backDown); + back.releaseHold(backUp); + back.dispose(); + + const enter = setup(); + enter.startHold(down); + vi.advanceTimersByTime(200); + expect(enter.onHold).not.toHaveBeenCalled(); + expect(enter.onEnter).toHaveBeenCalledTimes(1); + enter.dispose(); + }); + + it('requires a repeat when startHold gets no event', () => { + const { startHold, onEnter, onHold, dispose } = setup(); + startHold(); // no event → no key to reason about + vi.advanceTimersByTime(200); + expect(onEnter).toHaveBeenCalledTimes(1); + expect(onHold).not.toHaveBeenCalled(); + dispose(); + }); + }); + + describe('holdRequiresRepeat: explicit', () => { it('resolves a repeat-less press as a hold when false', () => { const { startHold, onEnter, onHold, dispose } = setup({ holdRequiresRepeat: false, @@ -138,6 +210,23 @@ describe('useHold', () => { expect(onHold).not.toHaveBeenCalled(); dispose(); }); + + it('when true, never resolves a repeat-less press as a hold', () => { + // Even for a key 'auto' would have resolved by timer. + const warm = setup(); + warm.startHold(backDown); + warm.releaseHold(backUp); + warm.dispose(); + + const { startHold, onEnter, onHold, dispose } = setup({ + holdRequiresRepeat: true, + }); + startHold(backDown); + vi.advanceTimersByTime(200); + expect(onHold).not.toHaveBeenCalled(); + expect(onEnter).toHaveBeenCalledTimes(1); + dispose(); + }); }); describe('key suppression after a hold', () => { From 6522471725a52677e93ea4e7594dd4d58c36d405 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 21:07:29 -0400 Subject: [PATCH 04/10] Release 1.4.1-1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a1d944e..1ef3da7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@solidtv/solid", - "version": "1.4.1-0", + "version": "1.4.1-1", "description": "SolidTV", "type": "module", "exports": { From df97e0184070113ca6d498b8b814a8f07d0b3aea Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 21:41:59 -0400 Subject: [PATCH 05/10] Revert "fix(useHold): use auto-repeat where a key has it, timer where it doesn't" This reverts commit 4a858c0700e241030d423c9378737b99e1e357c4. --- docs/primitives/useHold.md | 67 +++++++--------------- src/core/focusManager.ts | 40 ------------- src/primitives/useFocusManager.ts | 2 - src/primitives/useHold.ts | 61 +++++--------------- tests/keySuppression.test.tsx | 29 ++-------- tests/useHold.spec.ts | 95 +------------------------------ 6 files changed, 43 insertions(+), 251 deletions(-) diff --git a/docs/primitives/useHold.md b/docs/primitives/useHold.md index 6e4ea8d..f6772ff 100644 --- a/docs/primitives/useHold.md +++ b/docs/primitives/useHold.md @@ -26,14 +26,14 @@ const [holdRight, releaseRight] = useHold({ #### `UseHoldProps` -| Prop | Type | Description | Default | -| --------------------------- | ------------------- | ----------------------------------------------------------------------------------------------------------------------- | ------------ | -| `onHold` | `HoldCallback` | Called once the hold threshold is exceeded. | **Required** | -| `onEnter` | `HoldCallback` | Called on press or key entry. May be delayed depending on config. | **Required** | -| `onRelease` | `HoldCallback` | Called after a successful hold is released. | `undefined` | -| `holdThreshold` | `number` | Time in milliseconds to wait before triggering `onHold`. | `500` | -| `performOnEnterImmediately` | `boolean` | Whether `onEnter` is triggered immediately or only if released early. | `false` | -| `holdRequiresRepeat` | `boolean \| 'auto'` | Whether a hold must be confirmed by an auto-repeat key-down. See [Keys without auto-repeat](#keys-without-auto-repeat). | `'auto'` | +| Prop | Type | Description | Default | +| --------------------------- | -------------- | --------------------------------------------------------------------------------------------------------------------------------- | ------------ | +| `onHold` | `HoldCallback` | Called once the hold threshold is exceeded. | **Required** | +| `onEnter` | `HoldCallback` | Called on press or key entry. May be delayed depending on config. | **Required** | +| `onRelease` | `HoldCallback` | Called after a successful hold is released. | `undefined` | +| `holdThreshold` | `number` | Time in milliseconds to wait before triggering `onHold`. | `500` | +| `performOnEnterImmediately` | `boolean` | Whether `onEnter` is triggered immediately or only if released early. | `false` | +| `holdRequiresRepeat` | `boolean` | Whether a hold must be confirmed by an auto-repeat key-down. See [Platforms without auto-repeat](#platforms-without-auto-repeat). | `true` | Each callback receives the same context a `KeyHandler` gets — the `KeyboardEvent` that began the press, the element whose handler ran, and the focused element: @@ -130,50 +130,27 @@ suppressKeyUntilRelease(event, () => console.log('key released')); --- -### Keys without auto-repeat +### Platforms without auto-repeat -When the timer fires having seen neither a key-up nor an auto-repeat, the press -is ambiguous: the key is either still held, or already released with its key-up -swallowed. `holdRequiresRepeat` decides how that resolves. +A hold is confirmed by an auto-repeat key-down. If a press delivers neither a +key-up nor an auto-repeat by the threshold, the press is ambiguous, and by +default it resolves as a **tap** — the behavior that keeps taps working on +remotes that swallow key-up. -This is a **per-key** property, not a per-platform one. On webOS, OK emits -auto-repeat but swallows key-up; Back is the reverse — it emits key-up but no -OS-level `repeat === true`. A hold on Back is therefore unreachable if -auto-repeat is required, and a timer is the only way to detect it. - -| Value | Behavior | -| ------------------ | -------------------------------------------------------------------- | -| `'auto'` (default) | Use auto-repeat where the key needs it, fall back to timer where not | -| `false` | Always resolve by timer | -| `true` | Require an auto-repeat; never resolve a repeat-less press as a hold | - -`'auto'` keys off whether the key delivers key-up, which is the only thing that -disambiguates the case: - -- **Key delivers key-up** (webOS Back): reaching the threshold without one means - it is genuinely still down. No auto-repeat needed — resolve by timer → `onHold`. -- **Key swallows key-up** (webOS OK): a finished press looks identical to one - still held, so only an auto-repeat can confirm a hold. Without one → `onEnter`. - -The focus manager records key-up delivery per key as it observes it, so this -needs no configuration. It does mean **the first press of a key resolves as a -tap**, since nothing has been observed yet. For a key whose behavior you already -know, say so and skip the warm-up: +On a platform whose remote input layer delivers no auto-repeat at all, that makes +a hold unreachable. Set `holdRequiresRepeat: false` to resolve the ambiguous case +as a hold instead, matching the legacy `keyHoldOptions` behavior: ```tsx -// webOS Back: emits key-up, but never emits auto-repeat. -const [holdBack, releaseBack] = useHold({ - onHold: exitApp, - onEnter: goBack, - holdThreshold: 1000, - holdRequiresRepeat: false, // resolve by timer from the very first press +const [holdEnter, releaseEnter] = useHold({ + onHold: openContextMenu, + onEnter: openTile, + holdRequiresRepeat: false, // no auto-repeat on this platform }); - -; ``` -An early key-up still resolves as a tap under every setting — this only governs -the no-key-up **and** no-repeat case. +An early key-up still resolves as a tap either way, so this only changes the +no-key-up **and** no-repeat case. --- diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index fd28af9..935083a 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -450,45 +450,6 @@ const keyHoldTimeouts: { [key: KeyNameOrKeyCode]: number | true } = {}; const keyOf = (e: KeyboardEvent): KeyNameOrKeyCode => e.key || e.keyCode; -// Keys this platform has actually been seen delivering a key-up for. -// -// Whether a hold can be detected by timer alone comes down to this. If a key -// delivers key-up, reaching the hold threshold without one means the key is -// genuinely still down, and no auto-repeat is needed to confirm the hold. If it -// does not (webOS OK), a press that is already over is indistinguishable from -// one still held, so only an auto-repeat can confirm it. -// -// This is per-key, not per-platform: webOS emits auto-repeat for OK but not for -// Back, and swallows key-up for OK but not for Back. -const keysSeenReleasing = new Set(); - -/** - * Record that `keyOrEvent` delivered a key-up. The focus manager calls this for - * every key-up it sees; call it directly only when driving key events without - * {@link useFocusManager}. - */ -export const noteKeyUpDelivered = ( - keyOrEvent: KeyboardEvent | KeyNameOrKeyCode, -): void => { - keysSeenReleasing.add( - typeof keyOrEvent === 'object' ? keyOf(keyOrEvent) : keyOrEvent, - ); -}; - -/** - * Whether this key has been observed delivering a key-up in this session. - * - * `false` for a key not yet pressed — absence of evidence, not evidence that the - * key never releases. Callers that need a decision before any press has been - * observed should be told the answer explicitly rather than inferring it. - */ -export const keyDeliversKeyUp = ( - keyOrEvent: KeyboardEvent | KeyNameOrKeyCode, -): boolean => - keysSeenReleasing.has( - typeof keyOrEvent === 'object' ? keyOf(keyOrEvent) : keyOrEvent, - ); - // Keys whose auto-repeat key-downs are dropped before propagation, mapped to an // optional callback run when the key is finally released. // @@ -575,7 +536,6 @@ const handleKeyEvents = ( propagateKeyPress(keydown, mappedKeyEvent, false); } else if (keyup) { const key: KeyNameOrKeyCode = keyOf(keyup); - keysSeenReleasing.add(key); // 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. diff --git a/src/primitives/useFocusManager.ts b/src/primitives/useFocusManager.ts index ca185af..f1c7298 100644 --- a/src/primitives/useFocusManager.ts +++ b/src/primitives/useFocusManager.ts @@ -4,8 +4,6 @@ export { setActiveElementCore, suppressKeyUntilRelease, releaseKeySuppression, - keyDeliversKeyUp, - noteKeyUpDelivered, type KeyMap, type KeyHoldOptions, } from '../core/focusManager.js'; diff --git a/src/primitives/useHold.ts b/src/primitives/useHold.ts index 17c9f3b..9442f20 100644 --- a/src/primitives/useHold.ts +++ b/src/primitives/useHold.ts @@ -1,9 +1,5 @@ import { createMemo } from 'solid-js'; -import { - suppressKeyUntilRelease, - keyDeliversKeyUp, - noteKeyUpDelivered, -} from '../core/focusManager.js'; +import { suppressKeyUntilRelease } from '../core/focusManager.js'; import type { ElementNode } from '../core/elementNode.js'; /** @@ -33,21 +29,13 @@ export type UseHoldProps = { /** * Whether a hold must be confirmed by an auto-repeat key-down. * - * - `'auto'` (default): use auto-repeat if the key needs it, otherwise resolve - * by timer. A key that has been seen delivering key-up needs no auto-repeat — - * reaching the threshold without a key-up means it is still down. A key that - * swallows key-up does need one, since a finished press is otherwise - * indistinguishable from one still held. - * - `false`: always resolve by timer, matching the legacy `keyHoldOptions` - * behavior. Deterministic from the very first press. - * - `true`: require an auto-repeat, never resolving a repeat-less press as a - * hold. - * - * `'auto'` infers from keys seen earlier in the session, so the first press of - * a key resolves as a tap. Set this explicitly for a key you already know — - * e.g. `false` for Back on webOS, which emits no auto-repeat. + * `true` (default): a press with neither key-up nor auto-repeat by the + * threshold resolves as a tap. `false`: it resolves as a hold, matching the + * legacy `keyHoldOptions` behavior. Set this to `false` on platforms whose + * remote input layer delivers no auto-repeat, where a hold gesture would + * otherwise be unreachable. */ - holdRequiresRepeat?: boolean | 'auto'; + holdRequiresRepeat?: boolean; }; /** @@ -62,16 +50,11 @@ export type UseHoldProps = { * when the timer fires it resolves to a hold → `onHold`. * - if key-up arrives before the timer, it's a tap → `onEnter` (fires * immediately, no latency, on platforms that deliver key-up). - * - if the timer fires with neither, the key is either still held or already - * released with its key-up swallowed. `holdRequiresRepeat` decides which, and - * by default (`'auto'`) infers it: a key known to deliver key-up must still be - * down, so it resolves to a hold by timer alone; a key that swallows key-up - * resolves to a tap, keeping the primary action working on webOS OK at the - * cost of ~`holdThreshold` ms latency. - * - * Auto-repeat is therefore used where it exists and not required where it does - * not — webOS emits it for OK but not for Back, so Back holds resolve by timer - * while OK holds are confirmed by repeat. + * - if neither key-up nor auto-repeat arrives, the timer resolves to a tap → + * `onEnter` after `holdThreshold` ms. This is the key-up-independent path that + * keeps taps working on webOS, at the cost of ~`holdThreshold` ms latency. + * Set `holdRequiresRepeat: false` to resolve this ambiguous case as a hold + * instead, on platforms that deliver no auto-repeat at all. * * Once `onHold` fires the key is still physically down, so the focus manager * drops its remaining auto-repeats until it is released. Without that, a hold @@ -113,18 +96,7 @@ export function useHold(props: UseHoldProps): [HoldHandler, HoldHandler] { const performOnEnterImmediately = createMemo( () => props.performOnEnterImmediately ?? false, ); - const holdRequiresRepeat = createMemo( - () => props.holdRequiresRepeat ?? 'auto', - ); - - // At the threshold with no auto-repeat seen, is this press still held (a hold) - // or already over with its key-up swallowed (a tap)? Only a key that delivers - // key-up can answer that on its own. - const requiresRepeat = (e: KeyboardEvent | undefined) => { - const mode = holdRequiresRepeat(); - if (mode !== 'auto') return mode; - return !(e !== undefined && keyDeliversKeyUp(e)); - }; + const holdRequiresRepeat = createMemo(() => props.holdRequiresRepeat ?? true); let holdTimeout = -1; let enterFired = false; // onEnter already fired for this press @@ -174,7 +146,7 @@ export function useHold(props: UseHoldProps): [HoldHandler, HoldHandler] { holdTimeout = setTimeout(() => { holdTimeout = -1; - if (repeated || !requiresRepeat(e)) { + if (repeated || !holdRequiresRepeat()) { // Held past the threshold → hold gesture. holdFired = true; // The key is still down and will keep repeating. Drop those repeats so @@ -194,11 +166,6 @@ export function useHold(props: UseHoldProps): [HoldHandler, HoldHandler] { }; const releaseHold: HoldHandler = (e, target, handlerElm) => { - // Reaching here at all is proof this key delivers key-up, which is what - // `'auto'` reads. The focus manager records this too; doing it here as well - // keeps `'auto'` working behind a custom focus manager. - if (e) noteKeyUpDelivered(e); - if (holdTimeout !== -1) { // Released before the threshold → tap. Fires immediately where key-up is // delivered, avoiding the timer latency. diff --git a/tests/keySuppression.test.tsx b/tests/keySuppression.test.tsx index 0006a51..2a55b2b 100644 --- a/tests/keySuppression.test.tsx +++ b/tests/keySuppression.test.tsx @@ -4,14 +4,13 @@ import { useFocusManager, suppressKeyUntilRelease, releaseKeySuppression, - keyDeliversKeyUp, } from '@solidtv/solid/primitives'; import { renderer, waitForUpdate } from './setup.js'; -const keydown = (repeat = false, key = 'Enter') => - document.dispatchEvent(new KeyboardEvent('keydown', { key, repeat })); -const keyup = (key = 'Enter') => - document.dispatchEvent(new KeyboardEvent('keyup', { key })); +const keydown = (repeat = false) => + document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', repeat })); +const keyup = () => + document.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); async function setup() { const onEnter = v.vi.fn(); @@ -89,24 +88,4 @@ v.describe('key suppression', () => { dispose(); }); - - // Uses keys no other test in this file touches: the registry is a - // session-long capability probe and deliberately has no reset. - v.test('records which keys deliver key-up', async () => { - const { dispose } = await setup(); - - // Absence of evidence, not evidence the key never releases. - v.assert.equal(keyDeliversKeyUp('Back'), false); - - keydown(false, 'Back'); - v.assert.equal(keyDeliversKeyUp('Back'), false); // key-down proves nothing - - keyup('Back'); - v.assert.equal(keyDeliversKeyUp('Back'), true); - - // Per-key: a key that never released stays unknown, in the same session. - v.assert.equal(keyDeliversKeyUp('ColorF0Red'), false); - - dispose(); - }); }); diff --git a/tests/useHold.spec.ts b/tests/useHold.spec.ts index 7f8c955..67d224c 100644 --- a/tests/useHold.spec.ts +++ b/tests/useHold.spec.ts @@ -4,29 +4,15 @@ import type { ElementNode } from '../src/core/elementNode.ts'; const suppressKeyUntilRelease = vi.fn<(e: KeyboardEvent | string | number, onRelease?: () => void) => void>(); - -// Stands in for the focus manager's key-up registry, which `'auto'` reads. -const seenReleasing = new Set(); -const keyOf = (e: KeyboardEvent | string | number) => - typeof e === 'object' ? e.key : e; - vi.mock('../src/core/focusManager.ts', () => ({ suppressKeyUntilRelease: (...args: unknown[]) => (suppressKeyUntilRelease as (...a: unknown[]) => void)(...args), - keyDeliversKeyUp: (e: KeyboardEvent | string | number) => - seenReleasing.has(keyOf(e)), - noteKeyUpDelivered: (e: KeyboardEvent | string | number) => { - seenReleasing.add(keyOf(e)); - }, })); const { useHold } = await import('../src/primitives/useHold.ts'); -// webOS: OK emits auto-repeat but swallows key-up; Back is the reverse. const down = { key: 'Enter', repeat: false } as KeyboardEvent; const downRepeat = { key: 'Enter', repeat: true } as KeyboardEvent; -const backDown = { key: 'Back', repeat: false } as KeyboardEvent; -const backUp = { key: 'Back' } as KeyboardEvent; const target = { id: 'target' } as unknown as ElementNode; const handlerElm = { id: 'handlerElm' } as unknown as ElementNode; @@ -47,7 +33,6 @@ describe('useHold', () => { beforeEach(() => { vi.useFakeTimers(); suppressKeyUntilRelease.mockClear(); - seenReleasing.clear(); }); afterEach(() => vi.useRealTimers()); @@ -121,73 +106,16 @@ describe('useHold', () => { dispose(); }); - describe("holdRequiresRepeat: 'auto' (default)", () => { - it('requires a repeat for a key that swallows key-up', () => { - // webOS OK: no key-up has ever been observed for this key, so a press - // that is already over is indistinguishable from one still held. + describe('holdRequiresRepeat', () => { + it('resolves a repeat-less press as a tap by default', () => { const { startHold, onEnter, onHold, dispose } = setup(); - startHold(down); + startHold(down); // no repeat, no key-up vi.advanceTimersByTime(200); expect(onEnter).toHaveBeenCalledTimes(1); expect(onHold).not.toHaveBeenCalled(); dispose(); }); - it('resolves by timer for a key known to deliver key-up', () => { - // webOS Back: emits no auto-repeat, but does emit key-up. One prior press - // is what teaches 'auto' that. - const { startHold, releaseHold, onEnter, onHold, dispose } = setup(); - startHold(backDown); - releaseHold(backUp); - expect(onEnter).toHaveBeenCalledTimes(1); - - // Now hold it. No auto-repeat will ever arrive, and none is needed. - startHold(backDown); - vi.advanceTimersByTime(200); - expect(onHold).toHaveBeenCalledTimes(1); - expect(onEnter).toHaveBeenCalledTimes(1); - dispose(); - }); - - it('still resolves an early key-up as a tap for such a key', () => { - const { startHold, releaseHold, onEnter, onHold, dispose } = setup(); - startHold(backDown); - releaseHold(backUp); // warms the registry - startHold(backDown); - releaseHold(backUp); // released before the threshold → tap - vi.advanceTimersByTime(200); - expect(onEnter).toHaveBeenCalledTimes(2); - expect(onHold).not.toHaveBeenCalled(); - dispose(); - }); - - it('tracks key-up delivery per key, not per platform', () => { - // Back has released before; Enter never has. The same session must - // resolve them differently. - const back = setup(); - back.startHold(backDown); - back.releaseHold(backUp); - back.dispose(); - - const enter = setup(); - enter.startHold(down); - vi.advanceTimersByTime(200); - expect(enter.onHold).not.toHaveBeenCalled(); - expect(enter.onEnter).toHaveBeenCalledTimes(1); - enter.dispose(); - }); - - it('requires a repeat when startHold gets no event', () => { - const { startHold, onEnter, onHold, dispose } = setup(); - startHold(); // no event → no key to reason about - vi.advanceTimersByTime(200); - expect(onEnter).toHaveBeenCalledTimes(1); - expect(onHold).not.toHaveBeenCalled(); - dispose(); - }); - }); - - describe('holdRequiresRepeat: explicit', () => { it('resolves a repeat-less press as a hold when false', () => { const { startHold, onEnter, onHold, dispose } = setup({ holdRequiresRepeat: false, @@ -210,23 +138,6 @@ describe('useHold', () => { expect(onHold).not.toHaveBeenCalled(); dispose(); }); - - it('when true, never resolves a repeat-less press as a hold', () => { - // Even for a key 'auto' would have resolved by timer. - const warm = setup(); - warm.startHold(backDown); - warm.releaseHold(backUp); - warm.dispose(); - - const { startHold, onEnter, onHold, dispose } = setup({ - holdRequiresRepeat: true, - }); - startHold(backDown); - vi.advanceTimersByTime(200); - expect(onHold).not.toHaveBeenCalled(); - expect(onEnter).toHaveBeenCalledTimes(1); - dispose(); - }); }); describe('key suppression after a hold', () => { From 55441640fe0647be59ee05800133c84c64b3a144 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 21:49:07 -0400 Subject: [PATCH 06/10] fix(focus): match a suppressed key on keyCode as well as name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `key` is not a stable identity for a physical key across key-down and key-up. webOS reports Back's key-down as { key: 'GoBack', keyCode: 461 } and its key-up as { key: 'Unidentified', keyCode: 461 } — the same physical key under two names, sharing only the keyCode. The suppression latch keyed on `e.key || e.keyCode`, so a hold on Back registered under 'GoBack' and the key-up looked up 'Unidentified'. It never matched: the key stayed suppressed until the next fresh key-down, and the latch's onRelease never fired. Identify a key by every name its event carries, and treat two events as the same key if any identity matches. A suppression is indexed under each of its identities and lifted through any of them. 'Unidentified' is excluded as an identity, since it names no particular key and would conflate every key that reports it. Tests cover the logged webOS Back sequence verbatim, including a full useHold-driven tap / hold / release through the real focus manager. Co-Authored-By: Claude Opus 5 --- docs/primitives/useHold.md | 17 ++++ src/core/focusManager.ts | 75 +++++++++++++---- tests/keySuppression.test.tsx | 152 +++++++++++++++++++++++++++++++++- 3 files changed, 224 insertions(+), 20 deletions(-) diff --git a/docs/primitives/useHold.md b/docs/primitives/useHold.md index f6772ff..59efb03 100644 --- a/docs/primitives/useHold.md +++ b/docs/primitives/useHold.md @@ -128,6 +128,23 @@ import { suppressKeyUntilRelease(event, () => console.log('key released')); ``` +Pass the `KeyboardEvent` rather than a key name where you have one. `key` is not a +stable identity for a physical key across key-down and key-up — webOS reports +Back as: + +``` +key-down { key: 'GoBack', keyCode: 461, repeat: false } +key-up { key: 'Unidentified', keyCode: 461 } +``` + +Given the event, a key is matched on both its name and its keyCode, so a key-up +that renames the key still lifts the suppression. Such a key must also be mapped +by keyCode for its release to route at all: + +```tsx +useFocusManager({ Back: [461, 'GoBack'] }); +``` + --- ### Platforms without auto-repeat diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index 935083a..0dfe135 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -450,8 +450,27 @@ const keyHoldTimeouts: { [key: KeyNameOrKeyCode]: number | true } = {}; const keyOf = (e: KeyboardEvent): KeyNameOrKeyCode => e.key || e.keyCode; -// Keys whose auto-repeat key-downs are dropped before propagation, mapped to an -// optional callback run when the key is finally released. +// `key` is not a stable identity for a physical key across key-down and key-up. +// webOS reports Back's key-down as { key: 'GoBack', keyCode: 461 } and its +// key-up as { key: 'Unidentified', keyCode: 461 } — same key, two names, with +// only the keyCode shared. So a key is identified by *every* name its event +// carries, and two events are the same key if any identity matches. +const UNIDENTIFIED = 'Unidentified'; + +const keyIdentities = ( + keyOrEvent: KeyboardEvent | KeyNameOrKeyCode, +): KeyNameOrKeyCode[] => { + if (typeof keyOrEvent !== 'object') return [keyOrEvent]; + const ids: KeyNameOrKeyCode[] = []; + // 'Unidentified' names no particular key. Treating it as an identity would + // conflate every key that reports it. + if (keyOrEvent.key && keyOrEvent.key !== UNIDENTIFIED) + ids.push(keyOrEvent.key); + if (keyOrEvent.keyCode) ids.push(keyOrEvent.keyCode); + return ids; +}; + +// Keys whose auto-repeat key-downs are dropped before propagation. // // A hold action typically moves focus (opening a context menu, say) while the // key is still physically down. Two things then go wrong, and neither can be @@ -463,12 +482,29 @@ const keyOf = (e: KeyboardEvent): KeyNameOrKeyCode => e.key || e.keyCode; // Both are propagation concerns, so the latch lives here. The release callback // is what lets a suppressor still learn about a key-up it can no longer receive // through the focus path. -const suppressedKeys = new Map void) | undefined>(); +type Suppression = { + ids: KeyNameOrKeyCode[]; + onRelease?: () => void; +}; + +// Indexed under every identity of the suppressed key, so a key-up naming the +// key differently still finds it. Entries for one key share a Suppression. +const suppressedKeys = new Map(); + +const findSuppression = (ids: KeyNameOrKeyCode[]): Suppression | undefined => { + for (const id of ids) { + const found = suppressedKeys.get(id); + if (found) return found; + } + return undefined; +}; -const liftSuppression = (key: KeyNameOrKeyCode): void => { - const onRelease = suppressedKeys.get(key); - suppressedKeys.delete(key); - onRelease?.(); +const liftSuppression = (ids: KeyNameOrKeyCode[]): void => { + const found = findSuppression(ids); + if (!found) return; + // Drop every alias, not just the one that matched. + for (const id of found.ids) suppressedKeys.delete(id); + found.onRelease?.(); }; /** @@ -480,6 +516,10 @@ const liftSuppression = (key: KeyNameOrKeyCode): void => { * when suppression lifts, whichever way it lifts, and is delivered regardless of * where focus has moved in the meantime. * + * Pass the `KeyboardEvent` where you have one: the key is then matched on both + * its name and its keyCode, which is what lets a key-up reporting a different + * `key` for the same physical key (webOS Back) still lift the suppression. + * * `useHold` calls this itself when a hold fires; call it directly only when * implementing hold behavior outside that primitive. */ @@ -487,10 +527,10 @@ export const suppressKeyUntilRelease = ( keyOrEvent: KeyboardEvent | KeyNameOrKeyCode, onRelease?: () => void, ): void => { - suppressedKeys.set( - typeof keyOrEvent === 'object' ? keyOf(keyOrEvent) : keyOrEvent, - onRelease, - ); + const ids = keyIdentities(keyOrEvent); + if (ids.length === 0) return; + const suppression: Suppression = { ids, onRelease }; + for (const id of ids) suppressedKeys.set(id, suppression); }; /** @@ -500,9 +540,7 @@ export const suppressKeyUntilRelease = ( export const releaseKeySuppression = ( keyOrEvent: KeyboardEvent | KeyNameOrKeyCode, ): void => { - liftSuppression( - typeof keyOrEvent === 'object' ? keyOf(keyOrEvent) : keyOrEvent, - ); + liftSuppression(keyIdentities(keyOrEvent)); }; const handleKeyEvents = ( @@ -512,12 +550,13 @@ const handleKeyEvents = ( ) => { if (keydown) { const key: KeyNameOrKeyCode = keyOf(keydown); + const ids = keyIdentities(keydown); if (keydown.repeat) { - if (suppressedKeys.has(key)) return; - } else if (suppressedKeys.has(key)) { + if (findSuppression(ids)) return; + } 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(key); + liftSuppression(ids); } const mappedKeyHoldEvent = keyHoldMapEntries[keydown.key] || keyHoldMapEntries[keydown.keyCode]; @@ -539,7 +578,7 @@ const handleKeyEvents = ( // 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. - if (suppressedKeys.has(key)) liftSuppression(key); + liftSuppression(keyIdentities(keyup)); const mappedKeyEvent = keyMapEntries[keyup.key] || keyMapEntries[keyup.keyCode]; if (keyHoldTimeouts[key] === true) { diff --git a/tests/keySuppression.test.tsx b/tests/keySuppression.test.tsx index 2a55b2b..9acbd0d 100644 --- a/tests/keySuppression.test.tsx +++ b/tests/keySuppression.test.tsx @@ -4,6 +4,7 @@ import { useFocusManager, suppressKeyUntilRelease, releaseKeySuppression, + useHold, } from '@solidtv/solid/primitives'; import { renderer, waitForUpdate } from './setup.js'; @@ -12,12 +13,12 @@ const keydown = (repeat = false) => const keyup = () => document.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); -async function setup() { +async function setup(props: Record = {}) { const onEnter = v.vi.fn(); let dispose!: () => void; dispose = renderer.render(() => { useFocusManager(); - return ; + return ; }) as unknown as () => void; await waitForUpdate(); return { onEnter, dispose }; @@ -89,3 +90,150 @@ v.describe('key suppression', () => { dispose(); }); }); + +// webOS reports Back's key-down and key-up under different `key` names, sharing +// only the keyCode. Verbatim from a device log: +// captureBack { key: 'GoBack', keyCode: 461, repeat: false } +// captureBackRelease { key: 'Unidentified', keyCode: 461 } +const backDown = (repeat = false) => + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'GoBack', keyCode: 461, repeat }), + ); +const backUp = () => + document.dispatchEvent( + new KeyboardEvent('keyup', { key: 'Unidentified', keyCode: 461 }), + ); + +v.describe('key identity across key-down and key-up', () => { + v.test('a key-up naming the key differently still lifts suppression', async () => { + const { dispose } = await setup(); + const onRelease = v.vi.fn(); + + backDown(); + suppressKeyUntilRelease( + new KeyboardEvent('keydown', { key: 'GoBack', keyCode: 461 }), + onRelease, + ); + + // Matched on keyCode 461, since 'Unidentified' names no key. + backUp(); + v.assert.equal(onRelease.mock.calls.length, 1); + + dispose(); + }); + + v.test('suppression is not left latched after such a key-up', async () => { + const seen: string[] = []; + const { dispose } = await setup({ + onCaptureKey: (e: KeyboardEvent) => { + seen.push(`${String(e.key)}${e.repeat ? ':repeat' : ''}`); + return false; + }, + }); + + backDown(); + suppressKeyUntilRelease( + new KeyboardEvent('keydown', { key: 'GoBack', keyCode: 461 }), + ); + backDown(true); // repeat → dropped, never reaches the capture phase + backUp(); // lifts, despite the different `key` + + // The next press must get through rather than staying wedged. + backDown(); + backDown(true); + + v.assert.deepEqual(seen, ['GoBack', 'GoBack', 'GoBack:repeat']); + + dispose(); + }); + + v.test('useHold drives a Back hold end to end on webOS', async () => { + const onHold = v.vi.fn(); + const onEnter = v.vi.fn(); + const onRelease = v.vi.fn(); + const reached: string[] = []; + + const dispose = renderer.render(() => { + // Back reaches the app as GoBack/461 on key-down and Unidentified/461 on + // key-up, so it has to be mapped by keyCode. + useFocusManager({ Back: [461, 'GoBack'] }); + const [startHold, releaseHold] = useHold({ + onHold, + onEnter, + onRelease, + holdThreshold: 1000, + holdRequiresRepeat: false, // webOS emits no auto-repeat for Back + }); + return ( + { + reached.push(e.repeat ? 'repeat' : 'press'); + return false; + }} + onBack={startHold} + onCaptureBackRelease={releaseHold} + /> + ); + }) as unknown as () => void; + await waitForUpdate(); + // Let autofocus settle on real timers — switching to fake ones with focus + // still pending would strand it and leave the focus path empty. + await new Promise((resolve) => setTimeout(resolve, 10)); + + v.vi.useFakeTimers(); + try { + // Tap: released well before the threshold. + backDown(); + backUp(); + v.assert.equal(onEnter.mock.calls.length, 1); + v.vi.advanceTimersByTime(1000); + v.assert.equal(onHold.mock.calls.length, 0); + + // Hold: no key-up, no auto-repeat — only the timer can resolve it. + backDown(); + v.vi.advanceTimersByTime(1000); + v.assert.equal(onHold.mock.calls.length, 1); + v.assert.equal(onEnter.mock.calls.length, 1); + + // The key is still down. Anything it emits now must be swallowed, so a + // hold that moved focus could not fire whatever it just focused. + reached.length = 0; + backDown(true); + v.assert.deepEqual(reached, []); + + // Release. The key-up names the key differently, and must still land. + v.assert.equal(onRelease.mock.calls.length, 0); + backUp(); + v.assert.equal(onRelease.mock.calls.length, 1); + + // ...and the key must not be left latched. + backDown(true); + v.assert.deepEqual(reached, ['repeat']); + } finally { + v.vi.useRealTimers(); + dispose(); + } + }); + + v.test('does not conflate two keys that both report Unidentified', async () => { + const { dispose } = await setup(); + const backRelease = v.vi.fn(); + + suppressKeyUntilRelease( + new KeyboardEvent('keydown', { key: 'GoBack', keyCode: 461 }), + backRelease, + ); + + // A different physical key, also anonymised on key-up. + document.dispatchEvent( + new KeyboardEvent('keyup', { key: 'Unidentified', keyCode: 462 }), + ); + v.assert.equal(backRelease.mock.calls.length, 0); + + backUp(); + v.assert.equal(backRelease.mock.calls.length, 1); + + dispose(); + }); +}); From 6ef7fae9730c96dc3b4967bec9376c77941be6c9 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 21:54:50 -0400 Subject: [PATCH 07/10] Release 1.4.1-2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1ef3da7..2970ce1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@solidtv/solid", - "version": "1.4.1-1", + "version": "1.4.1-2", "description": "SolidTV", "type": "module", "exports": { From ae0c0087457aea15a9f9bc82d6688e3d551e3aca Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 22:32:36 -0400 Subject: [PATCH 08/10] feat(focus)!: remove the legacy keyHoldOptions path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: `useFocusManager`'s second parameter (`keyHoldOptions` / `userKeyHoldMap` / `holdThreshold`) is removed, along with the `onKeyHold` fallback handler and the `on${Key}Hold` handlers it dispatched. Hold gestures are handled by the `useHold` primitive. The global path delayed every key-down for a hold-mapped key across the whole app, whether or not the focused element cared about holds, and it keyed its pending timeout on `e.key` — which webOS does not report consistently across key-down and key-up, so a short tap on Back could still fire BackHold. `useHold` is scoped to the element that owns the gesture and has none of that. Removed: - `useFocusManager(keyMap, keyHoldOptions)` second parameter - `KeyHoldOptions`, `KeyHoldMap`, `DefaultKeyHoldMap` types - `onKeyHold` from `FocusNode`, and `EventHandlers` from `NodeProps` (which is what supplied `onEnterHold` and friends) - the `isHold` argument threaded through `propagateKeyPress`/`runBubblePhase` Migration: move a `userKeyHoldMap` entry onto the element that owns the gesture. // before useFocusManager(keyMap, { userKeyHoldMap: { EnterHold: 'Enter' }, holdThreshold: 1000 }) // after const [holdEnter, releaseEnter] = useHold({ onHold: openMenu, onEnter: openTile, holdThreshold: 1000, }) Co-Authored-By: Claude Opus 5 --- docs/primitives/useFocusManager.md | 41 ++++++-------- docs/primitives/useHold.md | 2 +- docs/primitives/useMouse.md | 2 +- src/core/focusKeyTypes.ts | 18 ------ src/core/focusManager.ts | 88 +++++++----------------------- src/core/intrinsicTypes.ts | 8 +-- src/primitives/useFocusManager.ts | 1 - src/primitives/useHold.ts | 2 +- 8 files changed, 41 insertions(+), 121 deletions(-) diff --git a/docs/primitives/useFocusManager.md b/docs/primitives/useFocusManager.md index ee603e0..65fbf8f 100644 --- a/docs/primitives/useFocusManager.md +++ b/docs/primitives/useFocusManager.md @@ -22,13 +22,6 @@ const App = () => { Enter: 'Enter', Last: 'l', }, - // Second param is keyHoldMapEntries - { - userKeyHoldMap: { - EnterHold: 'Enter', - }, - holdThreshold: 150, //ms for how long to hold for - }, ); // Additional application logic... @@ -183,19 +176,27 @@ Note: There is no generic `onKeyRelease`. ### Hold Key Handling -Recommended approach to Hold Key Handling is with the [useHold](./useHold.md) primitive as this will not delay any keypress events for elements which do not need Hold. - -#### DEPRECATED - keyHold will be replaced with useHold +Hold gestures are handled by the [useHold](./useHold.md) primitive, which is +scoped to the elements that need it rather than delaying key-press events +globally. -You can specify which keys you'd like tracked for Hold events globally as the second param to `useFocusManager`. +The global `keyHoldOptions` / `userKeyHoldMap` second parameter has been removed, +along with the `onKeyHold` and `on${Key}Hold` handlers it dispatched. Move a +`userKeyHoldMap` entry to `useHold` on the element that owns the gesture: -1. The `keyHoldMap` looks for the key name and its corresponding value. -2. It calls the `on${keyHold}` handler after `holdThreshold` || 500 ms. -3. If the key is not handled, it calls the generic `onKeyHold` on the active element and then propagates up through the focus path until the key press is handled. +```tsx +// Before: useFocusManager(keyMap, { userKeyHoldMap: { EnterHold: 'Enter' }, holdThreshold: 1000 }) +// -The keyHandler signature is: `(this: ElementNode, e: Event, elm: ElementNode, finalFocusedElm: ElementNode) => boolean` +// After: +const [holdEnter, releaseEnter] = useHold({ + onHold: openMenu, + onEnter: openTile, + holdThreshold: 1000, +}); -To stop the propagation of a key press, the handler must return `true`. Any other return value or no return value will continue to propagate the key press through the focus path, looking for additional handlers. +; +``` ### Custom Key Mappings @@ -242,13 +243,7 @@ const App = () => { Up: ["ArrowUp", 38], Down: ["ArrowDown", 40], Enter: ["Enter", 13], - } as unknown as KeyMap, { - userKeyHoldMap: { - EnterHold: [ 'Enter', 13 ], - BackHold: [ 'b', 66 ], - } as unknown as KeyHoldMap, - holdThreshold: 1000, - }); + } as unknown as KeyMap); return ( diff --git a/docs/primitives/useHold.md b/docs/primitives/useHold.md index 59efb03..21d8284 100644 --- a/docs/primitives/useHold.md +++ b/docs/primitives/useHold.md @@ -156,7 +156,7 @@ remotes that swallow key-up. On a platform whose remote input layer delivers no auto-repeat at all, that makes a hold unreachable. Set `holdRequiresRepeat: false` to resolve the ambiguous case -as a hold instead, matching the legacy `keyHoldOptions` behavior: +as a hold instead, matching the behavior of the removed `keyHoldOptions`: ```tsx const [holdEnter, releaseEnter] = useHold({ diff --git a/docs/primitives/useMouse.md b/docs/primitives/useMouse.md index 978be3b..b955469 100644 --- a/docs/primitives/useMouse.md +++ b/docs/primitives/useMouse.md @@ -43,7 +43,7 @@ To use this, pass the `customStates` option to `useMouse`. When an element is clicked, `useMouse` handles the interaction as follows: 1. **`onMouseClick`**: If the element has an `onMouseClick` handler, it is called directly with the mouse event and the element instance. `onMouseClick` is mouse-specific and never fires from a keyboard/remote Enter. -2. **Everything else**: The element is focused (`setFocus()`) and a synthetic `Enter` key event (keydown + keyup) is dispatched. This routes through the `focusManager` exactly like a remote/keyboard Enter press — the capture phase, leaf→root bubbling, return-value propagation (a handler returning `true` stops it), per-element throttling, and keyHold all apply. In practice this means a click and a remote Enter invoke the same `onEnter` path. +2. **Everything else**: The element is focused (`setFocus()`) and a synthetic `Enter` key event (keydown + keyup) is dispatched. This routes through the `focusManager` exactly like a remote/keyboard Enter press — the capture phase, leaf→root bubbling, return-value propagation (a handler returning `true` stops it), and per-element throttling all apply. In practice this means a click and a remote Enter invoke the same `onEnter` path. Example of handling clicks: diff --git a/src/core/focusKeyTypes.ts b/src/core/focusKeyTypes.ts index 8357c52..6504072 100644 --- a/src/core/focusKeyTypes.ts +++ b/src/core/focusKeyTypes.ts @@ -28,13 +28,6 @@ export interface FocusNode { handlerElm: ElementNode, currentFocusedElm: ElementNode, ) => KeyHandlerReturn; - onKeyHold?: ( - this: ElementNode, - e: KeyboardEvent, - mappedKeyEvent: string | undefined, - handlerElm: ElementNode, - currentFocusedElm: ElementNode, - ) => KeyHandlerReturn; } export type KeyNameOrKeyCode = string | number; @@ -52,10 +45,6 @@ export interface KeyMap extends DefaultKeyMap { [key: string]: KeyNameOrKeyCode | KeyNameOrKeyCode[] | null; } -export interface DefaultKeyHoldMap { - EnterHold: KeyNameOrKeyCode | KeyNameOrKeyCode[] | null; -} - export type EventHandlers = { [K in keyof Map as `on${Capitalize}`]?: KeyHandler; } & { @@ -67,8 +56,6 @@ export type EventHandlers = { onCaptureKeyRelease?: KeyHandler; }; -export interface KeyHoldMap extends DefaultKeyHoldMap {} - export type KeyHandlerReturn = boolean | void; export type KeyHandler = ( @@ -83,8 +70,3 @@ export type ForwardFocusHandler = ( this: ElementNode, elm: ElementNode, ) => boolean | void; - -export type KeyHoldOptions = { - userKeyHoldMap: Partial; - holdThreshold?: number; -}; diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index 0dfe135..8a6f412 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -3,11 +3,7 @@ import { Config, isDev } from './config.js'; import { IRendererNode } from './dom-renderer/domRendererTypes.js'; export type * from './focusKeyTypes.js'; import { ElementNode } from './elementNode.js'; -import type { - KeyNameOrKeyCode, - KeyHoldOptions, - KeyMap, -} from './focusKeyTypes.js'; +import type { KeyNameOrKeyCode, KeyMap } from './focusKeyTypes.js'; import { isFunction } from './utils.js'; import { activeElement, @@ -30,9 +26,7 @@ const keyMapEntries: KeyMapEntries = { Escape: 'Escape', }; -const keyHoldMapEntries: Record = { - // Enter: 'EnterHold', -}; +const keyOf = (e: KeyboardEvent): KeyNameOrKeyCode => e.key || e.keyCode; const flattenKeyMap = ( keyMap: Partial, @@ -333,7 +327,6 @@ const runBubblePhase = ( fp: ElementNode[], e: KeyboardEvent, mappedEvent: string | undefined, - isHold: boolean, isUp: boolean, sameKey: boolean, currentTime: number, @@ -344,11 +337,9 @@ const runBubblePhase = ( ? `on${mappedEvent}Release` : `on${mappedEvent}` : undefined; - const fallbackHandlerKey: 'onKeyHold' | 'onKeyPress' | undefined = isUp + const fallbackHandlerKey: 'onKeyPress' | undefined = isUp ? undefined - : isHold - ? 'onKeyHold' - : 'onKeyPress'; + : 'onKeyPress'; let lastHandlerSeen: ElementNode | undefined; @@ -387,11 +378,10 @@ const runBubblePhase = ( const propagateKeyPress = ( e: KeyboardEvent, mappedEvent?: string, - isHold: boolean = false, isUp: boolean = false, ): boolean => { const currentTime = performance.now(); - const key = e.key || e.keyCode; + const key = keyOf(e); const sameKey = lastInputKey === key; lastInputKey = key; @@ -426,7 +416,6 @@ const propagateKeyPress = ( fp, e, mappedEvent, - isHold, isUp, sameKey, currentTime, @@ -434,7 +423,7 @@ const propagateKeyPress = ( if (handled) return true; if (isDev && Config.keyDebug && !isUp) { - const detail = `key="${e.key}", mappedEvent=${mappedEvent}, isHold=${isHold}, isUp=${isUp}`; + const detail = `key="${e.key}", mappedEvent=${mappedEvent}, isUp=${isUp}`; if (lastHandlerSeen) { console.log(`Keypress bubbled, ${detail}`, lastHandlerSeen); } else { @@ -445,11 +434,6 @@ const propagateKeyPress = ( return false; }; -const DEFAULT_KEY_HOLD_THRESHOLD = 500; // ms -const keyHoldTimeouts: { [key: KeyNameOrKeyCode]: number | true } = {}; - -const keyOf = (e: KeyboardEvent): KeyNameOrKeyCode => e.key || e.keyCode; - // `key` is not a stable identity for a physical key across key-down and key-up. // webOS reports Back's key-down as { key: 'GoBack', keyCode: 461 } and its // key-up as { key: 'Unidentified', keyCode: 461 } — same key, two names, with @@ -543,13 +527,8 @@ export const releaseKeySuppression = ( liftSuppression(keyIdentities(keyOrEvent)); }; -const handleKeyEvents = ( - delay: number, - keydown?: KeyboardEvent, - keyup?: KeyboardEvent, -) => { +const handleKeyEvents = (keydown?: KeyboardEvent, keyup?: KeyboardEvent) => { if (keydown) { - const key: KeyNameOrKeyCode = keyOf(keydown); const ids = keyIdentities(keydown); if (keydown.repeat) { if (findSuppression(ids)) return; @@ -558,52 +537,29 @@ const handleKeyEvents = ( // though its key-up never arrived. Settle it before handling this press. liftSuppression(ids); } - const mappedKeyHoldEvent = - keyHoldMapEntries[keydown.key] || keyHoldMapEntries[keydown.keyCode]; - const mappedKeyEvent = - keyMapEntries[keydown.key] || keyMapEntries[keydown.keyCode]; - if (mappedKeyHoldEvent) { - if (!keyHoldTimeouts[key]) { - keyHoldTimeouts[key] = window.setTimeout(() => { - keyHoldTimeouts[key] = true; - propagateKeyPress(keydown, mappedKeyHoldEvent, true); - }, delay); - } - return; - } - propagateKeyPress(keydown, mappedKeyEvent, false); + propagateKeyPress( + keydown, + keyMapEntries[keydown.key] || keyMapEntries[keydown.keyCode], + ); } else if (keyup) { - const key: KeyNameOrKeyCode = keyOf(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)); - const mappedKeyEvent = - keyMapEntries[keyup.key] || keyMapEntries[keyup.keyCode]; - if (keyHoldTimeouts[key] === true) { - delete keyHoldTimeouts[key]; - } else if (keyHoldTimeouts[key]) { - clearTimeout(keyHoldTimeouts[key]); - delete keyHoldTimeouts[key]; - // trigger key down event when hold didn't finish - propagateKeyPress(keyup, mappedKeyEvent, false); - } - propagateKeyPress(keyup, mappedKeyEvent, false, true); + propagateKeyPress( + keyup, + keyMapEntries[keyup.key] || keyMapEntries[keyup.keyCode], + true, + ); } }; -export const useFocusManager = ( - userKeyMap?: Partial, - keyHoldOptions?: KeyHoldOptions, -) => { +export const useFocusManager = (userKeyMap?: Partial) => { if (userKeyMap) { flattenKeyMap(userKeyMap, keyMapEntries); } - if (keyHoldOptions?.userKeyHoldMap) { - flattenKeyMap(keyHoldOptions.userKeyHoldMap, keyHoldMapEntries); - } // Capture the calling owner so signal updates and key-event reactions // can run inside it — needed for programmatic .setFocus(), post-mutation @@ -619,13 +575,10 @@ export const useFocusManager = ( Config.setActiveElement = (elm) => ownerContext(() => setActiveElementSignal(elm)); - const delay = keyHoldOptions?.holdThreshold || DEFAULT_KEY_HOLD_THRESHOLD; - const runKeyEvent = handleKeyEvents.bind(null, delay); - const keyPressHandler = (event: KeyboardEvent) => - ownerContext(() => runKeyEvent(event, undefined)); + ownerContext(() => handleKeyEvents(event, undefined)); const keyUpHandler = (event: KeyboardEvent) => - ownerContext(() => runKeyEvent(undefined, event)); + ownerContext(() => handleKeyEvents(undefined, event)); document.addEventListener('keydown', keyPressHandler); document.addEventListener('keyup', keyUpHandler); @@ -633,9 +586,6 @@ export const useFocusManager = ( onCleanup(() => { document.removeEventListener('keydown', keyPressHandler); document.removeEventListener('keyup', keyUpHandler); - for (const timeout of Object.values(keyHoldTimeouts)) { - if (timeout && timeout !== true) clearTimeout(timeout); - } suppressedKeys.clear(); }); }; diff --git a/src/core/intrinsicTypes.ts b/src/core/intrinsicTypes.ts index b4e31eb..4170ff0 100644 --- a/src/core/intrinsicTypes.ts +++ b/src/core/intrinsicTypes.ts @@ -10,12 +10,7 @@ import { ShaderRoundedProps, ShaderShadowProps, } from './shaders.js'; -import { - EventHandlers, - DefaultKeyMap, - KeyHoldMap, - FocusNode, -} from './focusKeyTypes.js'; +import { EventHandlers, DefaultKeyMap, FocusNode } from './focusKeyTypes.js'; import type { JSXElement } from 'solid-js'; export type AnimationSettings = Partial; @@ -123,7 +118,6 @@ export interface NodeProps extends RendererNode, EventHandlers, - EventHandlers, FocusNode, Partial< NewOmit< diff --git a/src/primitives/useFocusManager.ts b/src/primitives/useFocusManager.ts index f1c7298..313267f 100644 --- a/src/primitives/useFocusManager.ts +++ b/src/primitives/useFocusManager.ts @@ -5,6 +5,5 @@ export { suppressKeyUntilRelease, releaseKeySuppression, type KeyMap, - type KeyHoldOptions, } from '../core/focusManager.js'; export { activeElement, setActiveElement } from '../core/activeElement.js'; diff --git a/src/primitives/useHold.ts b/src/primitives/useHold.ts index 9442f20..190a972 100644 --- a/src/primitives/useHold.ts +++ b/src/primitives/useHold.ts @@ -31,7 +31,7 @@ export type UseHoldProps = { * * `true` (default): a press with neither key-up nor auto-repeat by the * threshold resolves as a tap. `false`: it resolves as a hold, matching the - * legacy `keyHoldOptions` behavior. Set this to `false` on platforms whose + * behavior of the removed `keyHoldOptions`. Set this to `false` on platforms whose * remote input layer delivers no auto-repeat, where a hold gesture would * otherwise be unreachable. */ From c85fc2b8106b105ec8f15e0e1301d9482bfed663 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 22:40:47 -0400 Subject: [PATCH 09/10] docs(focus): correct useFocusManager/useHold, note keys that can't report a hold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doc audit against the source turned up several inaccuracies: - `useFocusManager` and `focusPath` are exported from `@solidtv/solid/primitives`, not `@solidtv/solid`, and `useFocusManager` returns nothing — the docs showed `const focusPath = useFocusManager(...)` imported from the main entry. - Capture handlers are `onCapture${Key}` / `onCaptureKey`; the docs named them `capture${key}` / `captureKey`. - The bubble phase is a single interleaved walk — `onKeyPress` on an element is tried before its parent's `on${Key}` — not a second pass over the whole tree. - The key handler signature omitted the capture-phase `mappedEvent` argument and claimed a `boolean` return rather than `boolean | void`. `onKeyPress` takes the mapped event name as its second argument, which was undocumented. - The listed default key map included numeric keyCodes that are not mapped by default and omitted `l: 'Last'`. Since nothing maps keyCodes out of the box, that mattered for devices reporting keys by code. - Key release skipped the capture phase; `onCaptureKeyRelease` is the only catch-all for a release. - `printFocusHistory` / `getFocusHistory` were documented but exported from neither entry point. Export them from primitives alongside the rest of the focus API, and drop the claim that `printFocusHistory` is callable from the DevTools console — only `$f` is attached to `window`. Also document that only Left/Right/Up/Down/Enter/Last have typed handler props; `onBack` and friends are only accepted via ElementNode's index signature. For useHold, replace the "platforms without auto-repeat" guidance, which assumed a key that emits key-up on real release. Testing on an LG remote shows Back emits key-down and key-up together at press time, so tap and hold are indistinguishable and no setting recovers the gesture — `holdRequiresRepeat: false` cannot help, because the immediate key-up cancels the timer before it fires. Document the limitation, tabulate which signals do and don't permit hold detection, and give a snippet for checking a key on real hardware. Co-Authored-By: Claude Opus 5 --- docs/primitives/useFocusManager.md | 121 ++++++++++++++++++++--------- docs/primitives/useHold.md | 77 +++++++++++++----- src/primitives/useFocusManager.ts | 3 + src/primitives/useHold.ts | 10 ++- 4 files changed, 151 insertions(+), 60 deletions(-) diff --git a/docs/primitives/useFocusManager.md b/docs/primitives/useFocusManager.md index 65fbf8f..8943b9f 100644 --- a/docs/primitives/useFocusManager.md +++ b/docs/primitives/useFocusManager.md @@ -9,20 +9,19 @@ The `useFocusManager` primitive is designed to handle user input, manage focus p Import the `useFocusManager` and configure it with your custom key mappings: ```jsx -import { useFocusManager } from '@solidtv/solid'; +import { useFocusManager } from '@solidtv/solid/primitives'; const App = () => { - const focusPath = useFocusManager( - // These are the default, so you can just call useFocusManager() - { - Left: ['ArrowLeft', 37], - Right: ['ArrowRight', 39], - Up: ['ArrowUp', 38], - Down: ['ArrowDown', 40], - Enter: 'Enter', - Last: 'l', - }, - ); + // The defaults are already applied, so you can just call useFocusManager(). + // Anything you pass is merged over them. + useFocusManager({ + Left: ['ArrowLeft', 37], + Right: ['ArrowRight', 39], + Up: ['ArrowUp', 38], + Down: ['ArrowDown', 40], + Enter: 'Enter', + Last: 'l', + }); // Additional application logic... }; @@ -30,7 +29,13 @@ const App = () => { ### Focus Path Tracking -The `useFocusManager` returns a signal, `focusPath`, which is an array of elements that currently have focus. When the `activeElement` changes, the focus path is recalculated. During this process: +`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: + +```jsx +import { useFocusManager, focusPath } from '@solidtv/solid/primitives'; +``` + +When the `activeElement` changes, the focus path is recalculated. During this process: - All elements in focus will have a `focus` state added, and `onFocus(currentFocusedElm, prevFocusedElm, nodeWithCallback)` event is called. - Elements losing focus will have the `focus` state removed, and `onBlur(currentFocusedElm, prevFocusedElm, nodeWithCallback)` event is called. @@ -46,12 +51,26 @@ return {/* use hasFocus() */}; When a key is pressed: -1. The `keyMap` looks for the key name and its corresponding value. -2. It then looks for `capture${key}` and `captureKey` from top down. -3. It then calls the `on${key}` handler, searching from focused element back up the tree. -4. If the key is not handled, it calls the generic `onKeyPress` on the active element and then propagates up through the focus path until the key press is handled. +1. The `keyMap` resolves the event's `key` (falling back to its `keyCode`) to a mapped event name, e.g. `ArrowLeft` → `Left`. A key with no mapping still propagates, but only to the generic handlers. +2. **Capture phase**, root → focused leaf: on each element it looks for `onCapture${key}`, then `onCaptureKey`. If the mapping failed, the raw `e.key` is used in place of `${key}`. +3. **Bubble phase**, focused leaf → root: on each element it looks for `on${key}`, then falls back to `onKeyPress` on that _same_ element before moving to its parent. + +Note that step 3 is a single interleaved walk — `onKeyPress` on an element is tried before its parent's `on${key}`, not as a separate pass after the whole tree. -The keyHandler signature is: `(this: ElementNode, e: Event, elm: ElementNode, finalFocusedElm: ElementNode) => boolean` +The key handler signature is: + +```ts +type KeyHandler = ( + this: ElementNode, + e: KeyboardEvent, + target: ElementNode, // the element whose handler is running + handlerElm: ElementNode, // the focused leaf element + mappedEvent?: string, // capture-phase handlers only +) => boolean | void; +``` + +`onKeyPress` takes the mapped event name as its second argument instead: +`(e, mappedKeyEvent, handlerElm, currentFocusedElm)`. To stop the propagation of a key press, the handler must return `true`. Any other return value or no return value will continue to propagate the key press through the focus path, looking for additional handlers. @@ -128,15 +147,18 @@ After every `printFocusHistory` call, `window.$f` is set to the DOM div of the m #### Manual printing (`printFocusHistory`) -`printFocusHistory(n)` can be called at any time — including directly from the browser DevTools console — to print the last N entries. `count` is required. +`printFocusHistory(n)` prints the last N entries at any time. `n` is required. ```javascript -import { printFocusHistory } from '@solidtv/solid'; +import { printFocusHistory } from '@solidtv/solid/primitives'; printFocusHistory(20); +``` -// Also works directly in the browser DevTools console (no import needed once the app is running) -printFocusHistory(20); +It is not attached to `window`, so it is not callable from the DevTools console on its own. If you want it there, assign it yourself during dev setup: + +```javascript +if (import.meta.env.DEV) window.printFocusHistory = printFocusHistory; ``` #### Inspecting the buffer programmatically (`getFocusHistory`) @@ -144,7 +166,10 @@ printFocusHistory(20); `getFocusHistory()` returns the full ring buffer as a read-only array of `FocusHistoryEntry` objects. This is useful for custom devtools panels, automated tests, or sending focus traces to a logging service. ```typescript -import { getFocusHistory, type FocusHistoryEntry } from '@solidtv/solid'; +import { + getFocusHistory, + type FocusHistoryEntry, +} from '@solidtv/solid/primitives'; const history: Readonly = getFocusHistory(); ``` @@ -169,10 +194,11 @@ Per-element metadata (focus count, last focused timestamp) is stored in a `WeakM On release of a key: -1. The `keyMap` looks for the key name and its corresponding value. -2. It calls the `on${key}Release` handler first. +1. The `keyMap` resolves the key to a mapped event name, as for a key press. +2. **Capture phase**, root → leaf: `onCapture${key}Release`, then `onCaptureKeyRelease`. +3. **Bubble phase**, leaf → root: `on${key}Release`. -Note: There is no generic `onKeyRelease`. +Note: there is no generic `onKeyRelease` in the bubble phase — `onKeyPress` is not called for key-ups. `onCaptureKeyRelease` is the only catch-all for a release. ### Hold Key Handling @@ -200,27 +226,46 @@ const [holdEnter, releaseEnter] = useHold({ ### Custom Key Mappings -You can pass in an array of keys for a single event. The custom keys object will be merged with the default key mapping: +You can pass in an array of keys for a single event. What you pass is written over the default mapping, so you only need to declare what differs. + +Note the direction: the map you pass is `{ EventName: key(s) }`, while the table it merges into is keyed the other way, `{ key: EventName }`. These are the defaults: ```js -const defaultKeyMap = { +{ ArrowLeft: 'Left', ArrowRight: 'Right', ArrowUp: 'Up', ArrowDown: 'Down', Enter: 'Enter', + l: 'Last', ' ': 'Space', Backspace: 'Back', Escape: 'Escape', - 37: 'Left', - 39: 'Right', - 38: 'Up', - 40: 'Down', - 13: 'Enter', - 32: 'Space', - 8: 'Back', - 27: 'Escape', -}; +} +``` + +**No numeric keyCodes are mapped by default.** Devices that report keys by keyCode — or that report a different `key` on key-down and key-up, as LG does for Back — need those added explicitly: + +```js +useFocusManager({ + Back: [461, 'GoBack', 'Backspace'], + Enter: ['Enter', 13], +}); +``` + +#### Typing of custom handlers + +Only `Left`, `Right`, `Up`, `Down`, `Enter` and `Last` have typed handler props +(`onLeft`, `onLeftRelease`, `onCaptureLeft`, …). Handlers for any other mapping — +including the built-in `Back`, `Space` and `Escape`, and anything you add +yourself — dispatch correctly at runtime, but are only accepted by the compiler +through `ElementNode`'s `[key: string]: unknown` index signature. You get no +autocompletion and no argument checking on them, so annotate the handler itself: + +```tsx +const onBack: KeyHandler = (e, target, focused) => { ... }; + +; ``` ### Example @@ -229,7 +274,7 @@ Here's a complete example of how to use `useFocusManager`: ```jsx import { createSignal } from 'solid-js'; -import { useFocusManager } from '@solidtv/solid'; +import { useFocusManager } from '@solidtv/solid/primitives'; import { Button } from '@solidtv/solid-ui'; const App = () => { diff --git a/docs/primitives/useHold.md b/docs/primitives/useHold.md index 21d8284..a94c434 100644 --- a/docs/primitives/useHold.md +++ b/docs/primitives/useHold.md @@ -26,14 +26,14 @@ const [holdRight, releaseRight] = useHold({ #### `UseHoldProps` -| Prop | Type | Description | Default | -| --------------------------- | -------------- | --------------------------------------------------------------------------------------------------------------------------------- | ------------ | -| `onHold` | `HoldCallback` | Called once the hold threshold is exceeded. | **Required** | -| `onEnter` | `HoldCallback` | Called on press or key entry. May be delayed depending on config. | **Required** | -| `onRelease` | `HoldCallback` | Called after a successful hold is released. | `undefined` | -| `holdThreshold` | `number` | Time in milliseconds to wait before triggering `onHold`. | `500` | -| `performOnEnterImmediately` | `boolean` | Whether `onEnter` is triggered immediately or only if released early. | `false` | -| `holdRequiresRepeat` | `boolean` | Whether a hold must be confirmed by an auto-repeat key-down. See [Platforms without auto-repeat](#platforms-without-auto-repeat). | `true` | +| Prop | Type | Description | Default | +| --------------------------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------- | ------------ | +| `onHold` | `HoldCallback` | Called once the hold threshold is exceeded. | **Required** | +| `onEnter` | `HoldCallback` | Called on press or key entry. May be delayed depending on config. | **Required** | +| `onRelease` | `HoldCallback` | Called after a successful hold is released. | `undefined` | +| `holdThreshold` | `number` | Time in milliseconds to wait before triggering `onHold`. | `500` | +| `performOnEnterImmediately` | `boolean` | Whether `onEnter` is triggered immediately or only if released early. | `false` | +| `holdRequiresRepeat` | `boolean` | Whether a hold must be confirmed by an auto-repeat key-down. See [Not every key can report a hold](#not-every-key-can-report-a-hold). | `true` | Each callback receives the same context a `KeyHandler` gets — the `KeyboardEvent` that began the press, the element whose handler ran, and the focused element: @@ -147,27 +147,66 @@ useFocusManager({ Back: [461, 'GoBack'] }); --- -### Platforms without auto-repeat +### Not every key can report a hold -A hold is confirmed by an auto-repeat key-down. If a press delivers neither a -key-up nor an auto-repeat by the threshold, the press is ambiguous, and by -default it resolves as a **tap** — the behavior that keeps taps working on -remotes that swallow key-up. +A hold is detected from what the device sends between press and release. Some +keys send nothing usable, and on those a hold is **not detectable at all** — by +this primitive or any other. -On a platform whose remote input layer delivers no auto-repeat at all, that makes -a hold unreachable. Set `holdRequiresRepeat: false` to resolve the ambiguous case -as a hold instead, matching the behavior of the removed `keyHoldOptions`: +Verified on an LG remote: the Back button emits its key-down and key-up +back-to-back the instant it is pressed, before the user has let go. Nothing +distinguishes a tap from a five-second hold, because both produce exactly the +same two events at exactly the same time. + +There is no setting that recovers a hold here. `holdRequiresRepeat: false` does +not help: the key-up arrives immediately and cancels the hold timer long before +it can fire, so the press always resolves as a tap. That is the correct outcome — +the alternative would be firing `onHold` on a plain tap. + +**Design around it.** If a gesture must work on every device, do not put it on a +key that cannot report one. Put the hold on OK/Enter, which does report a real +press duration on the remotes tested, and give Back a plain `onEnter` action. + +The three signals a key can offer, in the order the primitive prefers them: + +| Signal | Hold detectable? | | +| --------------------------- | ---------------- | ------------------------------------------------------------------------------ | +| Auto-repeat key-downs | Yes | Confirms the key is still down. The default and most reliable path. | +| Key-up only on real release | Yes | No key-up by the threshold means still held — set `holdRequiresRepeat: false`. | +| Key-up immediately on press | **No** | Tap and hold are indistinguishable. LG Back behaves this way. | + +If a press delivers no key-up and no auto-repeat by the threshold, it is +ambiguous, and by default resolves as a **tap** — which is what keeps taps +working on remotes that swallow key-up entirely (webOS OK). Set +`holdRequiresRepeat: false` for a key you know reports key-up only on real +release, so the timer alone resolves the hold: ```tsx const [holdEnter, releaseEnter] = useHold({ onHold: openContextMenu, onEnter: openTile, - holdRequiresRepeat: false, // no auto-repeat on this platform + holdRequiresRepeat: false, // this key emits no auto-repeat }); ``` -An early key-up still resolves as a tap either way, so this only changes the -no-key-up **and** no-repeat case. +Confirm behavior per key on real hardware before relying on it — it varies by +key and by device, not just by platform. Logging the raw events is enough: + +```tsx + { + console.log('down', e.key, e.keyCode, e.repeat, performance.now()); + return false; + }} + onCaptureKeyRelease={(e) => { + console.log('up', e.key, e.keyCode, performance.now()); + return false; + }} +/> +``` + +If the `up` line appears at press time rather than release time, that key cannot +report a hold. --- diff --git a/src/primitives/useFocusManager.ts b/src/primitives/useFocusManager.ts index 313267f..10e192c 100644 --- a/src/primitives/useFocusManager.ts +++ b/src/primitives/useFocusManager.ts @@ -4,6 +4,9 @@ export { setActiveElementCore, suppressKeyUntilRelease, releaseKeySuppression, + printFocusHistory, + getFocusHistory, + type FocusHistoryEntry, type KeyMap, } from '../core/focusManager.js'; export { activeElement, setActiveElement } from '../core/activeElement.js'; diff --git a/src/primitives/useHold.ts b/src/primitives/useHold.ts index 190a972..82b87e6 100644 --- a/src/primitives/useHold.ts +++ b/src/primitives/useHold.ts @@ -31,9 +31,13 @@ export type UseHoldProps = { * * `true` (default): a press with neither key-up nor auto-repeat by the * threshold resolves as a tap. `false`: it resolves as a hold, matching the - * behavior of the removed `keyHoldOptions`. Set this to `false` on platforms whose - * remote input layer delivers no auto-repeat, where a hold gesture would - * otherwise be unreachable. + * behavior of the removed `keyHoldOptions`. + * + * Set this to `false` for a key that emits no auto-repeat but does emit + * key-up on real release: the timer alone can then resolve the hold. It does + * *not* rescue a key that emits key-up immediately at press time (LG's Back + * button does this) — that key-up cancels the timer before it fires, and no + * setting can distinguish such a tap from a hold. */ holdRequiresRepeat?: boolean; }; From c9e3f662a84b365600202e12650ac4602005ea2d Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Sun, 2 Aug 2026 22:57:17 -0400 Subject: [PATCH 10/10] Release 1.5.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2970ce1..2f1839b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@solidtv/solid", - "version": "1.4.1-2", + "version": "1.5.0", "description": "SolidTV", "type": "module", "exports": {