From c86ec7f7a502e4b031f30054f1149c1dc92bd21d Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 28 Oct 2026 21:54:00 +0100 Subject: [PATCH] fix(nav): Back finds an item that moved to another list by its name --- .../src/lib/listNav.restoreMoved.dom.test.tsx | 75 +++++++++++++++++++ frontend/src/lib/listNav.ts | 36 ++++++--- 2 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 frontend/src/lib/listNav.restoreMoved.dom.test.tsx diff --git a/frontend/src/lib/listNav.restoreMoved.dom.test.tsx b/frontend/src/lib/listNav.restoreMoved.dom.test.tsx new file mode 100644 index 00000000..a759247a --- /dev/null +++ b/frontend/src/lib/listNav.restoreMoved.dom.test.tsx @@ -0,0 +1,75 @@ +// @vitest-environment jsdom +// +// Back to a page whose item changed lists while it was away. The notification center holds a row +// that asked for the reader in "Needs you" for the visit; on the next visit the same row, read now, +// sits in the stream below. The restore matched the unnamed list by its place, missed the name there +// and fell back to the saved index — the focus landed on the neighbour, and Enter opened the wrong +// notification. Its own file: listNav keeps module-level state. +import { afterEach, describe, expect, it } from "vitest"; +import { act, cleanup, render } from "@testing-library/react"; +import { useRef } from "react"; + +import { navItemAttrs, navPosition, restoreNavPosition, useListNav } from "./listNav"; + +afterEach(cleanup); + +function List({ items }: { items: string[] }) { + const ref = useRef(null); + useListNav(ref, { enabled: true }); + return ( +
+ {items.map((t, i) => ( +
+ +
+ ))} +
+ ); +} + +const Page = ({ lists }: { lists: string[][] }) => ( +
+ {lists.map((items, i) => ( + + ))} +
+); + +const button = (name: string) => + [...document.querySelectorAll("button")].find((b) => b.textContent === name)!; +const focused = () => document.activeElement?.closest("[data-nav-index]")?.textContent; +const wait = (ms: number) => act(() => new Promise((r) => setTimeout(r, ms))); + +async function leaveAndReturn(before: string[][], from: string, after: string[][]) { + const { unmount } = render(); + button(from).focus(); + const pos = navPosition()!; + unmount(); // the page switch + render(); // Back + const cancel = restoreNavPosition(pos); + await wait(120); + cancel(); +} + +describe("Back to an item that moved to another list", () => { + it("finds it by its name in the list it moved to, not its neighbour at the old place", async () => { + await leaveAndReturn([["subscribed", "download"], ["older"]], "subscribed", [ + ["download"], + ["subscribed", "older"], + ]); + expect(focused()).toBe("subscribed"); + }); + + it("finds it when the list it left is gone altogether", async () => { + await leaveAndReturn([["subscribed"], ["older"]], "subscribed", [["subscribed", "older"]]); + expect(focused()).toBe("subscribed"); + }); + + it("does not guess between two items of that name elsewhere: the saved place", async () => { + await leaveAndReturn([["twin", "download"], ["older"]], "twin", [ + ["download"], + ["twin", "older", "twin"], + ]); + expect(focused()).toBe("download"); + }); +}); diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index 6d688fea..1ceaf0cf 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -460,17 +460,28 @@ export function navPosition(): NavPosition | null { const RESTORE_MS = 2000; const RESTORE_POLL_MS = 50; -/** The item in `list` to hand the focus back to: the saved place while it still holds the saved +/** The item to hand the focus back to: in `list`, the saved place while it still holds the saved * name, else the nearest item of that name (names repeat — a log console's identical request - * lines), else the saved place. */ -function restoreTarget(list: NavList, pos: NavPosition): HTMLElement | null { - const at = itemAt(list, pos.index); - if (!pos.name || (at && itemName(at) === pos.name)) return at; - const distance = (el: HTMLElement) => Math.abs(indexOf(el) - pos.index); - const named = ownItems(list.el) - .filter((it) => itemName(it) === pos.name) - .sort((a, b) => distance(a) - distance(b)); - return named[0] ?? at; + * lines). Then the one item of that name in another of the page's lists: an item can change lists + * while the page was away (a notification asked for the reader on the visit it was opened from and + * sits in the stream on the next), and the place left behind is now its neighbour's. Only when the + * name is the page's alone — of two, either could be the one. Else the saved place. */ +function restoreTarget( + all: readonly NavList[], + list: NavList | undefined, + pos: NavPosition, +): HTMLElement | null { + const at = list ? itemAt(list, pos.index) : null; + if (list && (!pos.name || (at && itemName(at) === pos.name))) return at; + if (!pos.name) return null; + const named = (l: NavList) => ownItems(l.el).filter((it) => itemName(it) === pos.name); + if (list) { + const distance = (el: HTMLElement) => Math.abs(indexOf(el) - pos.index); + const here = named(list).sort((a, b) => distance(a) - distance(b)); + if (here[0]) return here[0]; + } + const [only, ...more] = all.flatMap(named); + return only && more.length === 0 ? only : at; } /** Focus the item at `pos` once its list is on screen again. A NAMED list (a rail) is found by its @@ -478,7 +489,8 @@ function restoreTarget(list: NavList, pos: NavPosition): HTMLElement | null { * watched in between, after the page's cache expired — shifts every place after it, and matching by * place landed the focus in the wrong rail. An unnamed list only by its * place, and only once the page has exactly as many lists as when it was left: with more or fewer, - * list N may be another list, and is the honest outcome. Gives up when the time is up, and at + * list N may be another list, and is the honest outcome — unless the item's name is the + * page's alone (`restoreTarget`). Gives up when the time is up, and at * once when focus is already somewhere (the reader moved on, a dialog took it). The item is focused * where the scroll restore put it, never scrolled to. (A stand-in the focus keeper placed when the view * it held was swapped away counts as untouched, like .) Returns a cancel. */ @@ -499,7 +511,7 @@ export function restoreNavPosition(pos: NavPosition): () => void { // The saved place still holding the saved name wins, and among several items of that name the // nearest one: names repeat (a log console's `http.request GET /api/feed 200` lines, two // downloads of one video), and the first of them is rarely the one left behind. - const el = list ? restoreTarget(list, pos) : null; + const el = restoreTarget(all, list, pos); if (el) return focusItem(el, false); if (Date.now() < deadline) timer = setTimeout(attempt, RESTORE_POLL_MS); };