diff --git a/frontend/src/components/DiagnosticsSettings.tsx b/frontend/src/components/DiagnosticsSettings.tsx index 724aeb98..87bce22e 100644 --- a/frontend/src/components/DiagnosticsSettings.tsx +++ b/frontend/src/components/DiagnosticsSettings.tsx @@ -4,6 +4,7 @@ import { useTranslation } from "react-i18next"; import { api, type DiagnosticsLevel } from "../lib/api"; import { levelInForce } from "../lib/diagnostics"; +import { focusIsUnclaimed } from "../lib/useFocusKeeper"; import { updateMeCache, useMe } from "../lib/useMe"; import { Button } from "./ui/Button"; import InlineNotice from "./ui/InlineNotice"; @@ -61,7 +62,7 @@ export default function DiagnosticsSettings() { const from = closedFrom.current; if (!from || from.isConnected) return; closedFrom.current = null; - if (document.activeElement && document.activeElement !== document.body) return; + if (!focusIsUnclaimed()) return; pickerRef.current?.querySelector('[aria-pressed="true"]')?.focus(); }); const watchFocus = () => { diff --git a/frontend/src/components/IssueComposer.tsx b/frontend/src/components/IssueComposer.tsx index 78173275..aeb00228 100644 --- a/frontend/src/components/IssueComposer.tsx +++ b/frontend/src/components/IssueComposer.tsx @@ -19,6 +19,7 @@ import { import { moduleDef } from "../lib/modules"; import type { Page } from "../lib/urlState"; import { qk } from "../lib/queryKeys"; +import { focusIsUnclaimed } from "../lib/useFocusKeeper"; import { LEVEL_TAG, logClock } from "./logLineParts"; import { Button } from "./ui/Button"; import { IconButton } from "./ui/IconButton"; @@ -54,7 +55,7 @@ export default function IssueComposer({ id }: { id: string }) { useEffect( () => () => { window.setTimeout(() => { - if (document.activeElement === document.body) + if (focusIsUnclaimed()) document.querySelector('nav [aria-current="page"]')?.focus(); }, 0); }, diff --git a/frontend/src/components/ui/PageScroller.tsx b/frontend/src/components/ui/PageScroller.tsx index 29a0d5a4..647d1a83 100644 --- a/frontend/src/components/ui/PageScroller.tsx +++ b/frontend/src/components/ui/PageScroller.tsx @@ -1,6 +1,6 @@ import { useCallback, useLayoutEffect, useRef, useState, type ReactNode } from "react"; import { useScrollFade } from "../../lib/useScrollFade"; -import { useFocusKeeper } from "../../lib/useFocusKeeper"; +import { FOCUS_REST_ATTRS, useFocusKeeper } from "../../lib/useFocusKeeper"; import { StickyTopProvider } from "./stickyTop"; import { CONTENT_ID } from "./landmarks"; import { ensureNavId, peekNavId, subscribeNav } from "../../lib/history"; @@ -158,11 +158,13 @@ export default function PageScroller({ return ( // `id` + `tabIndex={-1}`: the skip link's target (ui/SkipLink) takes the focus, so the next Tab - // lands on the page's first control. + // lands on the page's first control. A click on the page's background focuses it too, which is + // the focus RESTING, not a choice a Back restore must respect (`FOCUS_REST_ATTRS`).
diff --git a/frontend/src/lib/history.navPosition.dom.test.tsx b/frontend/src/lib/history.navPosition.dom.test.tsx index 779acbad..eb1bdd06 100644 --- a/frontend/src/lib/history.navPosition.dom.test.tsx +++ b/frontend/src/lib/history.navPosition.dom.test.tsx @@ -10,6 +10,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { act, cleanup, render, waitFor } from "@testing-library/react"; import { rememberNavPosition } from "./history"; import { navItemAttrs, useListNav } from "./listNav"; +import { FOCUS_REST_ATTRS } from "./useFocusKeeper"; function List({ id, count, label }: { id: string; count: number; label?: string }) { const ref = useRef(null); @@ -146,6 +147,22 @@ describe("rememberNavPosition · Back focuses the item a view was left from", () }); }); +// E28 review H1: the page region is `
` (the skip link's target), so a mouse click +// on the page's blank area focuses it. The restore took that for a control the reader had chosen and +// gave up: Back left the focus on the page instead of on the item the view was opened from. +describe("rememberNavPosition · the focus resting on the page region", () => { + it("still restores when the page region holds the focus", async () => { + const { unmount } = render(); + const stamped = leaveFrom("b1", 4); + unmount(); + const region = render(
); + region.getByTestId("main").focus(); + landOn(stamped); + render(); + await waitFor(() => expect(focused()).toBe("b1")); + }); +}); + describe("rememberNavPosition · the list it comes back to (deep-review E11 round 2)", () => { // Discover: a title opened from "Recently added" and watched — the page's cache expires and // "Continue watching" fills with that very title, a rail ABOVE. Found by its place, focus landed in diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index b32bfd82..415c5e43 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -1,7 +1,7 @@ import { useEffect, useRef, useState, type RefObject } from "react"; import { dispatch, useKeyScope } from "./keymap"; import { scrollParent } from "./listPaging"; -import { isStandInFocus } from "./useFocusKeeper"; +import { focusIsUnclaimed } from "./useFocusKeeper"; // ── Keyboard navigation over lists and grids (E11 S4, UX-6) ────────────────────────────────────── // Before this, a feed card cost ~10 Tab stops (thumb, play, info, title, channel, five actions), so @@ -148,6 +148,9 @@ function onFocusIn(e: FocusEvent): void { const list = item && lists.get(item.parentElement?.closest(`[${LIST}]`) as HTMLElement); if (item && list) last = { list, index: indexOf(item), el: item }; + // The focus coming to REST on the page region (a click on its background) is not leaving the list + // for other content — like a click on , which fires no focusin at all. + else if (focusIsUnclaimed(target)) return; // Focus went on to page content that is not an item — the search box, a filter chip: the reader // left the list, and whatever it shows next (new results) is not "the slot they were in" (deep-review // E11 round 2). A layer opened FROM the item — its menu, a confirm, the Undo toast its removal put @@ -375,8 +378,7 @@ function refocusSlot(list: NavList, index: number, removed: HTMLElement): void { if (++frames < REMOVE_FRAMES) requestAnimationFrame(wait); return; } - const active = document.activeElement; - if (active && active !== document.body && !isStandInFocus(active)) return; + if (!focusIsUnclaimed()) return; const n = list.count(); if (n > 0) focusIndex(list, Math.min(index, n - 1)); }; @@ -450,8 +452,7 @@ export function restoreNavPosition(pos: NavPosition): () => void { const deadline = Date.now() + RESTORE_MS; let timer: ReturnType | undefined; const attempt = () => { - const active = document.activeElement; - if (active && active !== document.body && !isStandInFocus(active)) return; + if (!focusIsUnclaimed()) return; const all = ordered(); const list = pos.label ? all.find((l) => labelOf(l) === pos.label) diff --git a/frontend/src/lib/useFocusKeeper.ts b/frontend/src/lib/useFocusKeeper.ts index 4a08de02..df02c0ef 100644 --- a/frontend/src/lib/useFocusKeeper.ts +++ b/frontend/src/lib/useFocusKeeper.ts @@ -41,6 +41,19 @@ export function isStandInFocus(el: Element | null): boolean { return el !== null && el === standIn; } +/** Marks an element that can hold the focus without being a control: the page's scroll region + * (`
`, the skip link's target) takes it from a mouse click on the page's blank + * area. Holding it there is the focus RESTING on the page, the same as . */ +export const FOCUS_REST_ATTRS = { "data-focus-rest": "" } as const; + +/** Whether the focus is nowhere the reader chose — , the keeper's stand-in, or a resting place + * (`FOCUS_REST_ATTRS`) — so a restore may take it. The page region used to count as a choice: a + * click on its background focused it, and Back then left the focus there instead of on the item + * the view was opened from (E28 review H1). */ +export function focusIsUnclaimed(el: Element | null = document.activeElement): boolean { + return !el || el === document.body || isStandInFocus(el) || el.hasAttribute("data-focus-rest"); +} + /** Where the focus goes once `lost` (removed with the nodes of `record`) has left the page — never * outside `root`: a slot with nothing focusable left (a page swapped for its loading fallback) must * not send the focus to the first control of the whole document. */ @@ -80,8 +93,7 @@ export function useFocusKeeper(ref: RefObject): void { const lost = held; if (!lost || lost.isConnected) return; held = null; - const active = document.activeElement; - if (active && active !== document.body) return; // something already took the focus + if (!focusIsUnclaimed()) return; // something already took the focus for (const r of records) { const target = slotFor(r, lost, root); if (target) {