e2e test it baby

app/hooks/use-scroll-restoration.ts

@@ -1,15 +1,22 @@
 import { useEffect, useRef } from 'react'
 import { useLocation, useNavigation } from 'react-router-dom'
 
-export function useScrollRestoration(elementRef: React.RefObject<HTMLElement>) {
+/**
+ * Given a ref to a scrolling container element, keep track of its scroll
+ * position before navigation and restore it on return (e.g., back/forward nav).
+ * Note that `location.key` is used as the cache key, not `location.pathname`,
+ * so the same path navigated to at different points in the history stack will
+ * not share the same scroll position.
+ */
+export function useScrollRestoration(container: React.RefObject<HTMLElement>) {
   const cache = useRef(new Map<string, number>())
   const { key } = useLocation()
   const { state } = useNavigation()
   useEffect(() => {
     if (state === 'loading') {
-      cache.current.set(key, elementRef.current?.scrollTop ?? 0)
+      cache.current.set(key, container.current?.scrollTop ?? 0)
     } else if (state === 'idle' && cache.current.has(key)) {
-      elementRef.current?.scrollTo(0, cache.current.get(key)!)
+      container.current?.scrollTo(0, cache.current.get(key)!)
     }
-  }, [key, state, elementRef])
+  }, [key, state, container])
 }

app/layouts/helpers.tsx

@@ -15,7 +15,7 @@ export function ContentPane() {
   const ref = useRef<HTMLDivElement>(null)
   useScrollRestoration(ref)
   return (
-    <div ref={ref} className="flex flex-col overflow-auto">
+    <div ref={ref} className="flex flex-col overflow-auto" data-testid="scroll-container">
       <div className="flex flex-grow flex-col pb-8">
         <SkipLinkTarget />
         <main className="[&>*]:gutter">

app/test/e2e/scroll-restore.e2e.ts

@@ -0,0 +1,58 @@
+import { type Page, expect, test } from './utils'
+
+async function expectScrollTop(page: Page, expected: number) {
+  const container = await page.getByTestId('scroll-container')
+  const getScrollTop = () => container.evaluate((el: HTMLElement) => el.scrollTop)
+  await expect.poll(getScrollTop, { timeout: 2000 }).toBe(expected)
+}
+
+async function scrollTo(page: Page, to: number) {
+  const container = await page.getByTestId('scroll-container')
+  await container.evaluate((el: HTMLElement, to) => el.scrollTo(0, to), to)
+}
+
+test('scroll restore', async ({ page }) => {
+  // open small window to make scrolling easier
+  await page.setViewportSize({ width: 800, height: 500 })
+
+  // nav to disks and scroll it
+  await page.goto('/projects/mock-project/disks')
+  await expectScrollTop(page, 0)
+  await scrollTo(page, 143)
+
+  // nav to snapshots
+  await page.getByRole('link', { name: 'Snapshots' }).click()
+  await expectScrollTop(page, 0)
+
+  // go back to disks, scroll is restored, scroll it some more
+  await page.goBack()
+  await expect(page).toHaveURL('/projects/mock-project/disks')
+  await expectScrollTop(page, 143)
+  await scrollTo(page, 190)
+
+  // go forward to snapshots, now scroll it
+  await page.goForward()
+  await expect(page).toHaveURL('/projects/mock-project/snapshots')
+  await expectScrollTop(page, 0)
+  await scrollTo(page, 30)
+
+  // new nav to disks
+  await page.getByRole('link', { name: 'Disks' }).click()
+  await expectScrollTop(page, 0)
+
+  // back to snapshots, scroll is restored
+  await page.goBack()
+  await expect(page).toHaveURL('/projects/mock-project/snapshots')
+  await expectScrollTop(page, 30)
+
+  // back again to disks, newer scroll value is restored
+  await page.goBack()
+  await expect(page).toHaveURL('/projects/mock-project/disks')
+  await expectScrollTop(page, 190)
+
+  // forward again to newest disks history entry, scroll remains 0
+  await page.goForward()
+  await page.goForward()
+  await expect(page).toHaveURL('/projects/mock-project/disks')
+  await expectScrollTop(page, 0)
+})