From e2ee9b691c533a03483cdf2c5b2e808ab5493aec Mon Sep 17 00:00:00 2001 From: dtoro Date: Wed, 8 Apr 2026 21:52:42 +0200 Subject: [PATCH] fix: pass visible sequence to openPreview from the click site Previously the visible photo order was published only via a passive useEffect on Timeline, which had a timing race: arrow nav in preview could read a stale or empty sequence and fall back to the raw API order, breaking visual order navigation in tag mode and after filter changes. Fix: openPreview now accepts an optional visibleSequence parameter, and Timeline's onDoubleClick passes the freshly-computed flat sequence directly. The store action adopts that sequence as the authoritative visiblePhotoIds for the preview session, falling back to the most-recently-published one for paths that don't have a click site (e.g. the global Space hotkey). The Timeline still publishes via useEffect for the Space-hotkey fallback path, but the click path no longer depends on it. Co-Authored-By: Claude Opus 4.6 (1M context) --- frontend/src/components/timeline/Timeline.tsx | 25 ++++++++++++------- frontend/src/store/photoStore.ts | 20 +++++++++++++-- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/frontend/src/components/timeline/Timeline.tsx b/frontend/src/components/timeline/Timeline.tsx index e41de9c..5847d72 100644 --- a/frontend/src/components/timeline/Timeline.tsx +++ b/frontend/src/components/timeline/Timeline.tsx @@ -297,20 +297,27 @@ export function Timeline() { [items] ) - // Publish the flat visible-order id sequence to the photo store so - // PreviewView arrow nav (and the filmstrip) walks the same order the - // user sees in the grid. Includes duplicates from tag grouping — - // landing on the same photo's "second" appearance in the next tag - // bucket is the right behavior in tag mode. - useEffect(() => { + // Flat visible-order id sequence — exactly the order the user reads + // off the grid (top-to-bottom, left-to-right within each row). + // Includes duplicates from tag-grouping; landing on the same photo's + // "second" appearance in the next tag bucket is the right behavior + // in tag mode. + const visibleSequence = useMemo(() => { const ids: string[] = [] for (const row of photoRows) { for (const cell of row.cells) { ids.push(cell.photo.id) } } - setVisiblePhotoIds(ids) - }, [photoRows, setVisiblePhotoIds]) + return ids + }, [photoRows]) + + // Publish to the photo store so PreviewView's arrow nav and filmstrip + // can walk the same order even when opened from a non-click path + // (e.g. the global Space hotkey). + useEffect(() => { + setVisiblePhotoIds(visibleSequence) + }, [visibleSequence, setVisiblePhotoIds]) // Locate the active photo in the visual grid. Returns the FIRST // (rowIndex, colIndex) where its id appears, since a tag-grouped view @@ -518,7 +525,7 @@ export function Timeline() { selectPhoto(photo.id, globalIndex) } }} - onDoubleClick={() => openPreview(photo.id)} + onDoubleClick={() => openPreview(photo.id, visibleSequence)} /> ))} diff --git a/frontend/src/store/photoStore.ts b/frontend/src/store/photoStore.ts index f524184..1b0bd99 100644 --- a/frontend/src/store/photoStore.ts +++ b/frontend/src/store/photoStore.ts @@ -26,7 +26,11 @@ interface PhotoStore { setActivePhoto: (id: string | null) => void setViewMode: (mode: ViewMode) => void setVisiblePhotoIds: (ids: string[]) => void - openPreview: (id: string) => void + /** Open preview on a specific photo. The visibleSequence (optional) + * is the ordered list of photo ids the user currently sees in the + * timeline; passing it from the click site avoids a race where the + * passive Timeline publisher hasn't updated yet. */ + openPreview: (id: string, visibleSequence?: string[]) => void closePreview: () => void } @@ -99,7 +103,19 @@ export const usePhotoStore = create((set) => ({ return { visiblePhotoIds } }), - openPreview: (id) => set({ viewMode: 'preview', activePhotoId: id }), + openPreview: (id, visibleSequence) => + set((s) => ({ + viewMode: 'preview', + activePhotoId: id, + // Adopt the caller-provided sequence when they pass one. Falls + // back to whatever Timeline most recently published, which is + // correct for paths like the global Space hotkey that don't have + // a click site to compute the sequence from. + visiblePhotoIds: + visibleSequence && visibleSequence.length > 0 + ? visibleSequence + : s.visiblePhotoIds, + })), closePreview: () => set({ viewMode: 'grid' }), })) \ No newline at end of file