fix(person-images): prevent flash of previous page's images (AOSSIE-Org#1315) (AOSSIE-Org#1321)

* fix(person-images): prevent flash of previous page's images (AOSSIE-Org#1315)

Scope the grid to the active cluster so the shared `images` slice left by the
previous page can no longer paint before the person's photos load. Track which
cluster the slice holds via `loadedClusterId`; until it syncs, render from the
`['person-images', clusterId]` query data, which is always scoped to the active
id. Reading the slice once synced keeps the optimistic favourite toggle working.

Adds a regression test that fails when the previous page's images render.

* fix(person-images): guard undefined clusterId and drop the any cast

Addresses CodeRabbit review on the PR: only treat the slice as synced when
clusterId is defined, and type the query-data fallback explicitly instead of
casting through any.

* refactor: clean up verbose comments

* fix(person-images): prevent stale heading flash

* ui consistency

---------

Co-authored-by: ROHAN PANDEY <95585299+rohan-pandeyy@users.noreply.github.com>

Author: tanmaysachann

frontend/src/pages/PersonImages/PersonImages.tsx

@@ -22,6 +22,10 @@ export const PersonImages = () => {
   const images = useSelector(selectImages);
   const [clusterName, setClusterName] = useState<string>('random_name');
   const [isEditing, setIsEditing] = useState<boolean>(false);
+
+  const [loadedClusterId, setLoadedClusterId] = useState<string | undefined>(
+    undefined,
+  );
   const { data, isLoading, isSuccess, isError } = usePictoQuery({
     queryKey: ['person-images', clusterId],
     queryFn: async () => fetchClusterImages({ clusterId: clusterId || '' }),
@@ -42,9 +46,21 @@ 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]);
+
+  // Fallback to query data until Redux slice syncs for this cluster (#1315)
+  const personImages =
+    clusterId !== undefined && loadedClusterId === clusterId
+      ? images
+      : ((data?.data as { images?: Image[] })?.images ?? []);
+  const displayName =
+    clusterId !== undefined && loadedClusterId === clusterId
+      ? clusterName
+      : ((data?.data as { cluster_name?: string })?.cluster_name ??
+        'random_name');
 
   const handleEditName = () => {
     setClusterName(clusterName);
@@ -67,8 +83,8 @@ export const PersonImages = () => {
     }
   };
   return (
-    <div className="p-6">
-      <div className="mb-6 flex items-center justify-between">
+    <div>
+      <div className="my-6 flex items-center justify-between">
         <Button
           variant="outline"
           onClick={() => navigate(`/${ROUTES.AI}`)}
@@ -107,21 +123,22 @@ export const PersonImages = () => {
           </Button>
         )}
       </div>
-      <h1 className="mb-6 text-2xl font-bold">{clusterName}</h1>
-      <div className="grid grid-cols-1 gap-4 sm:grid-cols-2 md:grid-cols-3 lg:grid-cols-4 xl:grid-cols-5">
-        {images.map((image, index) => (
-          <ImageCard
-            key={image.id}
-            image={image}
-            imageIndex={index}
-            className="w-full"
-            onClick={() => dispatch(setCurrentViewIndex(index))}
-          />
+      <h1 className="mb-6 text-2xl font-bold">{displayName}</h1>
+      <div className="grid grid-cols-1 gap-4 pb-6 sm:grid-cols-2 md:grid-cols-3 lg:grid-cols-4 xl:grid-cols-5">
+        {personImages.map((image, index) => (
+          <div key={image.id} className="group relative">
+            <ImageCard
+              image={image}
+              imageIndex={index}
+              className="w-full transition-transform duration-200 group-hover:scale-105"
+              onClick={() => dispatch(setCurrentViewIndex(index))}
+            />
+          </div>
         ))}
       </div>
 
       {/* Media Viewer Modal */}
-      {isImageViewOpen && <MediaView images={images} />}
+      {isImageViewOpen && <MediaView images={personImages} />}
     </div>
   );
 };

frontend/src/pages/__tests__/PersonImages.test.tsx

@@ -0,0 +1,166 @@
+import { useLayoutEffect } from 'react';
+import { fireEvent, render, screen, waitFor } from '@/test-utils';
+import { Routes, Route, useNavigate } from 'react-router';
+import { PersonImages } from '@/pages/PersonImages/PersonImages';
+import type { Image } from '@/types/Media';
+import * as apiFunctions from '@/api/api-functions';
+
+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: [],
+});
+
+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 CommitProbe = ({
+  onCommit,
+}: {
+  onCommit?: (headingText: string | null) => void;
+}) => {
+  useLayoutEffect(() => {
+    onCommit?.(document.querySelector('h1')?.textContent ?? null);
+  });
+
+  return null;
+};
+
+const PersonImagesWithNavigation = ({
+  onCommit,
+}: {
+  onCommit?: (headingText: string | null) => void;
+}) => {
+  const navigate = useNavigate();
+
+  return (
+    <>
+      <button type="button" onClick={() => navigate('/person/B')}>
+        Go to Person B
+      </button>
+      <PersonImages />
+      <CommitProbe onCommit={onCommit} />
+    </>
+  );
+};
+
+const renderPerson = (
+  clusterId: string,
+  options: {
+    withNavigation?: boolean;
+    onCommit?: (headingText: string | null) => void;
+  } = {},
+) =>
+  render(
+    <Routes>
+      <Route
+        path="/person/:clusterId"
+        element={
+          options.withNavigation ? (
+            <PersonImagesWithNavigation onCommit={options.onCommit} />
+          ) : (
+            <PersonImages />
+          )
+        }
+      />
+    </Routes>,
+    {
+      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 stale images or heading when navigating to a cached person', async () => {
+    const committedHeadings: Array<string | null> = [];
+    const { container, queryClient } = renderPerson('A', {
+      withNavigation: true,
+      onCommit: (headingText) => committedHeadings.push(headingText),
+    });
+
+    await waitFor(() => {
+      expect(
+        screen.getByRole('heading', { name: 'Alice' }),
+      ).toBeInTheDocument();
+      expect(hasPersonA(container)).toBe(true);
+    });
+
+    queryClient.setQueryData(['person-images', 'B'], {
+      success: true,
+      data: {
+        images: personBImages,
+        cluster_name: 'Bob',
+      },
+    });
+
+    committedHeadings.length = 0;
+    fireEvent.click(screen.getByRole('button', { name: 'Go to Person B' }));
+
+    expect(committedHeadings).not.toContain('Alice');
+    expect(
+      screen.queryByRole('heading', { name: 'Alice' }),
+    ).not.toBeInTheDocument();
+    expect(screen.getByRole('heading', { name: 'Bob' })).toBeInTheDocument();
+    expect(hasPersonA(container)).toBe(false);
+    expect(hasPersonB(container)).toBe(true);
+  });
+
+  test("renders the navigated cluster's images and name once loaded", async () => {
+    const { container } = renderPerson('B');
+
+    expect(
+      screen.queryByRole('heading', { name: 'Alice' }),
+    ).not.toBeInTheDocument();
+    expect(hasPersonA(container)).toBe(false);
+
+    expect(await screen.findByText('Bob')).toBeInTheDocument();
+    await waitFor(() => {
+      expect(hasPersonB(container)).toBe(true);
+    });
+  });
+});