perf+ux: cut grid re-renders, coalesce discard, dedup bulk mutations

Frontend cleanup pass driven by the post-shadcn review. Performance - Memoize PhotoThumbnail and route cell click/double-click through stable handlers so heap-membership invalidation no longer re-renders every visible thumbnail. - Cap usePhotosQuery's eager background page-walk at 20 pages with a 50ms inter-page yield — was unbounded (up to 100k photos cold). - Drop the per-thumbnail loading spinner in favour of the existing pulse skeleton; only retry state still surfaces a spinner. UX - Coalesce rapid X/U presses into a single undo entry + one toast (1.2s window) so accidental bursts are easy to back out. - Optimistic rating/color updates with per-id snapshot rollback on error, matching the existing discard pattern. - Section-aware empty timeline state with a Clear-all-filters CTA. - Carry the search-match chip from the grid into the preview header. - Add a basket-icon badge for active heap membership so the green tint isn't the only signal (colorblind-safe). - Standardise error toasts via formatApiError(): FastAPI detail, validation arrays, axios message, with a 'Network Error' filter. Architecture - Extract useBulkPhotoMutations and stop duplicating bulkRating/bulkColor across RightSidebar and useKeyboardShortcuts. - Split RightSidebar (714 -> 448 LOC) and PhotoInfoPanel (952 -> 716) into co-located sub-components: BulkTakenAtEditor, BulkTagsEditor, TagsEditor, TakenAtEditor. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-15 10:13:39 +02:00
parent 7efac4354e
commit e65e798021
18 changed files with 992 additions and 646 deletions
--- a/frontend/src/hooks/useBulkPhotoMutations.ts
+++ b/frontend/src/hooks/useBulkPhotoMutations.ts
@@ -0,0 +1,117 @@
+import { useMutation, useQueryClient } from '@tanstack/react-query'
+import { photos as photosApi } from '../services/api'
+import { toast } from '../components/ToastContainer'
+import { formatApiError } from '../lib/apiError'
+import { LIBRARY_STATS_QUERY_KEY } from './useLibraryStatsQuery'
+import type { Photo } from '../types/photo'
+
+/**
+ * Centralised bulk-mutation hook used by both the RightSidebar multi-
+ * select panel and the keyboard shortcut layer. Before this existed the
+ * two paths each declared their own `bulkRatingMutation` /
+ * `bulkColorMutation` pair — identical signatures, slightly different
+ * optimistic behaviour, easy to drift.
+ *
+ * Each mutation:
+ *  - Applies an optimistic patch to every cached photo list AND the
+ *    per-photo cache so rating stars / color swatches flip instantly.
+ *  - Rolls back the patch on error and surfaces a toast.
+ *  - Invalidates the photo/library queries on success so server-side
+ *    derived fields (needs_review, date_warning, etc.) reconcile.
+ *
+ * Discard lives elsewhere — its two call sites have deliberately
+ * different semantics (keep-in-place for the X hotkey; strip-from-
+ * timeline for the sidebar bulk button) so they don't belong here.
+ */
+export function useBulkPhotoMutations() {
+  const queryClient = useQueryClient()
+
+  const invalidatePhotoQueries = () => {
+    queryClient.invalidateQueries({ queryKey: ['photo'] })
+    queryClient.invalidateQueries({ queryKey: ['photos'] })
+    queryClient.invalidateQueries({ queryKey: LIBRARY_STATS_QUERY_KEY })
+  }
+
+  // Snapshot enough of the current cache to roll back a failed mutation.
+  // We only track the photos actually being patched so the snapshot
+  // stays O(selection), not O(library).
+  const snapshotPhotos = (ids: string[]): Map<string, Partial<Photo>> => {
+    const snap = new Map<string, Partial<Photo>>()
+    const want = new Set(ids)
+    const lists = queryClient.getQueriesData<Photo[]>({ queryKey: ['photos'] })
+    for (const [, list] of lists) {
+      if (!list) continue
+      for (const p of list) if (want.has(p.id)) snap.set(p.id, { ...p })
+    }
+    for (const id of ids) {
+      if (snap.has(id)) continue
+      const single = queryClient.getQueryData<Photo>(['photo', id])
+      if (single) snap.set(id, { ...single })
+    }
+    return snap
+  }
+
+  // Apply a partial patch to every matching photo in every cached list,
+  // plus the per-photo cache. Used as the optimistic-update primitive.
+  const patchPhotos = (ids: string[], patch: Partial<Photo>) => {
+    const want = new Set(ids)
+    queryClient.setQueriesData<Photo[]>({ queryKey: ['photos'] }, (prev) =>
+      prev ? prev.map((p) => (want.has(p.id) ? { ...p, ...patch } : p)) : prev,
+    )
+    for (const id of ids) {
+      const cur = queryClient.getQueryData<Photo>(['photo', id])
+      if (cur) queryClient.setQueryData<Photo>(['photo', id], { ...cur, ...patch })
+    }
+  }
+
+  const restoreFromSnapshot = (snap: Map<string, Partial<Photo>>) => {
+    const ids = Array.from(snap.keys())
+    const want = new Set(ids)
+    queryClient.setQueriesData<Photo[]>({ queryKey: ['photos'] }, (prev) =>
+      prev
+        ? prev.map((p) =>
+            want.has(p.id) ? ({ ...p, ...snap.get(p.id) } as Photo) : p,
+          )
+        : prev,
+    )
+    for (const id of ids) {
+      const cur = queryClient.getQueryData<Photo>(['photo', id])
+      const orig = snap.get(id)
+      if (cur && orig) {
+        queryClient.setQueryData<Photo>(['photo', id], { ...cur, ...orig })
+      }
+    }
+  }
+
+  const bulkRating = useMutation({
+    mutationFn: ({ ids, rating }: { ids: string[]; rating: number }) =>
+      photosApi.bulkSetRating(ids, rating),
+    onMutate: ({ ids, rating }) => {
+      const snapshot = snapshotPhotos(ids)
+      patchPhotos(ids, { rating })
+      return { snapshot }
+    },
+    onError: (e, _vars, ctx) => {
+      if (ctx?.snapshot) restoreFromSnapshot(ctx.snapshot)
+      toast.error('Rating failed', formatApiError(e))
+    },
+    onSuccess: invalidatePhotoQueries,
+  })
+
+  const bulkColor = useMutation({
+    mutationFn: ({ ids, color }: { ids: string[]; color: string | null }) =>
+      photosApi.bulkSetColor(ids, color),
+    onMutate: ({ ids, color }) => {
+      const snapshot = snapshotPhotos(ids)
+      patchPhotos(ids, { color_label: color })
+      return { snapshot }
+    },
+    onError: (e, _vars, ctx) => {
+      if (ctx?.snapshot) restoreFromSnapshot(ctx.snapshot)
+      toast.error('Color label failed', formatApiError(e))
+    },
+    onSuccess: invalidatePhotoQueries,
+  })
+
+  return { bulkRating, bulkColor, invalidatePhotoQueries }
+}
--- a/frontend/src/hooks/useKeyboardShortcuts.ts
+++ b/frontend/src/hooks/useKeyboardShortcuts.ts
@@ -1,3 +1,4 @@
+import { useRef } from 'react'
 import { useHotkeys } from 'react-hotkeys-hook'
 import { useMutation, useQueryClient } from '@tanstack/react-query'
 import { usePhotoStore } from '../store/photoStore'
@@ -6,6 +7,8 @@ import { HEAPS_QUERY_KEY } from './useHeapsQuery'
 import { toast } from '../components/ToastContainer'
 import { registerUndoable, useUndoStore } from '../store/undoStore'
 import { LIBRARY_STATS_QUERY_KEY } from './useLibraryStatsQuery'
+import { useBulkPhotoMutations } from './useBulkPhotoMutations'
+import { formatApiError } from '../lib/apiError'
 import type { Photo } from '../types/photo'

 interface KeyboardShortcutsProps {
@@ -57,25 +60,10 @@ export function useKeyboardShortcuts(props: KeyboardShortcutsProps) {
    },
  })

-  const invalidatePhotoQueries = () => {
-    queryClient.invalidateQueries({ queryKey: ['photo'] })
-    queryClient.invalidateQueries({ queryKey: ['photos'] })
-    queryClient.invalidateQueries({ queryKey: LIBRARY_STATS_QUERY_KEY })
-  }
-
-  const bulkRatingMutation = useMutation({
-    mutationFn: ({ ids, rating }: { ids: string[]; rating: number }) =>
-      photosApi.bulkSetRating(ids, rating),
-    onSuccess: invalidatePhotoQueries,
-    onError: (e: any) => toast.error('Bulk rating failed', e.message || 'Unknown error'),
-  })
-
-  const bulkColorMutation = useMutation({
-    mutationFn: ({ ids, color }: { ids: string[]; color: string | null }) =>
-      photosApi.bulkSetColor(ids, color),
-    onSuccess: invalidatePhotoQueries,
-    onError: (e: any) => toast.error('Bulk color failed', e.message || 'Unknown error'),
-  })
+  const {
+    bulkRating: bulkRatingMutation,
+    bulkColor: bulkColorMutation,
+  } = useBulkPhotoMutations()

  /** Flip the cached photos to is_discarded=value in every list query
   *  without removing them. Lets the grid grey them in place instead of
@@ -123,7 +111,7 @@ export function useKeyboardShortcuts(props: KeyboardShortcutsProps) {
      markCachedDiscarded(ids, !discarded)
      toast.error(
        discarded ? 'Discard failed' : 'Restore failed',
-        e?.message || 'Unknown error'
+        formatApiError(e)
      )
    },
    onSuccess: (_data, { ids }) => {
@@ -134,6 +122,60 @@ export function useKeyboardShortcuts(props: KeyboardShortcutsProps) {
    },
  })

+  // Coalesced undo for rapid X (or U) presses. The mutation itself still
+  // fires per-press so the grayscale flip is instant; only the undo
+  // registration waits out COALESCE_MS so we emit one toast + one undo
+  // entry for a burst rather than N stacked ones.
+  const discardBatchRef = useRef<{
+    discarded: boolean
+    ids: Set<string>
+    timer: number | null
+  } | null>(null)
+  const DISCARD_COALESCE_MS = 1200
+
+  const flushDiscardBatch = () => {
+    const batch = discardBatchRef.current
+    if (!batch) return
+    discardBatchRef.current = null
+    const list = Array.from(batch.ids)
+    const discarded = batch.discarded
+    const verb = discarded ? 'Discarded' : 'Restored'
+    registerUndoable(
+      `${verb} ${list.length} photo${list.length === 1 ? '' : 's'}`,
+      async () => {
+        markCachedDiscarded(list, !discarded)
+        await (discarded
+          ? photosApi.bulkRestore(list)
+          : photosApi.bulkDiscard(list))
+        queryClient.invalidateQueries({ queryKey: LIBRARY_STATS_QUERY_KEY })
+        list.forEach((id) =>
+          queryClient.invalidateQueries({ queryKey: ['photo', id] })
+        )
+      }
+    )
+  }
+
+  const queueDiscardUndo = (ids: string[], discarded: boolean) => {
+    const batch = discardBatchRef.current
+    if (batch && batch.discarded === discarded) {
+      if (batch.timer !== null) window.clearTimeout(batch.timer)
+      ids.forEach((id) => batch.ids.add(id))
+      batch.timer = window.setTimeout(flushDiscardBatch, DISCARD_COALESCE_MS)
+      return
+    }
+    // Flush any in-flight batch of the opposite direction first so the
+    // two actions stay independently undoable.
+    if (batch) {
+      if (batch.timer !== null) window.clearTimeout(batch.timer)
+      flushDiscardBatch()
+    }
+    discardBatchRef.current = {
+      discarded,
+      ids: new Set(ids),
+      timer: window.setTimeout(flushDiscardBatch, DISCARD_COALESCE_MS),
+    }
+  }
+
  /** The set of photo ids the next culling action should apply to.
   *  - Multi-selection → all selected photos
   *  - Single selection → that one photo
@@ -158,30 +200,15 @@ export function useKeyboardShortcuts(props: KeyboardShortcutsProps) {
    // tint via the dedicated mutation (no cache strip, no timeline
    // removal). They disappear on hard reload because the section
    // filter excludes them in the wrong direction.
+    //
+    // Rapid X presses coalesce into a single undo entry + one toast
+    // (see queueDiscardUndo below) so hammering the key doesn't stack
+    // five toasts and force five Cmd+Z to back out.
    if (data.is_discarded === true || data.is_discarded === false) {
      const discarded = data.is_discarded
      discardMutation.mutate(
        { ids, discarded },
-        {
-          onSuccess: () => {
-            const verb = discarded ? 'Discarded' : 'Restored'
-            registerUndoable(
-              `${verb} ${ids.length} photo${ids.length === 1 ? '' : 's'}`,
-              async () => {
-                markCachedDiscarded(ids, !discarded)
-                await (discarded
-                  ? photosApi.bulkRestore(ids)
-                  : photosApi.bulkDiscard(ids))
-                queryClient.invalidateQueries({
-                  queryKey: LIBRARY_STATS_QUERY_KEY,
-                })
-                ids.forEach((id) =>
-                  queryClient.invalidateQueries({ queryKey: ['photo', id] })
-                )
-              }
-            )
-          },
-        }
+        { onSuccess: () => queueDiscardUndo(ids, discarded) }
      )
      return
    }
@@ -244,7 +271,7 @@ export function useKeyboardShortcuts(props: KeyboardShortcutsProps) {
      if (ctx?.previous) {
        queryClient.setQueryData(['heap-photo-ids', _vars.heapId], ctx.previous)
      }
-      toast.error('Heap update failed', e.message || 'Unknown error')
+      toast.error('Heap update failed', formatApiError(e))
    },
    onSuccess: (data, vars) => {
      const heap = (queryClient.getQueryData<Heap[]>(HEAPS_QUERY_KEY) ?? []).find(
@@ -324,7 +351,7 @@ export function useKeyboardShortcuts(props: KeyboardShortcutsProps) {
        await entry.undo()
      } catch (e: any) {
        useUndoStore.getState().push({ label: entry.label, undo: entry.undo })
-        toast.error('Undo failed', e?.message || 'Unknown error')
+        toast.error('Undo failed', formatApiError(e))
      }
    },
    HK_OPTS
--- a/frontend/src/hooks/usePhotosQuery.ts
+++ b/frontend/src/hooks/usePhotosQuery.ts
@@ -87,12 +87,19 @@ export function usePhotosQuery() {
    queryFn: async ({ signal }) => {
      // Two-phase fetch using cursor-based (keyset) pagination.
      // Phase 1 returns the first page (resolves the useQuery promise
-      // so consumers exit loading state). Phase 2 chains cursors in
-      // the background — each response includes a `next_cursor` that
+      // so consumers exit loading state). Phase 2 chains a bounded
+      // background loop — each response includes a `next_cursor` that
      // seeks directly to the next slice via an indexed range scan,
      // O(1) regardless of depth (no OFFSET skipping).
+      //
+      // MAX_PAGES is intentionally modest: 20 × 500 = 10 000 photos
+      // covers almost every browsing session up-front without burning
+      // through a 100k library on cold load. If a user scrolls past
+      // that horizon we'll add an infinite-query trigger; for now the
+      // cap keeps cold-load memory / network pressure sane.
      const PER_PAGE = 500
-      const MAX_PAGES = 200
+      const MAX_PAGES = 20
+      const INTER_PAGE_DELAY_MS = 50

      const first = await fetchCursorPage(
        { per_page: PER_PAGE, ...filterParams },
@@ -119,6 +126,10 @@ export function usePhotosQuery() {
                (prev) => (prev ? [...prev, ...more] : more)
              )
              if (!nextCursor || more.length < PER_PAGE) return
+              // Yield a beat between pages so the main thread stays
+              // responsive (thumbnail decode, scroll handling) while
+              // we're back-filling in the background.
+              await new Promise((r) => setTimeout(r, INTER_PAGE_DELAY_MS))
            } catch {
              return
            }