diff --git a/ui/src/album/AlbumGridView.jsx b/ui/src/album/AlbumGridView.jsx index 345d0fa15..983089416 100644 --- a/ui/src/album/AlbumGridView.jsx +++ b/ui/src/album/AlbumGridView.jsx @@ -11,6 +11,7 @@ import withWidth from '@material-ui/core/withWidth' import { Link } from 'react-router-dom' import { linkToRecord, useListContext, Loading } from 'react-admin' import { withContentRect } from 'react-measure' +import { useRollChanged } from './useRollChanged' import { useDrag } from 'react-dnd' import { AlbumContextMenu, @@ -230,9 +231,11 @@ const LoadedAlbumGrid = ({ ids, data, basePath, width }) => { ) } -const AlbumGridView = ({ albumListType, loaded, loading, ...props }) => { - const hide = - (loading && albumListType === 'random') || !props.data || !props.ids +const AlbumGridView = ({ albumListType, loaded, loading, seed, ...props }) => { + // A re-roll replaces every album, so the previous roll must not linger while it loads. Blanking + // on any load instead collapsed the grid to a spinner on each search keystroke. + const rerolling = useRollChanged(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 5108bfaa1..c346436f1 100644 --- a/ui/src/album/AlbumList.jsx +++ b/ui/src/album/AlbumList.jsx @@ -28,6 +28,7 @@ import { import AlbumListActions from './AlbumListActions' import AlbumTableView from './AlbumTableView' import AlbumGridView from './AlbumGridView' +import { useRollChanged } from './useRollChanged' import albumLists from './albumLists' import { getStoredDefaultView, @@ -177,9 +178,10 @@ const AlbumListTitle = ({ albumListType }) => { return } -const AlbumListPagination = ({ albumListType, ...rest }) => { +const AlbumListPagination = ({ albumListType, seed, ...rest }) => { const { loading } = useListContext() - if (loading && albumListType === 'random') { + const rerolling = useRollChanged(seed, loading) + if (rerolling && albumListType === 'random') { return null } return <Pagination {...rest} /> @@ -251,12 +253,13 @@ const AlbumList = (props) => { <AlbumListPagination rowsPerPageOptions={perPageOptions} albumListType={albumListType} + seed={seed} /> } title={<AlbumListTitle albumListType={albumListType} />} > {albumView.grid ? ( - <AlbumGridView albumListType={albumListType} {...props} /> + <AlbumGridView albumListType={albumListType} seed={seed} {...props} /> ) : ( <AlbumTableView {...props} /> )} diff --git a/ui/src/album/useRollChanged.jsx b/ui/src/album/useRollChanged.jsx new file mode 100644 index 000000000..43d0e4a4a --- /dev/null +++ b/ui/src/album/useRollChanged.jsx @@ -0,0 +1,16 @@ +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) + const wasLoading = useRef(loading) + + if (!loading && (shown.current === null || wasLoading.current)) { + shown.current = seed + } + wasLoading.current = loading + return shown.current !== seed +} diff --git a/ui/src/album/useRollChanged.test.jsx b/ui/src/album/useRollChanged.test.jsx new file mode 100644 index 000000000..b7920a331 --- /dev/null +++ b/ui/src/album/useRollChanged.test.jsx @@ -0,0 +1,53 @@ +import { renderHook } from '@testing-library/react-hooks' +import { describe, it, expect } from 'vitest' +import { useRollChanged } from './useRollChanged' + +describe('useRollChanged', () => { + const setup = (props) => + renderHook(({ seed, loading }) => useRollChanged(seed, loading), { + initialProps: props, + }) + + // A re-roll redirects and remounts the grid, so "mounted with a load in flight" is exactly the + // case where the store still holds the previous roll. Nothing is settled until that load lands. + 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) + }) + + it('reports no change when mounted with data already in hand', () => { + const { result } = setup({ seed: 's1', loading: false }) + expect(result.current).toBe(false) + }) + + // Typing in the search box refetches with the same seed: the albums on screen still belong to + // the roll being loaded, so they must stay put. + it('stays false while loading a filter change on the same roll', () => { + const { result, rerender } = setup({ seed: 's1', loading: false }) + rerender({ seed: 's1', loading: true }) + expect(result.current).toBe(false) + }) + + // A new seed is a new roll, so what is on screen is about to be replaced wholesale. + it('goes true while loading after the seed changes', () => { + const { result, rerender } = setup({ seed: 's1', loading: false }) + rerender({ seed: 's2', loading: true }) + expect(result.current).toBe(true) + }) + + it('clears once the new roll has loaded', () => { + const { result, rerender } = setup({ seed: 's1', loading: false }) + rerender({ seed: 's2', loading: true }) + expect(result.current).toBe(true) + rerender({ seed: 's2', loading: false }) + expect(result.current).toBe(false) + }) + + // The seed can land a render before loading flips, which would otherwise record the new roll as + // already shown and skip the blank entirely. + it('still reports a change when the seed arrives before loading starts', () => { + const { result, rerender } = setup({ seed: 's1', loading: false }) + rerender({ seed: 's2', loading: false }) + expect(result.current).toBe(true) + }) +})