From 62b9e404ef3d2535554d0dbaf2896170fdaccc1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 11 Nov 2026 21:23:17 +0100 Subject: [PATCH] fix(shell): return to the list's scroll position on Back Back from a detail page (a biomarker, a medication, a workout) opened the list at the top. The shell scrolls inside `
`, which stays mounted across routes, and the browser's own restoration only moves the document. Nothing kept the list's offset, so `
` stayed where the detail page had left it; and the one-shot skip on `popstate` was cleared by a timer that could run before the restored page rendered, in which case the forward-navigation reset fired anyway. The shell now records `
`'s offset per address while scrolling, reads it back on `popstate` and applies it once the page has rendered, retrying for up to a second while a list is still shorter than the offset. A forward navigation still starts at the top. --- .../baselines/source-function-lines.json | 2 +- .../baselines/version-stamp-comments.json | 4 +- .../auth-shell-scroll.browser.test.tsx | 116 ++++++++++++++++++ src/components/layout/auth-shell.tsx | 95 +++++++++----- 4 files changed, 180 insertions(+), 37 deletions(-) create mode 100644 src/components/layout/__tests__/auth-shell-scroll.browser.test.tsx diff --git a/src/__tests__/baselines/source-function-lines.json b/src/__tests__/baselines/source-function-lines.json index dd83b4bdc..8b862c9ff 100644 --- a/src/__tests__/baselines/source-function-lines.json +++ b/src/__tests__/baselines/source-function-lines.json @@ -138,7 +138,7 @@ "components/labs/lab-history-list.tsx :: LabHistoryList": 534, "components/labs/ocr-review-dialog.tsx :: OcrReviewDialog": 359, "components/labs/ocr-row-editor.tsx :: OcrRowEditor": 378, - "components/layout/auth-shell.tsx :: AuthShell": 518, + "components/layout/auth-shell.tsx :: AuthShell": 484, "components/layout/bottom-nav.tsx :: BottomNav": 262, "components/layout/capture-picker.tsx :: CapturePicker": 235, "components/layout/sidebar-nav.tsx :: SidebarNav": 356, diff --git a/src/__tests__/baselines/version-stamp-comments.json b/src/__tests__/baselines/version-stamp-comments.json index a99344b92..9a3ee2c39 100644 --- a/src/__tests__/baselines/version-stamp-comments.json +++ b/src/__tests__/baselines/version-stamp-comments.json @@ -1,7 +1,7 @@ { "$comment": "Comment lines that open with a release version, per file under src/. Maintained by src/__tests__/version-stamp-ratchet.test.ts.", "count": 2892, - "total": 9216, + "total": 9215, "entries": { "__tests__/account-payload-consumer-guard.test.ts": 2, "__tests__/acting-account-boundary-guard.test.ts": 7, @@ -1096,7 +1096,7 @@ "components/layout/__tests__/shared-record-chrome.test.tsx": 1, "components/layout/__tests__/sidebar-nav.test.tsx": 3, "components/layout/account-switcher-menu.tsx": 1, - "components/layout/auth-shell.tsx": 14, + "components/layout/auth-shell.tsx": 13, "components/layout/bottom-nav.tsx": 8, "components/layout/capture-picker.tsx": 5, "components/layout/nav-model.ts": 17, diff --git a/src/components/layout/__tests__/auth-shell-scroll.browser.test.tsx b/src/components/layout/__tests__/auth-shell-scroll.browser.test.tsx new file mode 100644 index 000000000..ec8a64d4b --- /dev/null +++ b/src/components/layout/__tests__/auth-shell-scroll.browser.test.tsx @@ -0,0 +1,116 @@ +/** + * The app shell's scroll position across navigation. + * + * `
` is the shell's one scroller and stays mounted across routes, and + * the browser restores only the document's offset on Back. So the shell + * keeps the offsets itself: a new page starts at the top, Back returns to + * where the list was left. + */ +import { useEffect, useState } from "react"; +import { describe, expect, it, vi } from "vitest"; + +import { usePathname } from "next/navigation"; +import { URL_STATE_EVENT } from "@/lib/url-state"; + +import { renderWithApp } from "../../../../tests/browser/render"; +import { stubFetch } from "../../../../tests/browser/fetch-stub"; + +// Chrome that is not the subject. +vi.mock("../sidebar-nav", () => ({ SidebarNav: () => null })); +vi.mock("../bottom-nav", () => ({ BottomNav: () => null })); +vi.mock("../top-bar", () => ({ TopBar: () => null })); +vi.mock("../offline-banner", () => ({ OfflineBanner: () => null })); +vi.mock("../shared-record-banner", () => ({ SharedRecordBanner: () => null })); +vi.mock("@/components/gamification/achievement-unlock-notifier", () => ({ + AchievementUnlockNotifier: () => null, +})); +vi.mock("@/components/insights/layout-coach-fab", () => ({ + LayoutCoachFab: () => null, +})); +vi.mock("@/components/insights/layout-coach-mount", () => ({ + LayoutCoachMount: () => null, +})); +vi.mock("@/components/onboarding/tour-launcher", () => ({ + TourLauncher: () => null, +})); +vi.mock("@/components/day/day-layer", () => ({ DayLayerMount: () => null })); +vi.mock("@/components/command-palette/command-palette.lazy", () => ({ + CommandPaletteMount: () => null, +})); +vi.mock("../global-shortcuts", () => ({ GlobalShortcuts: () => null })); + +import { AuthShell } from "../auth-shell"; + +/** + * A long list on `/labs`, a short detail page anywhere else. The list is + * short while it loads (a skeleton), and each page mounts afresh. + */ +function Page() { + const pathname = usePathname(); + return ; +} + +function PageBody({ pathname }: { pathname: string }) { + const [loaded, setLoaded] = useState(false); + useEffect(() => { + const timer = setTimeout(() => setLoaded(true), 200); + return () => clearTimeout(timer); + }, []); + const long = pathname === "/labs" && loaded; + return ( +
+ ); +} + +function main(): HTMLElement { + return document.getElementById("main-content")!; +} + +async function nextFrames(): Promise { + for (let i = 0; i < 3; i += 1) { + await new Promise((resolve) => requestAnimationFrame(resolve)); + } +} + +/** A link tap: the router pushes the entry and renders the next page. */ +async function openPage(path: string): Promise { + window.history.pushState(null, "", path); + window.dispatchEvent(new Event(URL_STATE_EVENT)); + await expect + .poll(() => document.querySelector(`[data-path="${path}"]`)) + .toBeTruthy(); +} + +describe("AuthShell scroll position", () => { + it("opens a new page at the top and returns to the list's offset on Back, once the list has loaded", async () => { + stubFetch(); + window.history.replaceState(null, "", "/labs"); + await renderWithApp( + + + , + { user: { id: "signed-in" } }, + ); + await expect + .poll(() => document.querySelector("[data-loaded]")) + .toBeTruthy(); + + main().scrollTop = 1500; + await nextFrames(); + expect(main().scrollTop).toBe(1500); + + await openPage("/labs/biomarker-1"); + await expect.poll(() => main().scrollTop).toBe(0); + + window.history.back(); + await expect + .poll(() => document.querySelector('[data-path="/labs"]')) + .toBeTruthy(); + await expect.poll(() => main().scrollTop).toBe(1500); + }); +}); diff --git a/src/components/layout/auth-shell.tsx b/src/components/layout/auth-shell.tsx index e6b519dc7..35a73b3dc 100644 --- a/src/components/layout/auth-shell.tsx +++ b/src/components/layout/auth-shell.tsx @@ -83,6 +83,66 @@ const PUBLIC_PATHS = [ "/claim/", ]; +/** + * `
` is the app shell's one scroller and stays mounted + * across routes, so its offset would carry from page to page, and the + * browser's own restoration on Back moves only the document, never `
`. + * A forward navigation starts at the top; Back and Forward return to the + * offset that entry was left at. + */ +function useShellScrollMemory(pathname: string): void { + // Recorded while scrolling: once the pathname has changed, `
` holds + // the next page and the offset is gone. + const offsets = useRef(new Map()); + // Set by `popstate` for the entry it lands on, taken by the next render. + const restore = useRef<{ url: string; top: number } | null>(null); + useEffect(() => { + const here = () => `${location.pathname}${location.search}`; + const remember = (event: Event) => { + const el = event.target; + if (el instanceof HTMLElement && el.id === "main-content") { + offsets.current.set(here(), el.scrollTop); + } + }; + const recall = () => { + restore.current = { url: here(), top: offsets.current.get(here()) ?? 0 }; + }; + const capture = { capture: true, passive: true } as const; + document.addEventListener("scroll", remember, capture); + window.addEventListener("popstate", recall); + return () => { + document.removeEventListener("scroll", remember, capture); + window.removeEventListener("popstate", recall); + }; + }, []); + + useEffect(() => { + const target = restore.current; + restore.current = null; + const main = document.getElementById("main-content"); + if (target?.url !== `${location.pathname}${location.search}`) { + if (main) main.scrollTop = 0; + // The body-scrolled shells (sign-in, setup, legal pages) scroll the + // document instead. + window.scrollTo(0, 0); + return; + } + if (!main) return; + // A list still painting its skeleton is shorter than the offset; keep + // trying until it has the height, for at most a second. + const until = performance.now() + 1000; + let frame = 0; + const apply = () => { + main.scrollTop = target.top; + if (main.scrollTop < target.top - 1 && performance.now() < until) { + frame = requestAnimationFrame(apply); + } + }; + apply(); + return () => cancelAnimationFrame(frame); + }, [pathname]); +} + /** * A session that ended without passing through the shell's session-end wipe * (an expired cookie the edge sent straight to the sign-in page, a sign-in @@ -213,40 +273,7 @@ export function AuthShell({ isBodyScrolled ? "body" : rendersAppShell ? "shell" : null, ); - // v1.25.1 — the authenticated shell keeps `
` - // mounted across every route change, so its scroll offset survives - // navigation: scroll down in one section (e.g. Settings → Sources - // Priority), hop to another, and the new view opens already scrolled. - // Reset the scroll container to the top on every forward navigation. - // A back/forward navigation skips the reset so the browser's own scroll - // restoration stands: `popstate` arms a one-shot skip for the single - // pathname change it triggers, and a macrotask clears the flag so it can - // never get stuck on a later forward navigation. - const skipNextScrollReset = useRef(false); - useEffect(() => { - const armSkip = () => { - skipNextScrollReset.current = true; - setTimeout(() => { - skipNextScrollReset.current = false; - }, 0); - }; - window.addEventListener("popstate", armSkip); - return () => window.removeEventListener("popstate", armSkip); - }, []); - - useEffect(() => { - if (skipNextScrollReset.current) { - skipNextScrollReset.current = false; - return; - } - const main = document.getElementById("main-content"); - if (main) { - main.scrollTop = 0; - } - // Fallback for the body-scrolled shells (login / onboarding / standalone - // legal pages) where the document, not `
`, owns the scroll. - window.scrollTo(0, 0); - }, [pathname]); + useShellScrollMemory(pathname); // Session END on a protected page (`/api/auth/me` answered 401/403): wipe // every client cache, since the QueryClient outlives navigation and health