diff --git a/src/lib/client/scroll-restore.ts b/src/lib/client/scroll-restore.ts index e923d3a..bbcc81e 100644 --- a/src/lib/client/scroll-restore.ts +++ b/src/lib/client/scroll-restore.ts @@ -5,9 +5,11 @@ // wishlist, shopping-list) are still empty at that point, so the saved // scrollY can't be reached and the browser clamps to 0. // -// We patch this by saving scrollY on beforeNavigate and re-applying it -// after popstate as soon as the document is tall enough — using rAF -// polling with a hard time budget so we never spin. +// We patch this by saving scrollY on beforeNavigate (keyed by the URL +// we're leaving — NOT location.pathname, which on popstate is already +// the new URL by the time the callback fires) and re-applying it after +// popstate as soon as the document is tall enough — rAF-polled with a +// hard time budget so we never spin. const STORAGE_KEY = 'kochwas:scroll'; const POLL_BUDGET_MS = 1500; @@ -34,26 +36,26 @@ function writeMap(map: ScrollMap): void { } } -function urlKey(): string { - if (typeof location === 'undefined') return ''; - return location.pathname + location.search; +function keyFor(url: URL): string { + return url.pathname + url.search; } -export function recordScroll(): void { +export function recordScroll(fromUrl: URL | null | undefined): void { if (typeof window === 'undefined') return; - const key = urlKey(); - if (!key) return; + if (!fromUrl) return; const map = readMap(); - map[key] = window.scrollY; + map[keyFor(fromUrl)] = window.scrollY; writeMap(map); } -export function restoreScroll(navType: string | null | undefined): void { +export function restoreScroll( + navType: string | null | undefined, + toUrl: URL | null | undefined +): void { if (typeof window === 'undefined') return; if (navType !== 'popstate') return; - const key = urlKey(); - if (!key) return; - const target = readMap()[key]; + if (!toUrl) return; + const target = readMap()[keyFor(toUrl)]; if (!target || target < MIN_RESTORE_Y) return; const start = performance.now(); diff --git a/src/routes/+layout.svelte b/src/routes/+layout.svelte index e675128..73f8a9b 100644 --- a/src/routes/+layout.svelte +++ b/src/routes/+layout.svelte @@ -98,8 +98,8 @@ } } - beforeNavigate(() => { - recordScroll(); + beforeNavigate((nav) => { + recordScroll(nav.from?.url); }); afterNavigate((nav) => { @@ -112,7 +112,7 @@ // wurde. void wishlistStore.refresh(); void shoppingCartStore.refresh(); - restoreScroll(nav.type); + restoreScroll(nav.type, nav.to?.url); }); onMount(() => { diff --git a/tests/unit/scroll-restore.test.ts b/tests/unit/scroll-restore.test.ts index ac164d8..6119ae8 100644 --- a/tests/unit/scroll-restore.test.ts +++ b/tests/unit/scroll-restore.test.ts @@ -20,6 +20,10 @@ function setViewportHeight(h: number) { Object.defineProperty(window, 'innerHeight', { value: h, configurable: true }); } +function url(path: string): URL { + return new URL(path, 'https://example.test'); +} + describe('scroll-restore', () => { let scrollToSpy: ReturnType; @@ -31,7 +35,6 @@ describe('scroll-restore', () => { scrollToSpy = vi .spyOn(window, 'scrollTo') .mockImplementation(() => undefined as unknown as void); - window.history.replaceState({}, '', '/wishlist'); }); afterEach(() => { @@ -39,64 +42,84 @@ describe('scroll-restore', () => { vi.useRealTimers(); }); - it('records scrollY keyed by pathname+search', () => { + it('records scrollY keyed by from-url pathname+search', () => { setScrollY(1200); - recordScroll(); + recordScroll(url('/wishlist')); const map = JSON.parse(sessionStorage.getItem(STORAGE_KEY) ?? '{}'); expect(map['/wishlist']).toBe(1200); }); it('keeps separate entries per URL', () => { setScrollY(500); - recordScroll(); - window.history.replaceState({}, '', '/?q=hi'); + recordScroll(url('/wishlist')); setScrollY(900); - recordScroll(); + recordScroll(url('/?q=hi')); const map = JSON.parse(sessionStorage.getItem(STORAGE_KEY) ?? '{}'); expect(map['/wishlist']).toBe(500); expect(map['/?q=hi']).toBe(900); }); + it('does not overwrite a stored URL when called with a different from-url', () => { + // This is the regression: on popstate, location.pathname is already + // the new URL. Recording must use nav.from.url (the page being left), + // not location, or we wipe the destination's saved scrollY. + setScrollY(500); + recordScroll(url('/')); + setScrollY(0); + recordScroll(url('/recipes/1')); + const map = JSON.parse(sessionStorage.getItem(STORAGE_KEY) ?? '{}'); + expect(map['/']).toBe(500); + expect(map['/recipes/1']).toBe(0); + }); + + it('skips when from-url is missing', () => { + setScrollY(900); + recordScroll(null); + expect(sessionStorage.getItem(STORAGE_KEY)).toBeNull(); + }); + it('skips restore for non-popstate navigation', () => { sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1000 })); setDocHeight(5000); - restoreScroll('link'); + restoreScroll('link', url('/wishlist')); + expect(scrollToSpy).not.toHaveBeenCalled(); + }); + + it('skips restore when to-url is missing', () => { + sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1000 })); + restoreScroll('popstate', null); expect(scrollToSpy).not.toHaveBeenCalled(); }); it('skips restore when no entry stored', () => { setDocHeight(5000); - restoreScroll('popstate'); + restoreScroll('popstate', url('/wishlist')); expect(scrollToSpy).not.toHaveBeenCalled(); }); it('skips restore for trivial scrollY (noise)', () => { sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 10 })); setDocHeight(5000); - restoreScroll('popstate'); + restoreScroll('popstate', url('/wishlist')); expect(scrollToSpy).not.toHaveBeenCalled(); }); it('scrolls immediately when document is already tall enough', async () => { sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1000 })); setDocHeight(5000); - restoreScroll('popstate'); - // rAF resolves on next microtask in jsdom + restoreScroll('popstate', url('/wishlist')); await new Promise((r) => requestAnimationFrame(() => r(null))); expect(scrollToSpy).toHaveBeenCalledWith({ top: 1000, left: 0, behavior: 'instant' }); }); it('waits via rAF until document grows tall enough', async () => { sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1500 })); - // Initially short — would clamp. setDocHeight(900); - restoreScroll('popstate'); + restoreScroll('popstate', url('/wishlist')); await new Promise((r) => requestAnimationFrame(() => r(null))); expect(scrollToSpy).not.toHaveBeenCalled(); - // Simulate async data loading and document growing. setDocHeight(3000); - // Allow rAF to fire again. await new Promise((r) => requestAnimationFrame(() => r(null))); expect(scrollToSpy).toHaveBeenCalledWith({ top: 1500, left: 0, behavior: 'instant' }); });