mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(ui): keep the Random grid blank while a refresh re-rolls
Refresh bumps the list version, which changes the random seed and remounts the grid. useRollChanged tracked the seed on screen in a ref inside the grid, so the remount started it empty and adopted the new seed on the first render, while the refetch had not begun and the store still held the previous roll. The grid painted the old albums for the length of the request and swapped when the new roll arrived. Move the ref up to AlbumList, which a refresh does not remount, and pass it to the grid and the pagination. The seed on screen then survives the remount, so a refresh reads as a re-roll and blanks until the new roll lands. A search keystroke keeps the seed and still leaves the grid in place.
This commit is contained in:
parent
56a36e4da4
commit
0b629858f7
@ -231,9 +231,17 @@ const LoadedAlbumGrid = ({ ids, data, basePath, width }) => {
|
||||
)
|
||||
}
|
||||
|
||||
const AlbumGridView = ({ albumListType, loaded, loading, seed, ...props }) => {
|
||||
const AlbumGridView = ({
|
||||
albumListType,
|
||||
loaded,
|
||||
loading,
|
||||
seed,
|
||||
shownSeed,
|
||||
...props
|
||||
}) => {
|
||||
// A re-roll replaces every album, so the previous roll must not linger while it loads.
|
||||
const rerolling = useRollChanged(seed, loading) && albumListType === 'random'
|
||||
const rerolling =
|
||||
useRollChanged(shownSeed, seed, loading) && albumListType === 'random'
|
||||
const hide = rerolling || !props.data || !props.ids
|
||||
return hide ? <Loading /> : <LoadedAlbumGrid {...props} />
|
||||
}
|
||||
|
||||
@ -1,3 +1,4 @@
|
||||
import { useRef } from 'react'
|
||||
import { useSelector } from 'react-redux'
|
||||
import { Redirect, useLocation } from 'react-router-dom'
|
||||
import {
|
||||
@ -178,9 +179,9 @@ const AlbumListTitle = ({ albumListType }) => {
|
||||
return <Title subTitle={title} args={{ smart_count: 2 }} />
|
||||
}
|
||||
|
||||
const AlbumListPagination = ({ albumListType, seed, ...rest }) => {
|
||||
const AlbumListPagination = ({ albumListType, seed, shownSeed, ...rest }) => {
|
||||
const { loading } = useListContext()
|
||||
const rerolling = useRollChanged(seed, loading)
|
||||
const rerolling = useRollChanged(shownSeed, seed, loading)
|
||||
if (rerolling && albumListType === 'random') {
|
||||
return null
|
||||
}
|
||||
@ -191,6 +192,7 @@ const randomStartingSeed = Math.random().toString()
|
||||
|
||||
const AlbumList = (props) => {
|
||||
const { width } = props
|
||||
const shownSeed = useRef(null)
|
||||
const albumView = useSelector((state) => state.albumView)
|
||||
const [perPage, perPageOptions] = useAlbumsPerPage(width)
|
||||
const location = useLocation()
|
||||
@ -254,12 +256,18 @@ const AlbumList = (props) => {
|
||||
rowsPerPageOptions={perPageOptions}
|
||||
albumListType={albumListType}
|
||||
seed={seed}
|
||||
shownSeed={shownSeed}
|
||||
/>
|
||||
}
|
||||
title={<AlbumListTitle albumListType={albumListType} />}
|
||||
>
|
||||
{albumView.grid ? (
|
||||
<AlbumGridView albumListType={albumListType} seed={seed} {...props} />
|
||||
<AlbumGridView
|
||||
albumListType={albumListType}
|
||||
seed={seed}
|
||||
shownSeed={shownSeed}
|
||||
{...props}
|
||||
/>
|
||||
) : (
|
||||
<AlbumTableView {...props} />
|
||||
)}
|
||||
|
||||
@ -1,11 +1,8 @@
|
||||
import { useRef } from 'react'
|
||||
|
||||
// useRollChanged reports that the albums on screen belong to a different roll than the one being
|
||||
// loaded. Only a seed change is a re-roll; a search keystroke refetches the same roll.
|
||||
export const useRollChanged = (seed, loading) => {
|
||||
// Starts empty, not at the current seed: a re-roll redirects and remounts this component, so an
|
||||
// initial value of `seed` would look already-settled while the stale roll is still on screen.
|
||||
const shown = useRef(null)
|
||||
// Reports that the albums on screen belong to a different roll than the one loading: only a seed
|
||||
// change is a re-roll. `shown` is owned above the grid, which a refresh remounts under the new seed.
|
||||
export const useRollChanged = (shown, seed, loading) => {
|
||||
const wasLoading = useRef(loading)
|
||||
|
||||
if (!loading && (shown.current === null || wasLoading.current)) {
|
||||
|
||||
@ -3,13 +3,14 @@ import { describe, it, expect } from 'vitest'
|
||||
import { useRollChanged } from './useRollChanged'
|
||||
|
||||
describe('useRollChanged', () => {
|
||||
const setup = (props) =>
|
||||
renderHook(({ seed, loading }) => useRollChanged(seed, loading), {
|
||||
// Passing `shown` in mimics AlbumList owning it across a remount.
|
||||
const setup = (props, shown = { current: null }) => ({
|
||||
shown,
|
||||
...renderHook(({ seed, loading }) => useRollChanged(shown, seed, loading), {
|
||||
initialProps: props,
|
||||
})
|
||||
}),
|
||||
})
|
||||
|
||||
// A re-roll remounts the grid, so on mount a load in flight means the store still holds the
|
||||
// previous roll.
|
||||
it('reports a change while a load is in flight on a fresh mount', () => {
|
||||
const { result } = setup({ seed: 's1', loading: true })
|
||||
expect(result.current).toBe(true)
|
||||
@ -46,4 +47,27 @@ describe('useRollChanged', () => {
|
||||
rerender({ seed: 's2', loading: false })
|
||||
expect(result.current).toBe(true)
|
||||
})
|
||||
|
||||
// Refresh remounts the grid under the new seed a render before the refetch starts.
|
||||
it('reports a change when a refresh remounts the grid before loading starts', () => {
|
||||
const { shown, unmount } = setup({ seed: 's1', loading: false })
|
||||
expect(shown.current).toBe('s1')
|
||||
unmount()
|
||||
|
||||
const remounted = setup({ seed: 's2', loading: false }, shown)
|
||||
expect(remounted.result.current).toBe(true)
|
||||
|
||||
remounted.rerender({ seed: 's2', loading: true })
|
||||
expect(remounted.result.current).toBe(true)
|
||||
remounted.rerender({ seed: 's2', loading: false })
|
||||
expect(remounted.result.current).toBe(false)
|
||||
})
|
||||
|
||||
it('reports no change when a remount keeps the same roll', () => {
|
||||
const { shown, unmount } = setup({ seed: 's1', loading: false })
|
||||
unmount()
|
||||
|
||||
const remounted = setup({ seed: 's1', loading: false }, shown)
|
||||
expect(remounted.result.current).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user