diff --git a/frontend/src/pages/PersonImages/PersonImages.tsx b/frontend/src/pages/PersonImages/PersonImages.tsx index ea646b8ad..4484f9be0 100644 --- a/frontend/src/pages/PersonImages/PersonImages.tsx +++ b/frontend/src/pages/PersonImages/PersonImages.tsx @@ -22,6 +22,11 @@ export const PersonImages = () => { const images = useSelector(selectImages); const [clusterName, setClusterName] = useState('random_name'); const [isEditing, setIsEditing] = useState(false); + // Which cluster the shared `images` slice currently holds. Keyed on clusterId + // (not `isLoading`) so it also covers person A -> person B when B is cached. + const [loadedClusterId, setLoadedClusterId] = useState( + undefined, + ); const { data, isLoading, isSuccess, isError } = usePictoQuery({ queryKey: ['person-images', clusterId], queryFn: async () => fetchClusterImages({ clusterId: clusterId || '' }), @@ -42,9 +47,18 @@ export const PersonImages = () => { const images = (res?.images || []) as Image[]; dispatch(setImages(images)); setClusterName(res?.cluster_name || 'random_name'); + setLoadedClusterId(clusterId); dispatch(hideLoader()); } - }, [data, isSuccess, isError, isLoading, dispatch]); + }, [data, isSuccess, isError, isLoading, dispatch, clusterId]); + + // Until the slice is synced for this cluster it still holds the previous + // page's images (issue #1315). Fall back to the cluster-scoped query data for + // that window; read the slice once synced so favourite toggles still work. + const personImages = + clusterId !== undefined && loadedClusterId === clusterId + ? images + : ((data?.data as { images?: Image[] })?.images ?? []); const handleEditName = () => { setClusterName(clusterName); @@ -109,7 +123,7 @@ export const PersonImages = () => {

{clusterName}

- {images.map((image, index) => ( + {personImages.map((image, index) => ( {
{/* Media Viewer Modal */} - {isImageViewOpen && } + {isImageViewOpen && } ); }; diff --git a/frontend/src/pages/__tests__/PersonImages.test.tsx b/frontend/src/pages/__tests__/PersonImages.test.tsx new file mode 100644 index 000000000..f1370a639 --- /dev/null +++ b/frontend/src/pages/__tests__/PersonImages.test.tsx @@ -0,0 +1,102 @@ +import { render, screen, waitFor } from '@/test-utils'; +import { Routes, Route } from 'react-router'; +import { PersonImages } from '@/pages/PersonImages/PersonImages'; +import type { Image } from '@/types/Media'; +import * as apiFunctions from '@/api/api-functions'; + +// ImageCard resolves thumbnails through Tauri's convertFileSrc; return the path +// unchanged so we can assert which person's images are in the DOM by their src. +jest.mock('@tauri-apps/api/core', () => ({ + invoke: jest.fn().mockResolvedValue(null), + convertFileSrc: (path: string) => path, +})); + +jest.mock('@/api/api-functions', () => ({ + fetchClusterImages: jest.fn(), + renameCluster: jest.fn(), +})); + +const fetchClusterImages = apiFunctions.fetchClusterImages as jest.Mock; + +const makeImage = (id: string, path: string): Image => ({ + id, + path, + thumbnailPath: path, + folder_id: 'folder', + isTagged: true, + isFavourite: false, + tags: [], +}); + +// "Person A" stands in for whatever the previous page (e.g. the Home gallery) +// left in the shared `images` slice; "Person B" is the cluster we navigate to. +const personAImages = [ + makeImage('a1', '/personA/1.jpg'), + makeImage('a2', '/personA/2.jpg'), +]; +const personBImages = [ + makeImage('b1', '/personB/1.jpg'), + makeImage('b2', '/personB/2.jpg'), +]; + +const renderPerson = (clusterId: string) => + render( + + } /> + , + { + // Pre-seed the shared slice with the previous page's images. This is the + // exact condition that produced the flash described in issue #1315. + preloadedState: { + images: { images: personAImages, currentViewIndex: -1 }, + }, + initialRoutes: [`/person/${clusterId}`], + }, + ); + +const imageSrcs = (container: HTMLElement) => + Array.from(container.querySelectorAll('img')).map((img) => + img.getAttribute('src'), + ); + +const hasPersonA = (container: HTMLElement) => + imageSrcs(container).some((src) => src?.startsWith('/personA/')); +const hasPersonB = (container: HTMLElement) => + imageSrcs(container).some((src) => src?.startsWith('/personB/')); + +beforeEach(() => { + fetchClusterImages.mockReset(); + fetchClusterImages.mockImplementation(async ({ clusterId }) => ({ + success: true, + data: { + images: clusterId === 'B' ? personBImages : personAImages, + cluster_name: clusterId === 'B' ? 'Bob' : 'Alice', + }, + })); +}); + +describe('PersonImages (issue #1315: no flash of the previous page)', () => { + test('never paints the stale slice images, on the first frame or after', async () => { + const { container } = renderPerson('B'); + + // First frame: before the cluster query resolves, the shared slice still + // holds Person A's images. The grid must not paint them, otherwise the user + // sees the flash. (This assertion fails against the pre-fix code.) + expect(hasPersonA(container)).toBe(false); + + // Let the query resolve, then confirm Person A never appears at any point. + await waitFor(() => { + expect(hasPersonB(container)).toBe(true); + }); + expect(hasPersonA(container)).toBe(false); + }); + + test("renders the navigated cluster's images and name once loaded", async () => { + const { container } = renderPerson('B'); + + expect(await screen.findByText('Bob')).toBeInTheDocument(); + await waitFor(() => { + expect(hasPersonB(container)).toBe(true); + }); + }); +});