From 250e3a9f90ce1253be3fdf059d81858a5db71111 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 19 Sep 2026 04:51:43 +0200 Subject: [PATCH] fix(home): Back keeps the question, and the focus finds its card by name Opening a card left Home, and Back came back to "Doesn't matter" with a different hero, whose card then dropped out of the Continue rail and moved the keyboard onto its neighbour. The budget, reroll seed and seen list live on the history entry now, and a returning list focuses the item by its name before its place. --- frontend/src/components/Home.dom.test.tsx | 26 +++++++++++ frontend/src/components/Home.pin.dom.test.tsx | 2 + frontend/src/components/Home.tsx | 40 +++++++++++++++-- .../src/lib/history.navPosition.dom.test.tsx | 24 +++++++++++ frontend/src/lib/listNav.ts | 43 +++++++++++++------ 5 files changed, 119 insertions(+), 16 deletions(-) diff --git a/frontend/src/components/Home.dom.test.tsx b/frontend/src/components/Home.dom.test.tsx index 50017494..4c2aab42 100644 --- a/frontend/src/components/Home.dom.test.tsx +++ b/frontend/src/components/Home.dom.test.tsx @@ -163,6 +163,8 @@ function mountWithClient() { } beforeEach(() => { + // Home keeps its question on the history entry (Back restores it): each test starts a fresh one. + window.history.replaceState(null, ""); discover.mockReset(); discoverPick.mockReset(); setPage.mockReset(); @@ -553,11 +555,35 @@ describe("Home", () => { first.unmount(); discoverPick.mockClear(); + // A later visit is a new history entry (setPage pushes a fresh one); the same entry would bring + // the rerolled question back on purpose (see the Back test below). + window.history.pushState(null, ""); mount(); await screen.findByText("Hero v1"); expect(discoverPick).not.toHaveBeenCalled(); }); + // Prod UAT 2026-09-19: a card opened from Home leaves the page, and Back came back to "Doesn't + // matter" and a different hero. The question is kept on the entry, so Back asks it again. + it("Back to the same entry brings back the budget and the rerolled hero", async () => { + discoverPick.mockResolvedValue({ + budget: "short", + hero: { tier: "surprise", card: ytCard("v9", "Rerolled") }, + }); + const first = mount(); + await screen.findByText("Hero v1"); + fireEvent.click(screen.getByText("home.budget.short")); + fireEvent.click(screen.getByText("home.hero.reroll")); + expect(await screen.findByText("Rerolled")).toBeTruthy(); + first.unmount(); + + discover.mockClear(); + mount(); + + expect(await screen.findByText("Rerolled")).toBeTruthy(); + expect(discover).toHaveBeenLastCalledWith("short", 0); + }); + // Measured in the browser (2026-09-07), before the fix: the palette's "Meglepetés" arrived at Home // as `start=2` — two picks deep before the page had painted once — because StrictMode invokes a // mount effect twice with the same render snapshot, and a reroll is not idempotent. The token is diff --git a/frontend/src/components/Home.pin.dom.test.tsx b/frontend/src/components/Home.pin.dom.test.tsx index 9b122160..6489ab53 100644 --- a/frontend/src/components/Home.pin.dom.test.tsx +++ b/frontend/src/components/Home.pin.dom.test.tsx @@ -149,6 +149,8 @@ const item = (label: string) => { }; beforeEach(() => { + // Home keeps its question on the history entry (Back restores it): each test starts a fresh one. + window.history.replaceState(null, ""); discover.mockReset().mockResolvedValue(payload()); comfortPins.mockReset().mockResolvedValue({ items: shelf }); addComfortPin.mockReset().mockResolvedValue({ ok: true, id: 1, created: true }); diff --git a/frontend/src/components/Home.tsx b/frontend/src/components/Home.tsx index 9ab7b13b..454f5098 100644 --- a/frontend/src/components/Home.tsx +++ b/frontend/src/components/Home.tsx @@ -348,26 +348,58 @@ function Hero({ ); } +const SAVED_KEY = "sfHome"; + +interface SavedQuestion { + budget: DiscoverBudget; + seed: number; + press: number; + seen: string[]; +} + +/** The question this history entry was left with, if it was left from Home. Read from + * `history.state`, which only this app writes — still checked field by field. */ +function readSavedQuestion(): SavedQuestion | null { + const raw = (window.history.state as Record | null)?.[SAVED_KEY] as + | Partial + | undefined; + if (!raw || !DISCOVER_BUDGETS.includes(raw.budget as DiscoverBudget)) return null; + return { + budget: raw.budget as DiscoverBudget, + seed: Number.isInteger(raw.seed) ? (raw.seed as number) : 0, + press: Number.isInteger(raw.press) ? (raw.press as number) : 0, + seen: Array.isArray(raw.seen) ? raw.seen.filter((k) => typeof k === "string") : [], + }; +} + export default function Home() { const { t } = useTranslation(); const qc = useQueryClient(); const { setPage } = useNavigationActions(); const { setJumpTo } = usePlexActions(); - const [budget, setBudget] = useState("any"); + // The question on screen lives on this page's history ENTRY as well: opening a card leaves Home + // (it unmounts), and Back used to come back to "Doesn't matter" and a different hero — whose card + // then left the Continue rail, so the keyboard landed on its neighbour (prod UAT 2026-09-19). + const [saved] = useState(readSavedQuestion); + const [budget, setBudget] = useState(saved?.budget ?? "any"); // 0 = "the hero the page loaded with". Any other value is a reroll, answered by its own query so // pressing the button does not refetch the rails under the user's cursor. - const [seed, setSeed] = useState(0); + const [seed, setSeed] = useState(saved?.seed ?? 0); // Which press this is. It rotates the tier the server leads with, so "Mást kérek" offers a // different KIND of answer each time (where you left off → your shelf → a surprise) rather than // the next row of the same rail. Reset with the budget: a new question starts the rotation over. - const [press, setPress] = useState(0); + const [press, setPress] = useState(saved?.press ?? 0); // The cards the reroll has already offered this session, newest last. A ROLLING window rather than // the whole history: an unbounded list would eventually exclude everything and leave the button // with nothing to give, whereas a bounded one cannot. Its size is `SEEN_WINDOW` — see there for // why it is 32 and not the 8 this comment used to quote. Kept here (not in the URL or a pref) // because it is a property of this sitting. - const [seen, setSeen] = useState([]); + const [seen, setSeen] = useState(saved?.seen ?? []); + useEffect(() => { + const question: SavedQuestion = { budget, seed, press, seen }; + window.history.replaceState({ ...window.history.state, [SAVED_KEY]: question }, ""); + }, [budget, seed, press, seen]); const [player, setPlayer] = useState<{ video: Video; startAt: number | null } | null>(null); const q = useQuery({ diff --git a/frontend/src/lib/history.navPosition.dom.test.tsx b/frontend/src/lib/history.navPosition.dom.test.tsx index 9ee1da77..b9c080a4 100644 --- a/frontend/src/lib/history.navPosition.dom.test.tsx +++ b/frontend/src/lib/history.navPosition.dom.test.tsx @@ -170,6 +170,30 @@ describe("rememberNavPosition · the list it comes back to (deep-review E11 roun await waitFor(() => expect(focused()).toBe("a1")); }); + // Home's Continue rail leaves out whatever the hero shows: back from a card, a different hero put a + // card back in front of it, and by its place the focus landed on the neighbour (prod UAT 2026-09-19). + it("finds the item by its name when the rail's contents moved", async () => { + const Rail = ({ titles }: { titles: string[] }) => { + const ref = useRef(null); + useListNav(ref, { enabled: true }); + return ( +
+ {titles.map((title, i) => ( +
+ {title} +
+ ))} +
+ ); + }; + const { unmount } = render(); + const stamped = leaveFrom("I Am Mother", 2); + unmount(); + landOn(stamped); + render(); + await waitFor(() => expect(focused()).toBe("I Am Mother")); + }); + it("gives up on an unnamed list when the page came back with another number of lists", async () => { const { unmount } = render(); const stamped = leaveFrom("a1", 2); diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index 330fe48e..4b143d64 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -164,6 +164,18 @@ function slotOfRemoved(delta: 1 | -1): { list: NavList; index: number } | null { : null; } +/** What an item is called: its own name when it gives one (`navItemAttrs`), else the open element's + * label or text — a title link's text is the title (deep-review E11 round 3). */ +function itemName(el: HTMLElement): string { + const open = el.querySelector("[data-nav-open]"); + return ( + el.getAttribute("data-nav-name") || + open?.getAttribute("aria-label") || + open?.textContent || + "" + ).trim(); +} + function focusItem(el: HTMLElement, reveal = true): void { if (roved !== el) { if (named) roved?.removeAttribute("role"); @@ -179,15 +191,7 @@ function focusItem(el: HTMLElement, reveal = true): void { // A card root is a plain
: focused, it has no role and no name, and a screen reader read it // as an unlabelled run of text. While it holds the focus it is a group named after what Enter // opens (deep-review E11 round 2). A row () keeps its own role. - // The item's own name when it gives one (`navItemAttrs`), else the open element's label or text: - // a title link's text is the title (deep-review E11 round 3). - const open = el.querySelector("[data-nav-open]"); - const name = ( - el.getAttribute("data-nav-name") || - open?.getAttribute("aria-label") || - open?.textContent || - "" - ).trim(); + const name = itemName(el); if ( el.tagName === "DIV" && name && @@ -321,12 +325,14 @@ function onKey(e: KeyboardEvent): void { // on screen again. An overlay (the YouTube player) never unmounts the list; it restores focus itself. /** Where focus sits in the page's lists: the list's place in reading order, how many lists the page - * had on screen, the list's accessible name when it has one (a rail's title), and the item's index. */ + * had on screen, the list's accessible name when it has one (a rail's title), the item's index, and + * the item's own name. */ export interface NavPosition { readonly list: number; readonly lists: number; readonly label?: string | null; readonly index: number; + readonly name?: string | null; } const labelOf = (list: NavList): string | null => list.el.getAttribute("aria-label"); @@ -336,7 +342,15 @@ export function navPosition(): NavPosition | null { if (!cur) return null; const all = ordered(); const list = all.indexOf(cur.list); - return list < 0 ? null : { list, lists: all.length, label: labelOf(cur.list), index: cur.index }; + return list < 0 + ? null + : { + list, + lists: all.length, + label: labelOf(cur.list), + index: cur.index, + name: itemName(cur.item) || null, + }; } // How long a returning page gets to put its lists back (a lazy chunk, cached data), and how often we @@ -364,7 +378,12 @@ export function restoreNavPosition(pos: NavPosition): () => void { : all.length === pos.lists ? all[pos.list] : undefined; - const el = list ? itemAt(list, pos.index) : null; + // The item by its NAME first, its place second: a rail's contents can shift while the page was + // away — Home's Continue rail leaves out whatever the hero shows, so a different hero moved every + // card after it one place and the focus landed on the neighbour (prod UAT 2026-09-19). + const named = + list && pos.name ? ownItems(list.el).find((it) => itemName(it) === pos.name) : null; + const el = named ?? (list ? itemAt(list, pos.index) : null); if (el) return focusItem(el, false); if (Date.now() < deadline) timer = setTimeout(attempt, RESTORE_POLL_MS); };