From f130a1c05a6effcf7dc55aee7b34b27f71117c14 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 13 Sep 2026 17:17:49 +0200 Subject: [PATCH] =?UTF-8?q?fix(keys):=20epic-end=20review=20round=201=20?= =?UTF-8?q?=E2=80=94=20Back,=20menus=20and=20dialogs=20keep=20the=20focus?= =?UTF-8?q?=20where=20it=20belongs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deep-review of E11 (19 raised, 14 confirmed; 2 refuted, 3 latent test-hardening notes dropped). - Back from a sub-view (a Plex show page, a Messages thread) restores the item it was opened from even when focus was still inside the view: the first attempt now waits one task, past the sub-view's own popstate listener (measured in Chrome: it left focus on ). - The restore listener is installed at load, so the first Back after a reload finds it; a push cancels a restore still waiting for the last landing's lists. - Navigating over an overlay asks it again, a render apart, while its entry stays: the Plex player peels one panel per Back step, so Shift+W over info + stats closed one panel and dropped the navigation. - j/k: a native + ))} + + ); + } + render(); + press("j"); + expect(focused()).toBe("t0"); + expect(screen.getByTestId("t0").hasAttribute("tabindex")).toBe(false); + press("j"); + expect(focused()).toBe("t1"); + expect(screen.getByTestId("t1").hasAttribute("tabindex")).toBe(false); + }); + + // Landing again on the item that already holds the roved tabindex (focus went to in between) + // forgot it, so it kept tabindex="-1" for good — and a click on its blank area focused the card. + it("gives the tabindex back after landing twice on the same item", () => { + render(); + press("j"); + expect(focused()).toBe("a0"); + act(() => (document.activeElement as HTMLElement).blur()); + press("j"); + expect(focused()).toBe("a0"); + press("j"); + expect(focused()).toBe("a1"); + expect(screen.getByTestId("a0").hasAttribute("tabindex")).toBe(false); + }); + + // Hiding the focused feed card unmounts it and drops focus to ; the next `j` started over at + // the first visible card instead of going on from the hidden card's slot. + it("goes on from the slot of an item that was removed under the focus", () => { + function Removable() { + const [ids, setIds] = useState([0, 1, 2, 3]); + const ref = useRef(null); + useListNav(ref, { enabled: true }); + return ( +
+ {ids.map((id, i) => ( +
+ +
+ ))} +
+ ); + } + withLayout( + (_, n) => ({ left: 0, top: n * 100 }), + () => { + render(); + press("j"); + press("j"); + press("j"); + expect(focused()).toBe("r2"); + act(() => screen.getByTestId("r2-hide").focus()); + fireEvent.click(screen.getByTestId("r2-hide")); + expect(document.activeElement).toBe(document.body); + press("j"); + expect(focused()).toBe("r3"); + }, + ); + }); +}); diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index 1e1a1bdd..fa3cf1ec 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -120,13 +120,44 @@ function firstVisible(): { list: NavList; index: number } | null { return fallback; } +/** The item focus was last inside, so a `j` after that item was REMOVED (a feed card hidden, a + * download deleted — focus falls to ) goes on from its slot instead of starting over. */ +let last: { list: NavList; index: number; el: HTMLElement } | null = null; + +function onFocusIn(e: FocusEvent): void { + const item = (e.target as HTMLElement | null)?.closest?.(`[${INDEX}]`); + const list = + item && lists.get(item.parentElement?.closest(`[${LIST}]`) as HTMLElement); + if (item && list) last = { list, index: indexOf(item), el: item }; +} + +/** Where `j` (`k`: the item before) lands after the last-focused item left the DOM: the item now in + * its slot, if that slot is on screen. An item that only scrolled out of a virtualized list has left + * the DOM too, but its slot is off screen then, so the reader's viewport (`firstVisible`) wins. */ +function slotOfRemoved(delta: 1 | -1): { list: NavList; index: number } | null { + if (!last || last.el.isConnected || lists.get(last.list.el) !== last.list) return null; + const n = last.list.count(); + const index = Math.min(Math.max(0, delta > 0 ? last.index : last.index - 1), n - 1); + const el = index >= 0 ? itemAt(last.list, index) : null; + if (!el) return null; + const box = scrollParent(last.list.el)?.getBoundingClientRect(); + const r = el.getBoundingClientRect(); + const band = (box?.top ?? 0) + (parseFloat(getComputedStyle(el).scrollMarginTop) || 0); + return r.bottom > band && r.top < (box?.bottom ?? window.innerHeight) + ? { list: last.list, index } + : null; +} + function focusItem(el: HTMLElement, reveal = true): void { - if (roved && roved !== el) roved.removeAttribute("tabindex"); - roved = null; - // An item that is focusable in its own right (a Plex card is a role=button) keeps its tabindex. - if (!el.hasAttribute("tabindex")) { - el.setAttribute("tabindex", "-1"); - roved = el; + if (roved !== el) { + roved?.removeAttribute("tabindex"); + roved = null; + // An item that is a Tab stop in its own right (a native