From b3ee6831d4e75e4015e74043f4efacc0237134dd Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Fri, 23 Oct 2026 09:52:00 -0400 Subject: [PATCH] feat: Config.inferKeyRepeat for devices that never set KeyboardEvent.repeat Some devices send a stream of repeat=false key-downs while a key is held, and a single key-up on release. useHold saw each as a new press, so a hold never fired, and the hold latch was lifted by what looked like a fresh press. With Config.inferKeyRepeat set (ms), the focus manager marks a key-down as a repeat when the same key sent a key-down within the window with no key-up in between. The window runs from the previous key-down, so a missed key-up cannot wedge a key. Off by default. Co-Authored-By: Claude Opus 5.5 --- docs/primitives/useFocusManager.md | 26 +++ docs/primitives/useHold.md | 8 + src/core/config.ts | 11 ++ src/core/focusManager.ts | 69 ++++++- tests/keySuppression.test.tsx | 298 +++++++++++++++++++++++++---- 5 files changed, 362 insertions(+), 50 deletions(-) diff --git a/docs/primitives/useFocusManager.md b/docs/primitives/useFocusManager.md index 41b954f3..46308c10 100644 --- a/docs/primitives/useFocusManager.md +++ b/docs/primitives/useFocusManager.md @@ -137,6 +137,32 @@ Config.preventDefaultOnHandledKeys = true; It is off by default, since in a browser it changes what handled keys do: a handled arrow no longer scrolls the page, say. +### Inferred Repeats (`Config.inferKeyRepeat`) + +Some devices never set `KeyboardEvent.repeat`. Holding a key sends a stream of +key-downs that all report `repeat: false`, followed by one key-up on release. +Anything that relies on `repeat` breaks: [useHold](./useHold.md) sees each +key-down as a new press, and a key latched by a hold is released by what looks +like a fresh press. + +Set `Config.inferKeyRepeat` to a window in milliseconds and the focus manager +marks a key-down as a repeat when the same key sent a key-down less than that +long ago with no key-up in between. Handlers then read `e.repeat` as `true`. + +```javascript +import { Config } from '@solidtv/solid'; + +Config.inferKeyRepeat = 700; +``` + +The window is measured from the key's previous key-down, so it only has to +cover the device's initial auto-repeat delay. A key-up that never arrives stops +mattering once the window passes, so a lost key-up cannot leave the key stuck. + +It is off by default; enable it only on the devices that need it. On a key +that never sends a key-up (webOS OK), a second tap inside the window would be +read as a repeat. + ### Focus History Logging Focus history logging records each focus change — whether triggered by a key press or programmatically — into a ring buffer of up to 50 entries. This is a **dev-only** feature: recording and printing are both gated behind the `isDev` flag and do nothing in production builds. diff --git a/docs/primitives/useHold.md b/docs/primitives/useHold.md index a94c4342..a8f35147 100644 --- a/docs/primitives/useHold.md +++ b/docs/primitives/useHold.md @@ -175,6 +175,14 @@ The three signals a key can offer, in the order the primitive prefers them: | 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. | +Some devices auto-repeat a held key without ever setting `repeat`: every +key-down reports `repeat: false`, and a single key-up arrives on release. To +`useHold` each of those looks like a new press that restarts the timer, so a +hold never fires and the release resolves as a tap. Set +[`Config.inferKeyRepeat`](./useFocusManager.md#inferred-repeats-configinferkeyrepeat) +on such a device and the focus manager marks them as repeats before `useHold` +sees them. + 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 diff --git a/src/core/config.ts b/src/core/config.ts index b8173c53..93daba94 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -73,6 +73,17 @@ export interface Config { lockStyles?: boolean; fontWeightAlias?: Record; throttleInput?: number; + /** + * For devices that never set `KeyboardEvent.repeat`: while a key is held + * they send a stream of `repeat=false` key-downs, and one key-up on release. + * With this set (in ms), a key-down of a key that has had no key-up since + * its previous key-down, arriving within this many ms of it, is marked as a + * repeat (`repeat` reads `true`) before anything handles it. It must cover + * the device's initial auto-repeat delay. A key-up that never arrives stops + * mattering once the window passes. Off by default: on a key that swallows + * key-up (webOS OK), a second tap inside the window would read as a repeat. + */ + inferKeyRepeat?: number; /** * Call `preventDefault()` on every key event the app consumed: a handler * returned `true`, or the focus manager dropped the press itself (a diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index d5e05f2f..91f0dc7c 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -488,18 +488,62 @@ type Suppression = { // key differently still finds it. Entries for one key share a Suppression. const suppressedKeys = new Map(); -// The suppression of an event's key: looked up under its name, then its -// keyCode (keyNameId, keyCodeId), either of which may be undefined. +// An event's entry in a map keyed by key identity: looked up under its name, +// then its keyCode (keyNameId, keyCodeId), either of which may be undefined. +const findByIdentity = ( + map: Map, + name: KeyNameOrKeyCode | undefined, + code: KeyNameOrKeyCode | undefined, +): T | undefined => { + let found: T | undefined; + if (name !== undefined) found = map.get(name); + if (found === undefined && code !== undefined) found = map.get(code); + return found; +}; + const findSuppression = ( name: KeyNameOrKeyCode | undefined, code: KeyNameOrKeyCode | undefined, -): Suppression | undefined => { - let found: Suppression | undefined; - if (name !== undefined) found = suppressedKeys.get(name); - if (found === undefined && code !== undefined) { - found = suppressedKeys.get(code); +): Suppression | undefined => findByIdentity(suppressedKeys, name, code); + +// Keys down with no key-up yet, for Config.inferKeyRepeat: when each last sent +// a key-down. Indexed under every identity like suppressedKeys, so a key-up +// naming the key differently (webOS Back) still ends the press. +type HeldKey = { ids: KeyNameOrKeyCode[]; time: number }; +const heldKeys = new Map(); + +// Marks a key-down as a repeat when it follows a key-down of the same key, +// with no key-up between, by less than `windowMs`. KeyboardEvent.repeat is a +// read-only getter; an own property shadows it, so handlers, useHold and the +// suppression check below all read the same value. +const inferRepeat = ( + e: KeyboardEvent, + name: KeyNameOrKeyCode | undefined, + code: KeyNameOrKeyCode | undefined, + windowMs: number, +): void => { + const now = performance.now(); + const held = findByIdentity(heldKeys, name, code); + if (held === undefined) { + const ids = keyIdentities(e); + const entry: HeldKey = { ids, time: now }; + for (let i = 0; i < ids.length; i++) heldKeys.set(ids[i]!, entry); + return; } - return found; + if (!e.repeat && now - held.time < windowMs) { + Object.defineProperty(e, 'repeat', { value: true }); + } + held.time = now; +}; + +const releaseHeldKey = ( + name: KeyNameOrKeyCode | undefined, + code: KeyNameOrKeyCode | undefined, +): void => { + const held = findByIdentity(heldKeys, name, code); + if (!held) return; + const ids = held.ids; + for (let i = 0; i < ids.length; i++) heldKeys.delete(ids[i]!); }; const liftSuppression = (found: Suppression | undefined): void => { @@ -559,6 +603,9 @@ const handleKeyEvents = ( if (keydown) { const name = keyNameId(keydown); const code = keyCodeId(keydown); + if (Config.inferKeyRepeat) { + inferRepeat(keydown, name, code, Config.inferKeyRepeat); + } if (keydown.repeat) { if (findSuppression(name, code)) return true; } else { @@ -576,7 +623,10 @@ 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. - liftSuppression(findSuppression(keyNameId(keyup), keyCodeId(keyup))); + const name = keyNameId(keyup); + const code = keyCodeId(keyup); + releaseHeldKey(name, code); + liftSuppression(findSuppression(name, code)); return propagateKeyPress( keyup, @@ -696,5 +746,6 @@ export const useFocusManager = ( eventTarget.removeEventListener('keydown', keyPressHandler); eventTarget.removeEventListener('keyup', keyUpHandler); suppressedKeys.clear(); + heldKeys.clear(); }); }; diff --git a/tests/keySuppression.test.tsx b/tests/keySuppression.test.tsx index bdcc8591..1f9acec9 100644 --- a/tests/keySuppression.test.tsx +++ b/tests/keySuppression.test.tsx @@ -5,10 +5,13 @@ import { releaseKeySuppression, useHold, } from '@solidtv/solid/primitives'; +import { Config } from '@solidtv/solid'; import { renderer, waitForUpdate } from './setup.js'; const keydown = (repeat = false) => - document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', repeat })); + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', repeat }), + ); const keyup = () => document.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); @@ -61,23 +64,26 @@ v.describe('key suppression', () => { 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(); + 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); + 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); + // 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(); - }); + dispose(); + }, + ); v.test('never suppresses non-repeat key-downs', async () => { const { onEnter, dispose } = await setup(); @@ -131,22 +137,25 @@ const backUp = () => ); 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(); + 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, - ); + 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); + // Matched on keyCode 461, since 'Unidentified' names no key. + backUp(); + v.assert.equal(onRelease.mock.calls.length, 1); - dispose(); - }); + dispose(); + }, + ); v.test('suppression is not left latched after such a key-up', async () => { const seen: string[] = []; @@ -242,24 +251,231 @@ v.describe('key identity across key-down and key-up', () => { } }); - v.test('does not conflate two keys that both report Unidentified', async () => { - const { dispose } = await setup(); - const backRelease = v.vi.fn(); + 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, - ); + 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(); + }, + ); +}); + +// Some devices never set `repeat`: holding a key sends a stream of +// `repeat=false` key-downs and a single key-up on release. Verbatim pattern from +// a device log: +// keydown Enter 13 repeat=false (×N while held) +// keyup Enter 13 repeat=false (once, on release) +const enterDown = () => + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', keyCode: 13 }), + ); +const enterUp = () => + document.dispatchEvent( + new KeyboardEvent('keyup', { key: 'Enter', keyCode: 13 }), + ); + +// Renders a focused view that records whether each key-down it sees is a +// repeat, then switches to fake timers so tests control the repeat window. +async function setupRecorder() { + const seen: string[] = []; + const { dispose } = await setup({ + onCaptureKey: (e: KeyboardEvent) => { + seen.push(`${String(e.key)}${e.repeat ? ':repeat' : ''}`); + return false; + }, + }); + v.vi.useFakeTimers(); + return { + seen, + dispose: () => { + v.vi.useRealTimers(); + dispose(); + }, + }; +} + +v.describe('inferred key repeat (Config.inferKeyRepeat)', () => { + v.afterEach(() => { + Config.inferKeyRepeat = undefined; + releaseKeySuppression('Enter'); + }); + + v.test('is off by default: every key-down is a fresh press', async () => { + const { seen, dispose } = await setupRecorder(); + + enterDown(); + v.vi.advanceTimersByTime(50); + enterDown(); + + v.assert.deepEqual(seen, ['Enter', 'Enter']); + dispose(); + }); + + v.test( + 'marks a key-down of a held key within the window as a repeat', + async () => { + Config.inferKeyRepeat = 700; + const { seen, dispose } = await setupRecorder(); + + enterDown(); + v.vi.advanceTimersByTime(500); // the device's initial repeat delay + enterDown(); + v.vi.advanceTimersByTime(50); + enterDown(); + + v.assert.deepEqual(seen, ['Enter', 'Enter:repeat', 'Enter:repeat']); + dispose(); + }, + ); + + v.test('a key-up ends the press, so the next key-down is fresh', async () => { + Config.inferKeyRepeat = 700; + const { seen, dispose } = await setupRecorder(); + + enterDown(); + enterUp(); + v.vi.advanceTimersByTime(50); + enterDown(); + + v.assert.deepEqual(seen, ['Enter', 'Enter']); + dispose(); + }); + + v.test('a missed key-up cannot wedge the key past the window', async () => { + Config.inferKeyRepeat = 700; + const { seen, dispose } = await setupRecorder(); + + enterDown(); // key-up never arrives + v.vi.advanceTimersByTime(700); + enterDown(); + + v.assert.deepEqual(seen, ['Enter', 'Enter']); + dispose(); + }); + + v.test( + 'the window runs from the previous key-down, not the first', + async () => { + Config.inferKeyRepeat = 700; + const { seen, dispose } = await setupRecorder(); + + enterDown(); + for (let i = 0; i < 20; i++) { + v.vi.advanceTimersByTime(100); + enterDown(); + } + + v.assert.equal(seen.length, 21); + v.assert.equal(seen.filter((s) => s === 'Enter').length, 1); + dispose(); + }, + ); + + v.test('does not conflate different keys', async () => { + Config.inferKeyRepeat = 700; + const { seen, dispose } = await setupRecorder(); - // A different physical key, also anonymised on key-up. + enterDown(); document.dispatchEvent( - new KeyboardEvent('keyup', { key: 'Unidentified', keyCode: 462 }), + new KeyboardEvent('keydown', { key: 'ArrowDown', keyCode: 40 }), ); - v.assert.equal(backRelease.mock.calls.length, 0); - - backUp(); - v.assert.equal(backRelease.mock.calls.length, 1); + v.assert.deepEqual(seen, ['Enter', 'ArrowDown']); dispose(); }); + + v.test( + 'a key-up naming the key differently still ends the press', + async () => { + Config.inferKeyRepeat = 700; + const { seen, dispose } = await setupRecorder(); + + backDown(); + backUp(); // Unidentified/461 + backDown(); + + v.assert.deepEqual(seen, ['GoBack', 'GoBack']); + dispose(); + }, + ); + + v.test('useHold resolves a hold on such a device end to end', async () => { + Config.inferKeyRepeat = 700; + const onHold = v.vi.fn(); + const onEnter = v.vi.fn(); + const onRelease = v.vi.fn(); + const reached: string[] = []; + + const dispose = renderer.render(() => { + useFocusManager(); + const [startHold, releaseHold] = useHold({ + onHold, + onEnter, + onRelease, + holdThreshold: 500, + }); + return ( + { + reached.push(e.repeat ? 'repeat' : 'press'); + return false; + }} + onEnter={startHold} + onEnterRelease={releaseHold} + /> + ); + }) as unknown as () => void; + await waitForUpdate(); + await new Promise((resolve) => setTimeout(resolve, 10)); + + v.vi.useFakeTimers(); + try { + // Tap. + enterDown(); + enterUp(); + v.assert.equal(onEnter.mock.calls.length, 1); + + // Hold: repeat=false key-downs every 50ms, no key-up until release. + enterDown(); + for (let i = 0; i < 12; i++) { + v.vi.advanceTimersByTime(50); + enterDown(); + } + v.assert.equal(onHold.mock.calls.length, 1); + v.assert.equal(onEnter.mock.calls.length, 1); + + // Still held: the latch must keep swallowing what the device sends. + reached.length = 0; + v.vi.advanceTimersByTime(50); + enterDown(); + v.assert.deepEqual(reached, []); + + enterUp(); + v.assert.equal(onRelease.mock.calls.length, 1); + + // The next press is fresh and gets through. + enterDown(); + v.assert.deepEqual(reached, ['press']); + } finally { + v.vi.useRealTimers(); + dispose(); + } + }); });