From 22d873ed490d864bcbd1dfd77c37cfea838c9279 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 28 Oct 2026 22:04:07 +0100 Subject: [PATCH] fix(nav): an item Back finds in another list is brought into view --- .../src/lib/listNav.restoreMoved.dom.test.tsx | 19 ++++++++++++++++++- frontend/src/lib/listNav.ts | 7 +++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/frontend/src/lib/listNav.restoreMoved.dom.test.tsx b/frontend/src/lib/listNav.restoreMoved.dom.test.tsx index a759247a..389f2124 100644 --- a/frontend/src/lib/listNav.restoreMoved.dom.test.tsx +++ b/frontend/src/lib/listNav.restoreMoved.dom.test.tsx @@ -5,7 +5,7 @@ // 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 { afterEach, describe, expect, it, vi } from "vitest"; import { act, cleanup, render } from "@testing-library/react"; import { useRef } from "react"; @@ -72,4 +72,21 @@ describe("Back to an item that moved to another list", () => { ]); expect(focused()).toBe("download"); }); + + it("brings an item found in another list into view; one in its own list stays put", async () => { + // The scroll restore put the page where the item WAS: found elsewhere, it would hold the focus + // off screen. + const reveal = vi.fn(); + Element.prototype.scrollIntoView = reveal; + await leaveAndReturn([["subscribed", "download"], ["older"]], "subscribed", [ + ["download"], + ["subscribed", "older"], + ]); + expect(reveal).toHaveBeenCalledTimes(1); + cleanup(); + reveal.mockClear(); + await leaveAndReturn([["subscribed", "download"]], "download", [["download", "subscribed"]]); + expect(focused()).toBe("download"); + expect(reveal).not.toHaveBeenCalled(); + }); }); diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index 1ceaf0cf..b2db0268 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -492,7 +492,8 @@ function restoreTarget( * 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 + * where the scroll restore put it, never scrolled to — but one found in another list is brought into + * view. (A stand-in the focus keeper placed when the view * it held was swapped away counts as untouched, like .) Returns a cancel. */ export function restoreNavPosition(pos: NavPosition): () => void { const deadline = Date.now() + RESTORE_MS; @@ -512,7 +513,9 @@ export function restoreNavPosition(pos: NavPosition): () => void { // 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 = restoreTarget(all, list, pos); - if (el) return focusItem(el, false); + // Where the scroll restore put it — unless it is found in another list than the one it was left + // in: the restored scroll is the old place's, and the item would hold the focus off screen. + if (el) return focusItem(el, !list?.el.contains(el)); if (Date.now() < deadline) timer = setTimeout(attempt, RESTORE_POLL_MS); }; // Not inside the popstate dispatch that landed: a sub-view's own listener (`useHistorySubview`)