Merge branch 'master' into add-scrobble-plugin-api

contrib/navidrome

@@ -2,7 +2,7 @@
   
 name=$RC_SVCNAME
 command="/opt/navidrome/${RC_SVCNAME}"
-command_args="-datafolder /opt/navidrome"
+command_args="--datafolder /opt/navidrome"
 command_user="${RC_SVCNAME}"
 pidfile="/var/run/${RC_SVCNAME}.pid"
 output_log="/opt/navidrome/${RC_SVCNAME}.log"

model/criteria/fields.go

@@ -71,6 +71,11 @@ var fieldMap = map[string]FieldInfo{
 	"albumlastplayed":      {},
 	"albumdateloved":       {},
 	"albumdaterated":       {},
+	"albumdateadded":       {},
+	"albumdatemodified":    {},
+	"albumduration":        {},
+	"albumsongcount":       {},
+	"albumsize":            {},
 	"artistrating":         {},
 	"artistloved":          {Boolean: true},
 	"artistplaycount":      {},

persistence/criteria_sql.go

@@ -21,6 +21,7 @@ const (
 	smartPlaylistJoinNone            smartPlaylistJoinType = 0
 	smartPlaylistJoinAlbumAnnotation smartPlaylistJoinType = 1 << iota
 	smartPlaylistJoinArtistAnnotation
+	smartPlaylistJoinAlbum
 )
 
 func (j smartPlaylistJoinType) has(other smartPlaylistJoinType) bool {
@@ -103,6 +104,11 @@ var smartPlaylistFields = map[string]smartPlaylistField{
 	"albumlastplayed":      {expr: "album_annotation.play_date", joinType: smartPlaylistJoinAlbumAnnotation},
 	"albumdateloved":       {expr: "album_annotation.starred_at", joinType: smartPlaylistJoinAlbumAnnotation},
 	"albumdaterated":       {expr: "album_annotation.rated_at", joinType: smartPlaylistJoinAlbumAnnotation},
+	"albumdateadded":       {expr: "album.created_at", joinType: smartPlaylistJoinAlbum},
+	"albumdatemodified":    {expr: "album.updated_at", joinType: smartPlaylistJoinAlbum},
+	"albumduration":        {expr: "album.duration", joinType: smartPlaylistJoinAlbum},
+	"albumsongcount":       {expr: "album.song_count", joinType: smartPlaylistJoinAlbum},
+	"albumsize":            {expr: "album.size", joinType: smartPlaylistJoinAlbum},
 	"artistrating":         {expr: "artist_annotation.rating", coalesceDefault: 0, joinType: smartPlaylistJoinArtistAnnotation},
 	"artistloved":          {expr: "artist_annotation.starred", coalesceDefault: false, joinType: smartPlaylistJoinArtistAnnotation},
 	"artistplaycount":      {expr: "artist_annotation.play_count", coalesceDefault: 0, joinType: smartPlaylistJoinArtistAnnotation},

persistence/criteria_sql_test.go

@@ -51,6 +51,11 @@ var _ = Describe("Smart playlist criteria SQL", func() {
 		Entry("not in playlist", criteria.NotInPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id NOT IN (SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))", "deadbeef-dead-beef", 1),
 		Entry("album annotation", criteria.Gt{"albumRating": 3}, "album_annotation.rating > ?", 3),
 		Entry("artist annotation", criteria.Is{"artistLoved": true}, "artist_annotation.starred = ?", true),
+		Entry("album column", criteria.Gt{"albumSongCount": 5}, "album.song_count > ?", 5),
+		Entry("album duration column", criteria.Lt{"albumDuration": 600}, "album.duration < ?", 600),
+		Entry("album size column", criteria.Gt{"albumSize": 1000}, "album.size > ?", 1000),
+		Entry("album date column", criteria.After{"albumDateAdded": time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)}, "album.created_at > ?", time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)),
+		Entry("album modified column", criteria.Before{"albumDateModified": time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)}, "album.updated_at < ?", time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)),
 		// Annotation fields use a COALESCE default (0 for numeric, false for bool) so that tracks
 		// with no annotation row behave as that default. To keep the annotation index usable, the
 		// COALESCE is dropped when the compared value cannot match the default (the missing-row
@@ -298,6 +303,11 @@ var _ = Describe("Smart playlist criteria SQL", func() {
 			Expect(newSmartPlaylistCriteria(criteria.Criteria{Sort: "random"}).OrderBy()).To(Equal("random() asc"))
 		})
 
+		It("sorts by album columns bare, with no COALESCE default", func() {
+			Expect(newSmartPlaylistCriteria(criteria.Criteria{Sort: "-albumDateAdded,trackNumber"}).OrderBy()).
+				To(Equal("album.created_at desc, media_file.track_number asc"))
+		})
+
 		It("sorts by multiple fields", func() {
 			Expect(newSmartPlaylistCriteria(criteria.Criteria{Sort: "title,-rating"}).OrderBy()).To(Equal("media_file.title asc, COALESCE(annotation.rating, 0) desc"))
 		})
@@ -323,6 +333,28 @@ var _ = Describe("Smart playlist criteria SQL", func() {
 		}
 	})
 
+	It("declares a joinType matching the table each field selects from", func() {
+		// Omitting the joinType still compiles, so without this the field would only fail at
+		// refresh time with "no such column".
+		joinByTable := map[string]smartPlaylistJoinType{
+			"media_file":        smartPlaylistJoinNone,
+			"annotation":        smartPlaylistJoinNone,
+			"album":             smartPlaylistJoinAlbum,
+			"album_annotation":  smartPlaylistJoinAlbumAnnotation,
+			"artist_annotation": smartPlaylistJoinArtistAnnotation,
+		}
+		for name, field := range smartPlaylistFields {
+			if field.expr == "" {
+				continue
+			}
+			table, _, ok := strings.Cut(field.expr, ".")
+			Expect(ok).To(BeTrue(), "field %q has expr %q with no table prefix", name, field.expr)
+			want, known := joinByTable[table]
+			Expect(known).To(BeTrue(), "field %q selects from unknown table %q", name, table)
+			Expect(field.joinType).To(Equal(want), "field %q selects from %q but declares the wrong joinType", name, table)
+		}
+	})
+
 	Describe("JSON condition merging", func() {
 		It("merges multiple role conditions in an OR group into a single EXISTS", func() {
 			expr := criteria.Any{
@@ -585,5 +617,21 @@ var _ = Describe("Smart playlist criteria SQL", func() {
 
 			Expect(newSmartPlaylistCriteria(c).RequiredJoins().has(smartPlaylistJoinArtistAnnotation)).To(BeTrue())
 		})
+
+		It("keeps a sort-only album join out of the expression joins", func() {
+			c := criteria.Criteria{Expression: criteria.All{criteria.Contains{"title": "love"}}, Sort: "-albumDateAdded"}
+			cSQL := newSmartPlaylistCriteria(c)
+
+			Expect(cSQL.ExpressionJoins()).To(Equal(smartPlaylistJoinNone))
+			Expect(cSQL.RequiredJoins().has(smartPlaylistJoinAlbum)).To(BeTrue())
+		})
+
+		It("distinguishes the album join from the album annotation join", func() {
+			c := criteria.Criteria{Expression: criteria.All{criteria.Gt{"albumRating": 3}}}
+			joins := newSmartPlaylistCriteria(c).RequiredJoins()
+
+			Expect(joins.has(smartPlaylistJoinAlbumAnnotation)).To(BeTrue())
+			Expect(joins.has(smartPlaylistJoinAlbum)).To(BeFalse())
+		})
 	})
 })

persistence/e2e/smartplaylist_test.go

@@ -204,6 +204,43 @@ var _ = Describe("Smart Playlists", func() {
 		})
 	})
 
+	Describe("Album aggregate fields", func() {
+		// Abbey Road and IV have two tracks each; the other four albums have one.
+		It("matches albums with more than one track", func() {
+			results := evaluateRule(`{"all":[{"gt":{"albumsongcount":1}}]}`)
+			Expect(results).To(ConsistOf("Come Together", "Something", "Stairway To Heaven", "Black Dog"))
+		})
+
+		It("matches single-track albums", func() {
+			results := evaluateRule(`{"all":[{"is":{"albumsongcount":1}}]}`)
+			Expect(results).To(ConsistOf("So What", "Bohemian Rhapsody", "All Along the Watchtower",
+				"We Are the Champions"))
+		})
+
+		It("matches albumDateAdded inTheLast 1 day", func() {
+			results := evaluateRule(`{"all":[{"inTheLast":{"albumdateadded":1}}]}`)
+			Expect(results).To(ConsistOf("Come Together", "Something", "Stairway To Heaven", "Black Dog",
+				"So What", "Bohemian Rhapsody", "All Along the Watchtower", "We Are the Champions"))
+		})
+
+		It("matches albumDateModified before a far-future date", func() {
+			results := evaluateRule(`{"all":[{"before":{"albumdatemodified":"2099-01-01"}}]}`)
+			Expect(results).To(HaveLen(8))
+		})
+
+		// Fixture durations are randomized, so only the aggregate being populated can be asserted.
+		It("resolves albumDuration and albumSize to non-zero aggregates", func() {
+			Expect(evaluateRule(`{"all":[{"gt":{"albumduration":0}}]}`)).To(HaveLen(8))
+			Expect(evaluateRule(`{"all":[{"gt":{"albumsize":0}}]}`)).To(HaveLen(8))
+		})
+
+		It("groups by album date and orders within the album (issue #5347)", func() {
+			results := evaluateRuleOrdered(
+				`{"all":[{"is":{"album":"Abbey Road"}}],"sort":"-albumdateadded,tracknumber"}`)
+			Expect(results).To(Equal([]string{"Come Together", "Something"}))
+		})
+	})
+
 	Describe("Logic operators", func() {
 		It("matches with ALL (AND)", func() {
 			results := evaluateRule(`{"all":[{"is":{"genre":"Blues"}},{"gt":{"bpm":130}}]}`)

persistence/smart_playlist_repository.go

@@ -122,7 +122,7 @@ func (r *playlistRepository) resolvePercentageLimit(pls *model.Playlist, rulesSQ
 	exprJoins := rulesSQL.ExpressionJoins()
 	countSq := Select("count(*) as count").From("media_file")
 	countSq = r.addMediaFileAnnotationJoin(countSq, userID)
-	countSq = r.addSmartPlaylistAnnotationJoins(countSq, exprJoins, userID)
+	countSq = r.addSmartPlaylistJoins(countSq, exprJoins, userID)
 	countSq = r.applyLibraryFilter(countSq, "media_file")
 
 	cond, err := rulesSQL.Where()
@@ -144,7 +144,7 @@ func (r *playlistRepository) resolvePercentageLimit(pls *model.Playlist, rulesSQ
 }
 
 // buildSmartPlaylistQuery constructs the SQL query to select media files matching the smart playlist criteria,
-// including necessary joins for annotations and library filtering.
+// including the joins its fields require and library filtering.
 func (r *playlistRepository) buildSmartPlaylistQuery(pls *model.Playlist, rulesSQL smartPlaylistCriteria, userID string) SelectBuilder {
 	orderBy := rulesSQL.OrderBy()
 	sq := Select("row_number() over (order by "+orderBy+") as id", "'"+pls.ID+"' as playlist_id", "media_file.id as media_file_id").
@@ -152,7 +152,7 @@ func (r *playlistRepository) buildSmartPlaylistQuery(pls *model.Playlist, rulesS
 	sq = r.addMediaFileAnnotationJoin(sq, userID)
 
 	requiredJoins := rulesSQL.RequiredJoins()
-	sq = r.addSmartPlaylistAnnotationJoins(sq, requiredJoins, userID)
+	sq = r.addSmartPlaylistJoins(sq, requiredJoins, userID)
 	sq = r.applyLibraryFilter(sq, "media_file")
 	return sq
 }
@@ -166,9 +166,8 @@ func (r *playlistRepository) addMediaFileAnnotationJoin(sq SelectBuilder, userID
 		" AND annotation.user_id = ?)", userID)
 }
 
-// addSmartPlaylistAnnotationJoins adds left joins to the annotation table for albums and artists as needed based on
-// the smart playlist criteria, filtering by user ID to include user-specific annotations in the evaluation.
-func (r *playlistRepository) addSmartPlaylistAnnotationJoins(sq SelectBuilder, joins smartPlaylistJoinType, userID string) SelectBuilder {
+// addSmartPlaylistJoins adds the left joins required by the criteria's fields.
+func (r *playlistRepository) addSmartPlaylistJoins(sq SelectBuilder, joins smartPlaylistJoinType, userID string) SelectBuilder {
 	if joins.has(smartPlaylistJoinAlbumAnnotation) {
 		sq = sq.LeftJoin("annotation AS album_annotation ON ("+
 			"album_annotation.item_id = media_file.album_id"+
@@ -181,6 +180,9 @@ func (r *playlistRepository) addSmartPlaylistAnnotationJoins(sq SelectBuilder, j
 			" AND artist_annotation.item_type = 'artist'"+
 			" AND artist_annotation.user_id = ?)", userID)
 	}
+	if joins.has(smartPlaylistJoinAlbum) {
+		sq = sq.LeftJoin("album ON album.id = media_file.album_id")
+	}
 	return sq
 }
 

persistence/smart_playlist_repository_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/criteria"
 	"github.com/navidrome/navidrome/model/request"
+	"github.com/navidrome/navidrome/utils/slice"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/pocketbase/dbx"
@@ -392,6 +393,40 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() {
 		})
 	})
 
+	Describe("Smart Playlists with Album Aggregate Criteria", func() {
+		BeforeEach(func() {
+			DeferCleanup(configtest.SetupConfig())
+			conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
+		})
+
+		trackIDsOf := func(rules *criteria.Criteria) []string {
+			newPls := model.Playlist{Name: "Album Aggregates", OwnerID: "userid", Rules: rules}
+			Expect(repo.Put(&newPls)).To(Succeed())
+			DeferCleanup(func() { _ = repo.Delete(newPls.ID) })
+
+			pls, err := repo.GetWithTracks(newPls.ID, true, false)
+			Expect(err).ToNot(HaveOccurred())
+			return slice.Map(pls.Tracks, func(t model.PlaylistTrack) string { return t.MediaFileID })
+		}
+
+		It("filters on albumSongCount", func() {
+			// albumMultiDisc (ID "104") is the only fixture album with SongCount > 3
+			rules := &criteria.Criteria{Expression: criteria.All{criteria.Gt{"albumSongCount": 3}}}
+
+			Expect(trackIDsOf(rules)).To(ConsistOf("2001", "2002", "2003", "2004"))
+		})
+
+		It("sorts by an album field not referenced in the expression (issue #5347)", func() {
+			// All four tracks share album 104, so the album date ties and disc/track number decide.
+			rules := &criteria.Criteria{
+				Expression: criteria.All{criteria.Is{"album": "Multi Disc Album"}},
+				Sort:       "-albumDateAdded,discNumber,trackNumber",
+			}
+
+			Expect(trackIDsOf(rules)).To(HaveExactElements("2002", "2004", "2003", "2001"))
+		})
+	})
+
 	Describe("Smart Playlists with Tag Criteria", func() {
 		var mfRepo model.MediaFileRepository
 		var testPlaylistID string

scheduler/log_adapter.go

@@ -11,7 +11,7 @@ func (l *logger) Info(msg string, keysAndValues ...any) {
 		"Scheduler: " + msg,
 	}
 	args = append(args, keysAndValues...)
-	log.Debug(args...)
+	log.Trace(args...)
 }
 
 func (l *logger) Error(err error, msg string, keysAndValues ...any) {

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/SongDatagrid.jsx

@@ -78,7 +78,7 @@ const useStyles = makeStyles({
   },
 })
 
-const DiscSubtitleRow = forwardRef(
+export const DiscSubtitleRow = forwardRef(
   ({ record, onClick, colSpan, contextAlwaysVisible }, ref) => {
     const translate = useTranslate()
     const isDesktop = useMediaQuery((theme) => theme.breakpoints.up('md'))
@@ -153,13 +153,17 @@ const DiscSubtitleRow = forwardRef(
             {subtitle}
           </Typography>
           {isLightboxOpen && !imageError && (
-            <Lightbox
-              imagePadding={50}
-              animationDuration={200}
-              imageTitle={record.album + ' - ' + subtitle}
-              mainSrc={fullImageUrl}
-              onCloseRequest={handleCloseLightbox}
-            />
+            // Lightbox portals out of the row, but React still bubbles its
+            // events up this tree, where the row's onClick would play the disc.
+            <span onClick={(e) => e.stopPropagation()}>
+              <Lightbox
+                imagePadding={50}
+                animationDuration={200}
+                imageTitle={record.album + ' - ' + subtitle}
+                mainSrc={fullImageUrl}
+                onCloseRequest={handleCloseLightbox}
+              />
+            </span>
           )}
         </TableCell>
         <TableCell>

ui/src/common/SongDatagrid.test.jsx

@@ -0,0 +1,74 @@
+import React from 'react'
+import { render, fireEvent, screen } from '@testing-library/react'
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { createTheme, ThemeProvider } from '@material-ui/core/styles'
+import { DiscSubtitleRow } from './SongDatagrid'
+
+vi.mock('../subsonic', () => ({
+  default: { getDiscCoverArtUrl: () => 'http://localhost/cover.jpg' },
+}))
+
+vi.mock('react-redux', () => ({ useDispatch: () => vi.fn() }))
+
+vi.mock('../common', () => ({ AlbumContextMenu: () => null }))
+
+vi.mock('react-dnd', () => ({ useDrag: () => [{}, vi.fn()] }))
+
+const record = {
+  id: 'song-1',
+  albumId: 'album-1',
+  album: 'The Album',
+  discNumber: 2,
+  discSubtitle: 'Bonus Disc',
+  updatedAt: '2024-01-01',
+}
+
+const renderRow = (onClick) =>
+  render(
+    <ThemeProvider theme={createTheme()}>
+      <table>
+        <tbody>
+          <DiscSubtitleRow record={record} onClick={onClick} colSpan={3} />
+        </tbody>
+      </table>
+    </ThemeProvider>,
+  )
+
+const openLightbox = () => {
+  fireEvent.click(document.querySelector('img'))
+  expect(document.querySelector('.ril__closeButton')).toBeTruthy()
+}
+
+describe('DiscSubtitleRow', () => {
+  beforeEach(() => vi.clearAllMocks())
+
+  it('plays the disc when the row is clicked', () => {
+    const onClick = vi.fn()
+    renderRow(onClick)
+    fireEvent.click(screen.getByText('Bonus Disc'))
+    expect(onClick).toHaveBeenCalledWith(2)
+  })
+
+  it('does not play the disc when opening the lightbox', () => {
+    const onClick = vi.fn()
+    renderRow(onClick)
+    openLightbox()
+    expect(onClick).not.toHaveBeenCalled()
+  })
+
+  it('does not play the disc when closing the lightbox', () => {
+    const onClick = vi.fn()
+    renderRow(onClick)
+    openLightbox()
+    fireEvent.click(document.querySelector('.ril__closeButton'))
+    expect(onClick).not.toHaveBeenCalled()
+  })
+
+  it('does not play the disc when clicking the lightbox backdrop', () => {
+    const onClick = vi.fn()
+    renderRow(onClick)
+    openLightbox()
+    fireEvent.click(document.querySelector('.ril__inner'))
+    expect(onClick).not.toHaveBeenCalled()
+  })
+})

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