From 20d155ba58b47c6dee9a67b0c860f8f7cd88ff73 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 26 Jul 2026 12:35:29 -0400 Subject: [PATCH] fix(ui): stop the Random album grid collapsing on every keystroke The grid was replaced by a spinner whenever the random list was loading, so each search keystroke collapsed it to spinner height and back. That blanket blanking is the flicker commit 9e559311a removed for every other list; random kept the exception so a re-roll would not flash the roll it is replacing. Blank on a seed change instead of on any load: a re-roll gets a new seed and still blanks, while a search keeps the seed and leaves the grid in place. The seed is tracked from empty rather than from the current value because a re-roll redirects and remounts the grid, which would otherwise look already-settled with the previous roll still on screen. Same rule for the pagination, which was hidden on the same condition. --- ui/src/album/AlbumGridView.jsx | 9 +++-- ui/src/album/AlbumList.jsx | 9 +++-- ui/src/album/useRollChanged.jsx | 16 +++++++++ ui/src/album/useRollChanged.test.jsx | 53 ++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 ui/src/album/useRollChanged.jsx create mode 100644 ui/src/album/useRollChanged.test.jsx 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) + }) +})