diff --git a/docs/primitives/createFocusStack.md b/docs/primitives/createFocusStack.md index 6893008f..ec0ce230 100644 --- a/docs/primitives/createFocusStack.md +++ b/docs/primitives/createFocusStack.md @@ -61,7 +61,7 @@ Stores the currently focused element. If the element is already active, it does ### `restoreFocus(): boolean` -Restores focus to the last stored element and removes it from the stack. +Restores focus to the last stored element and removes it from the stack. Elements destroyed since they were stored are removed and skipped, so focus goes to the most recent element that still exists. - Returns `true` if focus was successfully restored, otherwise `false`. diff --git a/src/core/dom-renderer/domRenderer.ts b/src/core/dom-renderer/domRenderer.ts index 818fc6e4..4ec1adf2 100644 --- a/src/core/dom-renderer/domRenderer.ts +++ b/src/core/dom-renderer/domRenderer.ts @@ -1358,6 +1358,8 @@ export class DOMNode extends EventEmitter implements IRendererNode { preventCleanup = true; + destroyed = false; + constructor( public stage: IRendererStage, public props: IRendererNodeProps, @@ -1380,6 +1382,7 @@ export class DOMNode extends EventEmitter implements IRendererNode { } destroy(): void { + this.markDestroyed(); elMap.delete(this); const parent = this.props.parent; if (parent instanceof DOMNode) { @@ -1388,6 +1391,14 @@ export class DOMNode extends EventEmitter implements IRendererNode { this.div.parentNode?.removeChild(this.div); } + // Like CoreNode, destroying a node marks its whole subtree destroyed. + private markDestroyed() { + this.destroyed = true; + for (const child of this.children) { + child.markDestroyed(); + } + } + get parent() { return this.props.parent; } diff --git a/src/primitives/createFocusStack.tsx b/src/primitives/createFocusStack.tsx index 2937adbd..abf3e148 100644 --- a/src/primitives/createFocusStack.tsx +++ b/src/primitives/createFocusStack.tsx @@ -14,7 +14,7 @@ * * Functions: * - `storeFocus(element: ElementNode, prevElement?: ElementNode)`: Stores the provided element in the focus stack. - * - `restoreFocus()`: Restores focus to the last stored element and removes it from the stack. Returns `true` if successful, `false` otherwise. + * - `restoreFocus()`: Restores focus to the last stored element and removes it from the stack, skipping (and removing) destroyed elements. Returns `true` if successful, `false` otherwise. * - `clearFocusStack()`: Empties the focus stack. */ import * as s from 'solid-js'; @@ -41,7 +41,11 @@ export function FocusStackProvider(props: { children: s.JSX.Element}) { function restoreFocus(): boolean { let wasFocused = false; setFocusStack((stack) => { - const prevElement = stack.pop(); + let prevElement = stack.pop(); + // Skip elements destroyed since they were stored + while (prevElement?.destroyed) { + prevElement = stack.pop(); + } if (prevElement && typeof prevElement.setFocus === 'function') { prevElement.setFocus(); wasFocused = true; diff --git a/tests/focusStack.test.tsx b/tests/focusStack.test.tsx new file mode 100644 index 00000000..03c9db6d --- /dev/null +++ b/tests/focusStack.test.tsx @@ -0,0 +1,79 @@ +import * as v from 'vitest'; +import * as lng from '@solidtv/solid'; +import { createSignal, Show } from 'solid-js'; +import { FocusStackProvider, useFocusStack } from '@solidtv/solid/primitives'; +import { renderer } from './setup.js'; + +const wait = (ms = 10) => new Promise((r) => setTimeout(r, ms)); + +// Renders `kept` plus a page under ; hiding the page destroys it and +// its child `inner`. +const setup = () => { + const [showPage, setShowPage] = createSignal(true); + let stack!: ReturnType; + let kept!: lng.ElementNode; + let page!: lng.ElementNode; + let inner!: lng.ElementNode; + + const Capture = () => { + stack = useFocusStack(false); + return null; + }; + + const dispose = renderer.render(() => ( + + + + + + + + + + )); + + return { stack, kept, page, inner, setShowPage, dispose }; +}; + +v.describe('useFocusStack restoreFocus', () => { + v.test('skips destroyed elements and focuses the next one', async () => { + const { stack, kept, page, inner, setShowPage, dispose } = setup(); + await wait(); + + stack.storeFocus(kept); + stack.storeFocus(page); + stack.storeFocus(inner); + + setShowPage(false); + await wait(); + v.expect(page.destroyed).toBe(true); + v.expect(inner.destroyed).toBe(true); + + v.expect(stack.restoreFocus()).toBe(true); + await wait(); + v.expect(lng.activeElement()).toBe(kept); + + // destroyed entries were removed along with the restored one + v.expect(stack.restoreFocus()).toBe(false); + + dispose(); + }); + + v.test('returns false when every stored element is destroyed', async () => { + const { stack, kept, page, setShowPage, dispose } = setup(); + await wait(); + + kept.setFocus(); + await wait(); + stack.storeFocus(page); + + setShowPage(false); + await wait(); + + v.expect(stack.restoreFocus()).toBe(false); + await wait(); + v.expect(lng.activeElement()).toBe(kept); + + dispose(); + }); +});