From 0b629858f7d24c7f93cf019b276a8cd31fa220c5 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 29 Jul 2026 16:25:23 -0400 Subject: [PATCH] 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. --- ui/src/album/AlbumGridView.jsx | 12 ++++++++-- ui/src/album/AlbumList.jsx | 14 +++++++++--- ui/src/album/useRollChanged.jsx | 9 +++----- ui/src/album/useRollChanged.test.jsx | 34 ++++++++++++++++++++++++---- 4 files changed, 53 insertions(+), 16 deletions(-) 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) + }) })