diff --git a/src/core/config.ts b/src/core/config.ts index 2b6e055..54e29da 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -63,6 +63,13 @@ export interface Config { * the `activeElement` signal decoupled from focus-manager logic. */ setActiveElement: (elm: ElementNode) => void; + /** + * When the focused element is removed from the tree, automatically refocus + * the nearest still-attached ancestor on its focus path (letting that + * ancestor's `forwardFocus`/`selected` logic pick a child). Prevents key + * events from dead-ending on a stale focus path. Defaults to true. + */ + focusLossRecovery: boolean; focusStateKey: DollarString; lockStyles?: boolean; fontWeightAlias?: Record; @@ -85,6 +92,7 @@ export const Config: Config = { }, convertToShader: defaultConvertToShader, setActiveElement: (elm) => setActiveElementSignal(elm), + focusLossRecovery: true, fontSettings: { fontFamily: 'Ubuntu', fontSize: 100, diff --git a/src/core/elementNode.ts b/src/core/elementNode.ts index 5408753..da6b377 100644 --- a/src/core/elementNode.ts +++ b/src/core/elementNode.ts @@ -52,6 +52,7 @@ import { NodeType, TextNode } from './nodeTypes.js'; import { ForwardFocusHandler, setActiveElementCore, + recoverActiveElement, FocusNode, } from './focusManager.js'; import { initClickInspector } from './clickInspector.js'; @@ -104,10 +105,12 @@ function runPostMutation() { postMutationQueued = false; // Phase 1: delete-flush + let didDestroy = false; if (elementDeleteQueue.length > 0) { for (const el of elementDeleteQueue) { if ((el._queueDelete ?? 0) < 0) { el.destroy(); + didDestroy = true; } el._queueDelete = undefined; } @@ -136,6 +139,19 @@ function runPostMutation() { nextActiveElement = null; setActiveElementCore(element); } + + // Phase 4: focus-loss recovery. Destroying nodes is the only way the + // active element leaves the tree, so this is free on non-delete passes. + // Skip when a focus change is pending — it applies on the next pass and + // supersedes whatever was lost. + if ( + didDestroy && + Config.focusLossRecovery && + deferredFocusElement === null && + nextActiveElement === null + ) { + recoverActiveElement(); + } } function addToLayoutQueue(node: ElementNode) { diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index e151c19..05631a3 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -202,6 +202,57 @@ export const printFocusHistory = (count: number): void => { } }; +// --------------------------------------------------------------------------- +// Focus-loss recovery +// --------------------------------------------------------------------------- + +/** + * An element is attached when every node up its parent chain is present in + * its parent's children array. `removeChild` doesn't clear `node.parent`, + * so a plain walk to the root can't detect removal — membership can. + */ +const isAttached = (elm: ElementNode): boolean => { + let current: ElementNode = elm; + while (current.parent) { + if (!current.parent.children.includes(current)) return false; + current = current.parent; + } + return true; +}; + +/** + * Called by the post-mutation scheduler after nodes were destroyed. If the + * active element was removed from the tree, refocus the nearest + * still-attached ancestor on its focus path — that ancestor's + * `forwardFocus`/`selected` logic then picks a sensible child. Without this, + * key events walk a stale focus path and navigation dead-ends. + */ +export const recoverActiveElement = (): void => { + const active = activeElement(); + if (!active || isAttached(active)) return; + + const fp = focusPath(); + for (const ancestor of fp) { + if (ancestor !== active && ancestor.rendered && isAttached(ancestor)) { + if (isDev) { + console.warn( + `[solidtv] Focus lost: ${getElementLabel(active)} was removed from the tree while focused. Recovering focus via ancestor ${getElementLabel(ancestor)}. Run printFocusHistory(10) to see recent focus movement.`, + { lost: active, recoveredVia: ancestor }, + ); + } + ancestor.setFocus(); + return; + } + } + + if (isDev) { + console.warn( + `[solidtv] Focus lost: ${getElementLabel(active)} was removed from the tree while focused, and no attached ancestor remains to recover to. Key events will not fire until setFocus() is called on a rendered element.`, + { lost: active }, + ); + } +}; + // --------------------------------------------------------------------------- /** @@ -438,7 +489,13 @@ const propagateKeyPress = ( if (lastHandlerSeen) { console.log(`Keypress bubbled, ${detail}`, lastHandlerSeen); } else { - console.log(`No event handler available for keypress: ${detail}`); + // Print the focus path leaf → root like a stack trace so it's obvious + // which elements had a chance to handle the key and didn't. + console.log( + `No event handler available for keypress: ${detail}\n focus path: ${fp + .map(getElementLabel) + .join(' → ')}`, + ); } } @@ -523,7 +580,22 @@ export const useFocusManager = ( document.addEventListener('keydown', keyPressHandler); document.addEventListener('keyup', keyUpHandler); + // Key events go nowhere until something is focused — the most common cause + // is a missing `autofocus` on the initial screen. Warn once if the app is + // still unfocused shortly after startup. + let autofocusCheckTimeout: number | undefined; + if (isDev) { + autofocusCheckTimeout = window.setTimeout(() => { + if (!activeElement()) { + console.warn( + '[solidtv] No element has focus 1s after useFocusManager() was called, so key events have nowhere to go. Set `autofocus` on an initial element or call setFocus().', + ); + } + }, 1000); + } + onCleanup(() => { + clearTimeout(autofocusCheckTimeout); document.removeEventListener('keydown', keyPressHandler); document.removeEventListener('keyup', keyUpHandler); for (const timeout of Object.values(keyHoldTimeouts)) { diff --git a/tests/focusRecovery.test.tsx b/tests/focusRecovery.test.tsx new file mode 100644 index 0000000..9086074 --- /dev/null +++ b/tests/focusRecovery.test.tsx @@ -0,0 +1,170 @@ +import * as v from 'vitest'; +import * as s from 'solid-js'; +import * as lng from '@solidtv/solid'; +import { renderer } from './setup.js'; + +// Focus changes and delete-flushes both settle in the post-mutation +// scheduler (microtask); a short macrotask wait covers chained passes. +const wait = (ms = 10) => new Promise((r) => setTimeout(r, ms)); + +v.describe('focus-loss recovery', () => { + v.test('refocuses the parent when the focused element is removed', async () => { + let app!: lng.ElementNode; + let btn!: lng.ElementNode; + const [visible, setVisible] = s.createSignal(true); + const warn = v.vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const dispose = renderer.render(() => ( + + + + + + )); + + await wait(); + btn.setFocus(); + await wait(); + v.assert.equal(lng.activeElement(), btn, 'btn is focused before removal'); + + setVisible(false); + await wait(); + v.assert.equal(lng.activeElement(), app, 'focus recovered to parent'); + v.assert.isTrue( + warn.mock.calls.some((args) => String(args[0]).includes('Focus lost')), + 'dev warning was logged', + ); + + warn.mockRestore(); + dispose(); + }); + + v.test('recovers to the nearest attached ancestor on deep removal', async () => { + let app!: lng.ElementNode; + let leaf!: lng.ElementNode; + const [visible, setVisible] = s.createSignal(true); + + const dispose = renderer.render(() => ( + + + + + + + + + + )); + + await wait(); + leaf.setFocus(); + await wait(); + v.assert.equal(lng.activeElement(), leaf); + + // Removes the whole section subtree; every ancestor of leaf below app is + // detached, so recovery must walk past them up to app. + setVisible(false); + await wait(); + v.assert.equal(lng.activeElement(), app, 'focus recovered to app root'); + + dispose(); + }); + + v.test('recovery re-runs forwardFocus so a sibling gets focus', async () => { + let row!: lng.ElementNode; + let second!: lng.ElementNode; + const [items, setItems] = s.createSignal(['a', 'b']); + + const dispose = renderer.render(() => ( + + + + {(item) => ( + { + if (item === 'b') second = el; + }} + id={item} + width={300} + height={150} + /> + )} + + + + )); + + await wait(); + row.setFocus(); + await wait(); + v.assert.equal(lng.activeElement()?.id, 'a', 'forwardFocus focused first child'); + + setItems(['b']); + await wait(); + v.assert.equal( + lng.activeElement(), + second, + 'recovery via row forwardFocus landed on the remaining child', + ); + + dispose(); + }); + + v.test('removing a non-focused element does not move focus', async () => { + let btn!: lng.ElementNode; + const [visible, setVisible] = s.createSignal(true); + + const dispose = renderer.render(() => ( + + + + + + + )); + + await wait(); + btn.setFocus(); + await wait(); + v.assert.equal(lng.activeElement(), btn); + + setVisible(false); + await wait(); + v.assert.equal(lng.activeElement(), btn, 'focus did not move'); + + dispose(); + }); + + v.test('Config.focusLossRecovery = false leaves focus untouched', async () => { + let btn!: lng.ElementNode; + const [visible, setVisible] = s.createSignal(true); + lng.Config.focusLossRecovery = false; + + try { + const dispose = renderer.render(() => ( + + + + + + )); + + await wait(); + btn.setFocus(); + await wait(); + v.assert.equal(lng.activeElement(), btn); + + setVisible(false); + await wait(); + v.assert.equal( + lng.activeElement(), + btn, + 'stale focus is preserved when recovery is disabled', + ); + + dispose(); + } finally { + lng.Config.focusLossRecovery = true; + } + }); +});