Skip to content

BUG: PhotoViewer in DiaryEntryDetailPage missing onPhotoAnnotated callback — View Original toggle never appears after save #1482

@steilerDev

Description

@steilerDev

BUG: PhotoViewer missing onPhotoAnnotated — View Original / Clear Annotations invisible after annotation save

Severity: Major
Component: Frontend UI — DiaryEntryDetailPage / PhotoViewer
Found in: E2E test authoring for Story #1473 (Photo Annotator Foundation)

Steps to Reproduce

  1. Navigate to a diary entry detail page that has at least one photo attached
  2. Click the photo card to open the PhotoViewer
  3. Click the Annotate (pencil) button
  4. Draw a rectangle shape on the SVG overlay
  5. Click Save — observe the PUT /api/photos/:id/annotation returns 200 with updated annotatedAt

Expected Behavior

After step 5, the viewer returns to view mode with:

  • The "View Original" eye-toggle button visible in the info bar (because annotatedAt is now set)
  • The "Clear Annotations" trash button visible in the info bar

Actual Behavior

Neither button appears after save. The info bar shows only the Annotate button and the counter.

Root Cause

DiaryEntryDetailPage renders PhotoViewer without wiring the optional onPhotoAnnotated callback:

// client/src/pages/DiaryEntryDetailPage/DiaryEntryDetailPage.tsx (line 286-290)
<PhotoViewer
  photos={photosResult.photos}
  initialIndex={selectedPhotoIndex}
  onClose={() => setSelectedPhotoIndex(null)}
  // onPhotoAnnotated is MISSING
/>

PhotoViewer.handleAnnotationSave calls onPhotoAnnotated?.(updatedPhoto) (optional chaining), so it silently does nothing. The photos prop stays stale (annotatedAt=null), and since currentPhoto = photos[currentIndex], the currentPhoto.annotatedAt remains null after save, causing the conditional {currentPhoto.annotatedAt && ...} blocks to stay hidden.

Fix

Wire up onPhotoAnnotated in DiaryEntryDetailPage to refresh the photos result when a photo is annotated or cleared:

<PhotoViewer
  photos={photosResult.photos}
  initialIndex={selectedPhotoIndex}
  onClose={() => setSelectedPhotoIndex(null)}
  onPhotoAnnotated={(updatedPhoto) => {
    photosResult.updatePhoto(updatedPhoto);
    // OR: photosResult.refresh();
  }}
/>

This requires usePhotos to expose either an updatePhoto(photo) method (optimistic update) or a refresh() method (re-fetch). An optimistic update is preferred for snappy UX.

Environment

Evidence

  • PhotoViewer.tsx line 94-99: handleAnnotationSave calls onPhotoAnnotated?.(updatedPhoto) — optional, silently skipped
  • DiaryEntryDetailPage.tsx line 286-290: PhotoViewer rendered without onPhotoAnnotated prop
  • E2E workaround documented in e2e/tests/photoAnnotation.spec.ts (Scenario 1 comment)

Notes

The Clear Annotations flow (handleClearAnnotation) also calls onPhotoAnnotated?.(clearedPhoto) — same bug, same fix. Both view-original toggle and clear-annotations button are affected.

[e2e-test-engineer] Filed during E2E test authoring. The E2E smoke test (Scenario 1) works around this by mocking GET /api/photos to inject the updated annotatedAt, then re-navigating. Once Bug #1474 is fixed, the workaround should be removed from the test.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions