fix(ui): restore each page's scroll position when navigating back (#5892)

* fix(ui): anchor detail pages to the top when opened from a list

React Router keeps the previous page's scroll offset, so opening an album from
a scrolled list started the detail page mid-song-list. Artist pages had the
same bug; it just shows less because the artist list is rarely long enough to
scroll far.

Keyed on the record id rather than mount, so detail-to-detail navigation (an
album's artist link) resets too, and so the scroll waits for the record instead
of firing against an empty page.

* feat(ui): remember each route's scroll offset

A hook that saves window.scrollY per history entry and, on navigation, tops a
pushed route or returns a popped one to where it was. Keyed on location.key so
back and forward each restore their own offset, and bounded because history
entries are not.

Callers pass their own readiness: restoring before the rows render leaves the
document too short, so the browser clamps to the top and the offset is lost with
no error to show for it.

* refactor(ui): fold scroll-to-top into route scroll restoration

useScrollToTop only ever forced the top, so it fixed opening a detail page from a
scrolled list and did nothing for the return trip: coming back to the list landed
wherever it happened to be. The new hook covers both, and its readiness argument
carries over the reason the old one keyed on the record id - waiting for the
record rather than firing against an empty page.

* feat(ui): restore the album list's position when returning to it

The list is the page worth remembering: scrolling a long way to find an album and
landing back at the top is the annoyance the detail-page scroll fix could not
reach.

Mounted as a pass-through around the List's child rather than beside it, because
React Admin calls Children.only on List children and clones that one child with
the list props - which is also how this reads loaded/total to wait for rows.

* fix(ui): key scroll restoration on the route, not location.key

Hash history never assigns location.key - history.js calls createLocation with an
undefined key and warns that it cannot carry state - so every route collapsed
into one slot. The detail page's own scroll listener then overwrote the list's
saved offset with 0, and going back restored that 0. Only the scroll-to-top half
worked, which is what made it look fine.

Keyed on pathname+search instead. The spec that would have caught this asserts
two routes keep separate offsets; the previous ones mocked the router and so
only ever confirmed the assumption.

* refactor(ui): tidy up scroll restoration after review

Four cleanups, none changing behaviour:

ScrollRestorer read loaded/total off whatever props React Admin happened to clone
in. useListContext is the contract for those values and is already used three
times elsewhere, including further down this same file; the clone payload is an
implementation detail that would fail silently if it ever changed.

The restore runs in a layout effect now. A passive effect is flushed after paint,
so an image-heavy grid was painted at the old offset and then jumped.

MAX_ENTRIES matches the naming every other module-level cap in common/ uses.

The per-instance guard against re-restoring was deletable with a green suite, so
it now has a spec: readiness flickering on one route must not scroll twice.

* fix(ui): resolve media queries on the first render

MUI defaults useMediaQuery and withWidth to SSR-safe two-pass rendering: the first
render reports the wrong breakpoint and a layout effect corrects it. Navidrome is
client-only, so every mount paints a layout it immediately reflows. Album tiles
render without their artist line and grow 14px when it arrives, and react-admin's
own buttons paint labelled before collapsing to icons, which together grew the
album list 79px right after a scroll restore and dragged the position with it.

Setting noSsr on the theme reaches react-admin's internals too. withWidth needs its
own option because it gates on a mount flag rather than on the media query.

* fix(ui): keep restoring the scroll offset until it sticks

A single scrollTo assumes the page is already at its final height. It often is not:
the artist page reports ready as soon as its cached header record renders, while the
albums below are still loading, so the browser clamps the offset to the top of a
viewport-tall document and it is lost with no error.

The restore now retries until the offset lands, yielding as soon as the position moves
somewhere we did not put it. Keying that on input events instead breaks the trackpad
back-swipe, whose wheel momentum keeps firing through the restore window.

Offsets are also committed when leaving the page rather than on every scroll. Tearing
the page down collapses the document and snaps to the top, and a live listener recorded
that clamp over the offset being left behind.

* refactor(ui): tidy up scroll restoration after review

Folds the offset commit into the scroll tracker's own cleanup now that the tracker
ignores the collapse snap, so one effect does what two did. Skips scheduling the retry
loop when the first scrollTo already landed, and only pays for the scrollHeight read on
scroll events that reach the top, since that is the only case the guard can fire.

The theme hook's own prefers-color-scheme query runs above the ThemeProvider carrying
the new prop, so it needs noSsr passed directly or the auto theme still renders dark
first and flips.

Also lifts the repeated fake-timer fixture in the specs into one helper and restores
real timers from afterEach, so a failing assertion no longer leaves them installed for
every later spec.

* fix(ui): bank the scroll offset before the next page tops itself

Committing the offset from the scroll listener's passive cleanup ran too late. The
incoming page scrolls to the top in its own layout effect, and the outgoing page's
listener is still attached at that moment, so it recorded that 0 over the offset being
left behind. Whether it did depended on when the browser dispatched the scroll event
relative to React's passive flush, which made returning to a detail page lose its
position intermittently.

Moving the commit back into a layout cleanup makes it deterministic: React runs those
in the mutation phase, before the incoming page's layout effects. The scrollHeight
guard in the listener went with it, since it only covered the case where the incoming
page was shorter than the viewport and the ordering covers all of them.

ui/src/album/AlbumList.jsx

@@ -1,3 +1,4 @@
+import { cloneElement } from 'react'
 import { useSelector } from 'react-redux'
 import { Redirect, useLocation } from 'react-router-dom'
 import {
@@ -23,6 +24,7 @@ import {
   Title,
   useAlbumsPerPage,
   useResourceRefresh,
+  useScrollRestoration,
   useSetToggleableFields,
 } from '../common'
 import AlbumListActions from './AlbumListActions'
@@ -39,6 +41,13 @@ import ExpandInfoDialog from '../dialogs/ExpandInfoDialog'
 import { humanize } from 'inflection'
 import { makeStyles } from '@material-ui/core/styles'
 
+// Waits for rows: restoring into an unrendered list leaves the page too short to hold the offset.
+const ScrollRestorer = ({ children, ...rest }) => {
+  const { loaded, total } = useListContext()
+  useScrollRestoration(loaded && total > 0)
+  return cloneElement(children, rest)
+}
+
 const useStyles = makeStyles({
   chip: {
     margin: 0,
@@ -255,11 +264,13 @@ const AlbumList = (props) => {
         }
         title={<AlbumListTitle albumListType={albumListType} />}
       >
-        {albumView.grid ? (
-          <AlbumGridView albumListType={albumListType} {...props} />
-        ) : (
-          <AlbumTableView {...props} />
-        )}
+        <ScrollRestorer>
+          {albumView.grid ? (
+            <AlbumGridView albumListType={albumListType} {...props} />
+          ) : (
+            <AlbumTableView {...props} />
+          )}
+        </ScrollRestorer>
       </List>
       <ExpandInfoDialog content={<AlbumInfo />} />
     </>

ui/src/album/AlbumShow.jsx

@@ -10,7 +10,7 @@ import { makeStyles } from '@material-ui/core/styles'
 import AlbumSongs from './AlbumSongs'
 import AlbumDetails from './AlbumDetails'
 import AlbumActions from './AlbumActions'
-import { useResourceRefresh, Title } from '../common'
+import { useResourceRefresh, useScrollRestoration, Title } from '../common'
 
 const useStyles = makeStyles(
   (theme) => ({
@@ -28,6 +28,7 @@ const AlbumShowLayout = (props) => {
   const { record } = context
   const classes = useStyles()
   useResourceRefresh('album', 'song')
+  useScrollRestoration(!!record?.id)
 
   return (
     <>

ui/src/artist/ArtistShow.jsx

@@ -13,7 +13,12 @@ import subsonic from '../subsonic'
 import AlbumGridView from '../album/AlbumGridView'
 import MobileArtistDetails from './MobileArtistDetails'
 import DesktopArtistDetails from './DesktopArtistDetails'
-import { useAlbumsPerPage, useResourceRefresh, Title } from '../common/index.js'
+import {
+  useAlbumsPerPage,
+  useResourceRefresh,
+  useScrollRestoration,
+  Title,
+} from '../common/index.js'
 import ArtistActions from './ArtistActions'
 import { makeStyles } from '@material-ui/core'
 
@@ -85,6 +90,7 @@ const ArtistShowLayout = (props) => {
   const [, perPageOptions] = useAlbumsPerPage(width)
   const classes = useStyles()
   useResourceRefresh('artist', 'album')
+  useScrollRestoration(!!record?.id)
 
   const maxPerPage = 90
   let perPage = 0

ui/src/common/ArtistLinkField.jsx

@@ -7,7 +7,8 @@ import { intersperse } from '../utils/index.js'
 import { useDispatch } from 'react-redux'
 import { closeExtendedInfoDialog } from '../actions/dialogs.js'
 
-const ALink = withWidth()((props) => {
+// noSSR: withWidth otherwise renders null until mounted, so the artist line pops in and grows the row.
+const ALink = withWidth({ noSSR: true })((props) => {
   const { artist, width, ...rest } = props
   const artistLink = useGetHandleArtistClick(width)
   const dispatch = useDispatch()

ui/src/common/index.js

@@ -28,6 +28,7 @@ export * from './useAlbumsPerPage'
 export * from './useGetHandleArtistClick'
 export * from './useInterval'
 export * from './useResourceRefresh'
+export * from './useScrollRestoration'
 export * from './useRefreshOnEvents'
 export * from './useToggleLove'
 export * from './useTraceUpdate'

ui/src/common/useScrollRestoration.jsx

@@ -0,0 +1,64 @@
+import { useEffect, useLayoutEffect, useRef } from 'react'
+import { useHistory, useLocation } from 'react-router-dom'
+
+// Keyed on the route because hash history assigns no location.key, so every page would
+// otherwise share one slot and overwrite the offset we came back for.
+const positions = new Map()
+const MAX_ENTRIES = 50
+const MAX_FRAMES = 60
+
+export const useScrollRestoration = (ready = true) => {
+  const { pathname, search } = useLocation()
+  const history = useHistory()
+  const key = pathname + search
+  const handled = useRef(null)
+  const latest = useRef(0)
+
+  useEffect(() => {
+    latest.current = window.scrollY
+    const track = () => {
+      latest.current = window.scrollY
+    }
+    window.addEventListener('scroll', track, { passive: true })
+    return () => window.removeEventListener('scroll', track)
+  }, [key])
+
+  // Layout cleanup, so the offset is banked while this page is torn down. A passive one runs
+  // after the incoming page tops itself, and this page's listener records that 0 first.
+  useLayoutEffect(
+    () => () => {
+      positions.delete(key)
+      positions.set(key, latest.current)
+      if (positions.size > MAX_ENTRIES) {
+        positions.delete(positions.keys().next().value)
+      }
+    },
+    [key],
+  )
+
+  // Layout effect: a passive one runs after paint, so the list would be painted at the old
+  // offset first and visibly jump.
+  useLayoutEffect(() => {
+    if (!ready || handled.current === key) return
+    handled.current = key
+    const saved = positions.get(key)
+    const top = history.action === 'POP' && saved !== undefined ? saved : 0
+    window.scrollTo({ top })
+    if (!top || Math.round(window.scrollY) === top) return
+
+    // The page can still be filling in, and until it is tall enough the browser clamps us to
+    // the top. Keep asking until the offset sticks, yielding if anything else moves us.
+    let frame
+    let frames = 0
+    let landed = Math.round(window.scrollY)
+    const retry = () => {
+      const y = Math.round(window.scrollY)
+      if (y === top || y !== landed || ++frames > MAX_FRAMES) return
+      window.scrollTo({ top })
+      landed = Math.round(window.scrollY)
+      frame = requestAnimationFrame(retry)
+    }
+    frame = requestAnimationFrame(retry)
+    return () => cancelAnimationFrame(frame)
+  }, [ready, key, history.action])
+}

ui/src/common/useScrollRestoration.test.jsx

@@ -0,0 +1,222 @@
+import { renderHook, act } from '@testing-library/react-hooks'
+import { render } from '@testing-library/react'
+import { useLayoutEffect } from 'react'
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
+
+const mockLocation = { pathname: '/album', search: '' }
+const mockHistory = { action: 'PUSH' }
+vi.mock('react-router-dom', () => ({
+  useLocation: () => mockLocation,
+  useHistory: () => mockHistory,
+}))
+
+import { useScrollRestoration } from './useScrollRestoration'
+
+describe('useScrollRestoration', () => {
+  beforeEach(() => {
+    window.scrollTo = vi.fn()
+    window.scrollY = 0
+    mockLocation.pathname = '/album'
+    mockLocation.search = ''
+    mockHistory.action = 'PUSH'
+  })
+  afterEach(() => {
+    vi.useRealTimers()
+    vi.clearAllMocks()
+  })
+
+  const scrollTo = (y) => {
+    window.scrollY = y
+    act(() => {
+      window.dispatchEvent(new Event('scroll'))
+    })
+  }
+
+  // Returns to a page whose rows have not arrived, so scrollTo still clamps. The returned setter
+  // grows the page to the height it reaches once they do.
+  const returnToUnfilledPage = (saved) => {
+    vi.useFakeTimers()
+    let maxScroll = saved
+    window.scrollTo = vi.fn(({ top }) => {
+      window.scrollY = Math.min(top, maxScroll)
+    })
+
+    const first = renderHook(() => useScrollRestoration())
+    scrollTo(saved)
+    first.unmount()
+
+    maxScroll = 0
+    window.scrollY = 0
+    mockHistory.action = 'POP'
+    renderHook(() => useScrollRestoration())
+    return (height) => (maxScroll = height)
+  }
+
+  const advanceFrames = () => act(() => vi.advanceTimersByTime(100))
+
+  it('starts a pushed route at the top', () => {
+    renderHook(() => useScrollRestoration())
+    expect(window.scrollTo).toHaveBeenCalledWith({ top: 0 })
+  })
+
+  it('restores the saved offset when returning to an entry', () => {
+    const first = renderHook(() => useScrollRestoration())
+    scrollTo(640)
+    first.unmount()
+
+    mockLocation.pathname = '/album/al-1/show'
+    const second = renderHook(() => useScrollRestoration())
+    expect(window.scrollTo).toHaveBeenLastCalledWith({ top: 0 })
+    second.unmount()
+
+    mockLocation.pathname = '/album'
+    mockHistory.action = 'POP'
+    renderHook(() => useScrollRestoration())
+    expect(window.scrollTo).toHaveBeenLastCalledWith({ top: 640 })
+  })
+
+  it('tops a pushed route even if that key was seen before', () => {
+    const first = renderHook(() => useScrollRestoration())
+    scrollTo(500)
+    first.unmount()
+
+    mockHistory.action = 'PUSH'
+    renderHook(() => useScrollRestoration())
+    expect(window.scrollTo).toHaveBeenLastCalledWith({ top: 0 })
+  })
+
+  // Restoring before the rows exist leaves the document too short: the browser clamps to the
+  // top and the offset is lost with no error.
+  it('waits for readiness before touching the scroll position', () => {
+    const { rerender } = renderHook(
+      ({ ready }) => useScrollRestoration(ready),
+      {
+        initialProps: { ready: false },
+      },
+    )
+    expect(window.scrollTo).not.toHaveBeenCalled()
+
+    rerender({ ready: true })
+    expect(window.scrollTo).toHaveBeenCalledWith({ top: 0 })
+  })
+
+  // Hash history assigns no location.key, so an implementation keyed on it would collapse every
+  // route into one slot and the detail page would erase the list's offset before we returned.
+  it('keeps a separate offset per route', () => {
+    const list = renderHook(() => useScrollRestoration())
+    scrollTo(900)
+    list.unmount()
+
+    mockLocation.pathname = '/album/al-1/show'
+    const detail = renderHook(() => useScrollRestoration())
+    scrollTo(0)
+    detail.unmount()
+
+    mockLocation.pathname = '/album'
+    mockHistory.action = 'POP'
+    renderHook(() => useScrollRestoration())
+    expect(window.scrollTo).toHaveBeenLastCalledWith({ top: 900 })
+  })
+
+  // Without the per-instance guard this fires again when readiness flickers, fighting a user who
+  // has already started scrolling. Nothing else in the suite pins it.
+  it('does not scroll again when readiness flickers on the same route', () => {
+    const { rerender } = renderHook(
+      ({ ready }) => useScrollRestoration(ready),
+      {
+        initialProps: { ready: true },
+      },
+    )
+    expect(window.scrollTo).toHaveBeenCalledTimes(1)
+
+    rerender({ ready: false })
+    rerender({ ready: true })
+    expect(window.scrollTo).toHaveBeenCalledTimes(1)
+  })
+
+  it('scrolls once per entry, not on every re-render', () => {
+    const { rerender } = renderHook(() => useScrollRestoration())
+    rerender()
+    rerender()
+    expect(window.scrollTo).toHaveBeenCalledTimes(1)
+  })
+
+  // The artist page reports ready as soon as its header record is cached, while the albums below
+  // are still loading. A single scrollTo lands on a viewport-tall document and is clamped to 0.
+  it('keeps trying until the page is tall enough to hold the offset', () => {
+    const fillPageTo = returnToUnfilledPage(800)
+    expect(window.scrollY).toBe(0)
+
+    fillPageTo(1200)
+    advanceFrames()
+    expect(window.scrollY).toBe(800)
+  })
+
+  it('stops retrying once the user scrolls somewhere else', () => {
+    const fillPageTo = returnToUnfilledPage(800)
+
+    fillPageTo(1200)
+    window.scrollY = 300
+    advanceFrames()
+    expect(window.scrollY).toBe(300)
+  })
+
+  // A trackpad back-swipe keeps firing wheel momentum through the restore, so treating input as
+  // the yield signal cancels the gesture's own restore.
+  it('keeps restoring while wheel momentum fires, as a back-swipe does', () => {
+    const fillPageTo = returnToUnfilledPage(800)
+
+    act(() => {
+      for (let i = 0; i < 10; i++) {
+        window.dispatchEvent(
+          new WheelEvent('wheel', { deltaX: -30, deltaY: 0 }),
+        )
+      }
+    })
+    fillPageTo(1200)
+    advanceFrames()
+    expect(window.scrollY).toBe(800)
+  })
+
+  // The incoming page tops itself in its own layout effect, while this page's scroll listener is
+  // still attached. Banking the offset any later than the teardown records that 0 instead.
+  it('records the offset before the next page scrolls to the top', () => {
+    const Outgoing = () => {
+      useScrollRestoration()
+      return null
+    }
+    const Incoming = () => {
+      useLayoutEffect(() => {
+        window.scrollY = 0
+        window.dispatchEvent(new Event('scroll'))
+      }, [])
+      return null
+    }
+
+    const { rerender } = render(<Outgoing />)
+    scrollTo(1400)
+
+    mockLocation.pathname = '/album/al-1/show'
+    rerender(<Incoming />)
+
+    mockLocation.pathname = '/album'
+    mockHistory.action = 'POP'
+    renderHook(() => useScrollRestoration())
+    expect(window.scrollTo).toHaveBeenLastCalledWith({ top: 1400 })
+  })
+
+  it('keeps saving after the restore, so leaving again remembers the new offset', () => {
+    const first = renderHook(() => useScrollRestoration())
+    scrollTo(200)
+    first.unmount()
+
+    mockHistory.action = 'POP'
+    const second = renderHook(() => useScrollRestoration())
+    expect(window.scrollTo).toHaveBeenLastCalledWith({ top: 200 })
+    scrollTo(900)
+    second.unmount()
+
+    renderHook(() => useScrollRestoration())
+    expect(window.scrollTo).toHaveBeenLastCalledWith({ top: 900 })
+  })
+})

ui/src/themes/useCurrentTheme.js

@@ -3,10 +3,14 @@ import useMediaQuery from '@material-ui/core/useMediaQuery'
 import themes from './index'
 import { AUTO_THEME_ID } from '../consts'
 import config from '../config'
-import { useEffect } from 'react'
+import { useEffect, useMemo } from 'react'
 
 const useCurrentTheme = () => {
-  const prefersLightMode = useMediaQuery('(prefers-color-scheme: light)')
+  // Runs above the ThemeProvider carrying the prop below, so it needs its own noSsr or the
+  // auto theme renders dark first and flips.
+  const prefersLightMode = useMediaQuery('(prefers-color-scheme: light)', {
+    noSsr: true,
+  })
   const theme = useSelector((state) => {
     if (state.theme === AUTO_THEME_ID) {
       return prefersLightMode ? themes.LightTheme : themes.DarkTheme
@@ -50,7 +54,15 @@ const useCurrentTheme = () => {
     document.body.style.backgroundColor = bgColor
   }, [theme])
 
-  return theme
+  // We never server-render, so let media queries resolve on the first render: the default
+  // defers them to an effect, which makes every mount paint the wrong breakpoint and reflow.
+  return useMemo(
+    () => ({
+      ...theme,
+      props: { ...theme.props, MuiUseMediaQuery: { noSsr: true } },
+    }),
+    [theme],
+  )
 }
 
 export default useCurrentTheme