fix(nav): Back finds an item that moved to another list by its name

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<HTMLDivElement>(null);
+  useListNav(ref, { enabled: true });
+  return (
+    <div ref={ref}>
+      {items.map((t, i) => (
+        <div key={`${t}-${i}`} {...navItemAttrs(i, t)}>
+          <button data-nav-open="">{t}</button>
+        </div>
+      ))}
+    </div>
+  );
+}
+
+const Page = ({ lists }: { lists: string[][] }) => (
+  <main>
+    {lists.map((items, i) => (
+      <List key={i} items={items} />
+    ))}
+  </main>
+);
+
+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(<Page lists={before} />);
+  button(from).focus();
+  const pos = navPosition()!;
+  unmount(); // the page switch
+  render(<Page lists={after} />); // 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");
+  });
+});

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. */
+ *  lines). Then the one item of that name in another of the page's lists: an item can change lists
-function restoreTarget(list: NavList, pos: NavPosition): HTMLElement | null {
+ *  while the page was away (a notification asked for the reader on the visit it was opened from and
-  const at = itemAt(list, pos.index);
+ *  sits in the stream on the next), and the place left behind is now its neighbour's. Only when the
-  if (!pos.name || (at && itemName(at) === pos.name)) return at;
+ *  name is the page's alone — of two, either could be the one. Else the saved place. */
-  const distance = (el: HTMLElement) => Math.abs(indexOf(el) - pos.index);
+function restoreTarget(
-  const named = ownItems(list.el)
+  all: readonly NavList[],
-    .filter((it) => itemName(it) === pos.name)
+  list: NavList | undefined,
-    .sort((a, b) => distance(a) - distance(b));
+  pos: NavPosition,
-  return named[0] ?? at;
+): 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 <body> is the honest outcome. Gives up when the time is up, and at
+ *  list N may be another list, and <body> 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 <body>.) 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);
   };