Repository navigation
feat(trip): add full-photo viewer with keyboard navigation - #34
Conversation
- fall back to the thumbnail when the display image fails - show a readable placeholder when no image can be loaded - reset image fallback state when navigating between photos - add regression coverage for failed image loading - update acceptance coverage documentation for the photo viewer
- show arrow-key and Escape shortcuts beneath viewer navigation - document keyboard, mouse, and focus behavior in README - verify shortcut hint visibility in the existing Playwright test
kctong529
left a comment
There was a problem hiding this comment.
Review — Approved
Thanks for implementing the full-photo viewer! This addresses frontend #24 and allows users to inspect their photos directly from the Trip gallery without entering the location-correction workflow.
The feature is appropriately scoped for our project plan and remains independent of backend photo retrieval, allowing us to develop and test the viewing experience while the upload and persistence workflows are still being implemented.
Implementation and design
The viewer uses the native HTML <dialog> element for modal behavior, with Previous/Next buttons, arrow-key navigation, Escape and backdrop dismissal, and focus restoration when closing. These interactions work consistently across both Gallery and Journey views.
The distinction between viewing and correcting a photo's location is particularly useful: selecting a thumbnail now opens a larger image, while location correction remains an explicit action. The implementation also handles missing metadata and unavailable images without interrupting navigation.
Manual tests performed
| Area | Verification | Result |
|---|---|---|
| Gallery viewing | Opened the Rome trip and selected a thumbnail; verified the displayed image and caption | Passed |
| Photo navigation | Navigated using Previous/Next buttons and arrow keys; confirmed navigation stops at the first and last photos | Passed |
| Gallery and Journey ordering | Verified that navigation follows the displayed ordering in both views | Passed |
| Keyboard accessibility | Opened the viewer using the keyboard, navigated controls with Tab, activated buttons with Enter/Space, and dismissed with Escape | Passed |
| Focus behavior | Confirmed that navigation controls retain focus appropriately; automated tests verify focus restoration after closing | Passed |
| Dismissal | Verified the Close button, Escape, and backdrop click | Passed |
| Missing metadata | Confirmed readable captions when location or capture time is unavailable | Passed |
| Responsive layout | Checked phone and tablet layouts using browser emulation; images and controls remained usable | Passed |
Improvements and automated verification
During manual testing, I also investigated how the viewer behaves when an image fails to load. With a valid thumbnail but an invalid display-image URL, the original implementation showed a broken image rather than using the available thumbnail.
I incorporated a few follow-up improvements to address this and improve usability:
- Image-loading resilience: Added a fallback to the thumbnail when the display image fails, a readable placeholder when neither image loads, and state reset when navigating between photos. I manually verified that the thumbnail fallback works.
- Regression coverage: Added an end-to-end test for failed image loading, verifying the unavailable-image state and continued navigation.
- Keyboard discoverability: Added an in-app hint for arrow-key navigation and Escape, documented the keyboard and mouse interactions in the README, and added a test assertion for the hint.
- Test documentation: Updated
tests/docs/acceptance-coverage.mdto reflect the new photo-viewer coverage.
Final verification:
| Check | Result |
|---|---|
| Lint | Passed — 30 warnings, 0 errors |
| Production build | Passed |
| TypeScript typecheck | Passed |
| Photo-viewer tests across desktop, tablet, and phone | 9/9 passed before the keyboard-hint update |
| Full Playwright acceptance suite | 230 passed, 2 skipped before the keyboard-hint update |
| Manual image-fallback verification | Passed |
The two skipped acceptance tests concern upload-size boundaries and are unrelated to this PR.
Scope and remaining work
The implementation meets the intended scope of frontend #24. It provides photo viewing and navigation using the existing gallery data without introducing backend image retrieval or additional photo-management functionality.
A few considerations remain for future integration:
- Image delivery and caching: Distinguish lightweight thumbnails from higher-resolution display images and originals. Thumbnails could benefit from browser caching for faster gallery navigation, while larger images would be loaded on demand with appropriate access controls.
- Dynamic photo collections: The viewer currently tracks the selected photo by array index. When photos are loaded from the backend, we should handle cases where the collection changes while the viewer is open—for example, if a photo is deleted, reordered, or removed from the current filter. Tracking the selected photo by ID and defining what happens when it disappears would make navigation more robust.
- Storage integration: Image rendition generation, signed-URL expiration and renewal, and appropriate caching policies should be addressed when connecting the viewer to real photo storage.
These are follow-up considerations rather than blockers for this PR, since the current implementation operates on a stable collection of mock photos.
Approved. The core viewing functionality has been verified, the image-loading issue identified during review has been addressed, and the changes remain appropriately focused on the photo-viewing experience.
Adds a full-photo viewer to the Trip gallery: opens from a thumbnail, previous/next with buttons and arrow keys, closes with Escape or the close button, and returns focus to the thumbnail. Covered by two new e2e tests (desktop, tablet, phone).