diff --git a/ui/src/album/AlbumGridView.jsx b/ui/src/album/AlbumGridView.jsx index e17fb617e..bd10f62dd 100644 --- a/ui/src/album/AlbumGridView.jsx +++ b/ui/src/album/AlbumGridView.jsx @@ -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 ? : } diff --git a/ui/src/album/AlbumList.jsx b/ui/src/album/AlbumList.jsx index c346436f1..9e4f94e1d 100644 --- a/ui/src/album/AlbumList.jsx +++ b/ui/src/album/AlbumList.jsx @@ -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 } -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} /> )} diff --git a/ui/src/album/useRollChanged.jsx b/ui/src/album/useRollChanged.jsx index 43d0e4a4a..b364bdc6b 100644 --- a/ui/src/album/useRollChanged.jsx +++ b/ui/src/album/useRollChanged.jsx @@ -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)) { diff --git a/ui/src/album/useRollChanged.test.jsx b/ui/src/album/useRollChanged.test.jsx index d0074a077..0745efb08 100644 --- a/ui/src/album/useRollChanged.test.jsx +++ b/ui/src/album/useRollChanged.test.jsx @@ -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) + }) })