From 4cbe0e06735888076910d31383c25762452677bb Mon Sep 17 00:00:00 2001 From: Frioh Date: Wed, 21 Oct 2026 04:12:40 +0100 Subject: [PATCH] feat(gallery): extract reusable typed gallery Improve Gallery empty-state messaging, apply semantic theme tokens, and add component tests and usage documentation. Closes #1260 Closes #1261 Closes #1262 Closes #1263 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 + docs/gallery.md | 36 +++ src/components/gallery/Gallery.tsx | 78 +++++++ src/components/gallery/Gallery.types.ts | 18 ++ src/components/gallery/ImageLightbox.tsx | 211 ++++++++++++++++++ .../gallery/__tests__/Gallery.test.tsx | 80 +++++++ src/components/property/ImageGallery.tsx | 147 +----------- src/components/property/ImageLightbox.tsx | 208 +---------------- .../property/__tests__/ImageGallery.test.tsx | 2 + 9 files changed, 436 insertions(+), 346 deletions(-) create mode 100644 docs/gallery.md create mode 100644 src/components/gallery/Gallery.tsx create mode 100644 src/components/gallery/Gallery.types.ts create mode 100644 src/components/gallery/ImageLightbox.tsx create mode 100644 src/components/gallery/__tests__/Gallery.test.tsx diff --git a/README.md b/README.md index 8aa1b5e2..d3ecdcba 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,8 @@ NEXT_PUBLIC_SENTRY_DSN=your_sentry_dsn ### Project Documentation +- **[Gallery](./docs/gallery.md)** - Reusable image gallery, empty states, theme support and types + - **[🔧 Mocking](./docs/mocking.md)** - Local mocking and development overrides - **[🗄️ Cache API](./docs/cache-api.md)** - Caching model and invalidation rules - **[🎣 Hooks Guide](./docs/hooks.md)** - Custom hooks reference diff --git a/docs/gallery.md b/docs/gallery.md new file mode 100644 index 00000000..121cb52f --- /dev/null +++ b/docs/gallery.md @@ -0,0 +1,36 @@ +# Gallery + +`src/components/gallery/Gallery.tsx` provides the shared client-side Gallery +(issues #1260–#1263). Import `Gallery` and its exported `GalleryProps` type +from `@/components/gallery/Gallery`. + +```tsx + +``` + +`images` is a required readonly array of URL strings, in display order; use `[]` +when empty. `label` is a required accessible collection name and image description. +`emptyTitle` and `emptyDescription` are optional strings. The default empty state +explains that images will appear when added, has a decorative icon and a status +region, and offers no inactive controls. The property `ImageGallery` adapter keeps +the existing `images` / `propertyName` API and supplies property-specific copy. + +The main image, thumbnails and more-images control are native buttons with focus +rings. The more-images button opens the first image beyond the preview. Escape +closes the lightbox; arrow keys navigate it. Replacing the image collection resets +loading and lightbox state. Rendering errors stay inside the existing error boundary. + +Gallery surfaces, text, borders and focus rings use the application's semantic +Tailwind tokens (`background`, `foreground`, `muted`, `border`, `ring`, `accent`). +They follow the existing `.dark` theme without a separate toggle or theme state. +The lightbox uses a deliberately dark viewing surface in either theme. + +Validation: `npm test -- --runInBand ImageGallery Gallery`, `npm run typecheck`, +and `npm run lint`. The tests cover empty/populated states, keyboard interaction, +lightbox indices, reuse, theme-token integration and collection replacement. +The type assertions in `Gallery.types.ts` are checked by `npm run typecheck`. diff --git a/src/components/gallery/Gallery.tsx b/src/components/gallery/Gallery.tsx new file mode 100644 index 00000000..2a502926 --- /dev/null +++ b/src/components/gallery/Gallery.tsx @@ -0,0 +1,78 @@ +'use client'; + +import { useState } from 'react'; +import Image from 'next/image'; +import { ImageIcon } from 'lucide-react'; +import { ErrorBoundary } from '@/components/ui/SimpleErrorBoundary'; +import { ImageLightbox } from './ImageLightbox'; + +export interface GalleryProps { + /** Image URLs in display order; the first image is featured. */ + readonly images: readonly string[]; + /** Accessible name and image description for this collection. */ + readonly label: string; + readonly emptyTitle?: string; + readonly emptyDescription?: string; +} + +/** Reusable image grid with an accessible empty state and lightbox. */ +export function Gallery(props: GalleryProps) { + return ( + + + + ); +} + +function GalleryContent({ + images, + label, + emptyTitle = 'No images available', + emptyDescription = 'Images will appear here when they are added.', +}: GalleryProps) { + const [lightboxIndex, setLightboxIndex] = useState(null); + const [mainImageLoaded, setMainImageLoaded] = useState(false); + const mainImage = images[0]; + const focusClasses = 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 focus-visible:ring-offset-background'; + + if (mainImage === undefined) { + return ( +
+
+
+
+ ); + } + + return ( +
+ + {images.length > 1 && ( +
+ {images.slice(1, 5).map((image, index) => ( + + ))} + {images.length > 5 && ( + + )} +
+ )} + setLightboxIndex(null)} /> +
+ ); +} diff --git a/src/components/gallery/Gallery.types.ts b/src/components/gallery/Gallery.types.ts new file mode 100644 index 00000000..33de5a64 --- /dev/null +++ b/src/components/gallery/Gallery.types.ts @@ -0,0 +1,18 @@ +import type { GalleryProps } from './Gallery'; +import type { ImageGalleryProps } from '../property/ImageGallery'; + +// Compile-time regression checks included in the project's strict typecheck. +export function checkGalleryTypes(props: GalleryProps): void { + // @ts-expect-error Image collections must not be mutated by consumers. + props.images.push('/extra.jpg'); + // @ts-expect-error Image URLs must be strings. + const invalidImages: GalleryProps = { images: [123], label: 'Artwork' }; + // @ts-expect-error A collection must have an accessible label. + const missingLabel: GalleryProps = { images: [] }; + // @ts-expect-error Empty-state copy must be text. + const invalidCopy: GalleryProps = { images: [], label: 'Artwork', emptyTitle: 42 }; + const readonlyImages = ['/one.jpg'] as const; + const reusable: GalleryProps = { images: readonlyImages, label: 'Artwork' }; + const property: ImageGalleryProps = { images: readonlyImages, propertyName: 'Villa' }; + void [invalidImages, missingLabel, invalidCopy, reusable, property]; +} diff --git a/src/components/gallery/ImageLightbox.tsx b/src/components/gallery/ImageLightbox.tsx new file mode 100644 index 00000000..a5dec938 --- /dev/null +++ b/src/components/gallery/ImageLightbox.tsx @@ -0,0 +1,211 @@ +'use client'; + +import React, { useState, useEffect, useCallback, useRef } from 'react'; +import Image from 'next/image'; +import { X, ChevronLeft, ChevronRight } from 'lucide-react'; + +export interface ImageLightboxProps { + images: readonly string[]; + label?: string; + initialIndex?: number; + isOpen: boolean; + onClose: () => void; +} + +export const ImageLightbox: React.FC = ({ + images, + label, + initialIndex = 0, + isOpen, + onClose, +}) => { + const [currentIndex, setCurrentIndex] = useState(initialIndex); + const [touchStart, setTouchStart] = useState(null); + const [touchEnd, setTouchEnd] = useState(null); + const closeButtonRef = useRef(null); + const previouslyFocused = useRef(null); + + // Reset index when lightbox opens with new initialIndex + useEffect(() => { + if (isOpen) { + setCurrentIndex(initialIndex); + } + }, [initialIndex, isOpen]); + + // Issue #1252: the overlay was a plain div, so a screen reader had no way + // of knowing a dialog had opened, and focus stayed wherever it was behind + // the overlay. Focus now moves into the dialog and is handed back to the + // element that opened it. + useEffect(() => { + if (!isOpen) return; + + previouslyFocused.current = document.activeElement as HTMLElement | null; + closeButtonRef.current?.focus(); + + return () => { + previouslyFocused.current?.focus?.(); + }; + }, [isOpen]); + + // Keyboard navigation + useEffect(() => { + if (!isOpen) return; + + const handleKeyDown = (e: KeyboardEvent) => { + switch (e.key) { + case 'ArrowLeft': + navigatePrevious(); + break; + case 'ArrowRight': + navigateNext(); + break; + case 'Escape': + onClose(); + break; + } + }; + + document.addEventListener('keydown', handleKeyDown); + document.body.style.overflow = 'hidden'; + + return () => { + document.removeEventListener('keydown', handleKeyDown); + document.body.style.overflow = 'unset'; + }; + }, [isOpen, currentIndex]); + + // Touch handlers for mobile swipe + const handleTouchStart = useCallback((e: React.TouchEvent) => { + setTouchEnd(null); + setTouchStart(e.targetTouches[0].clientX); + }, []); + + const handleTouchMove = useCallback((e: React.TouchEvent) => { + setTouchEnd(e.targetTouches[0].clientX); + }, []); + + const handleTouchEnd = useCallback(() => { + if (!touchStart || !touchEnd) return; + + const distance = touchStart - touchEnd; + const isLeftSwipe = distance > 50; + const isRightSwipe = distance < -50; + + if (isLeftSwipe) { + navigateNext(); + } + if (isRightSwipe) { + navigatePrevious(); + } + }, [touchStart, touchEnd]); + + const navigateNext = () => { + setCurrentIndex((prev) => (prev + 1) % images.length); + }; + + const navigatePrevious = () => { + setCurrentIndex((prev) => (prev - 1 + images.length) % images.length); + }; + + const handleThumbnailClick = (index: number) => { + setCurrentIndex(index); + }; + + if (!isOpen || images.length === 0) return null; + + return ( + // The dialog role and aria-modal announce the overlay to assistive tech. + // the rest of the page is behind a modal, and the label names it. +
+ {/* Close Button */} + + + {/* Main Image Container */} +
+ {label + + {/* Navigation Buttons */} + {images.length > 1 && ( + <> + + + + )} + + {/* Image Counter */} + {/* Issue #1252: the position is announced when it + changes. */} +
+ {currentIndex + 1} / {images.length} +
+
+ + {/* Thumbnail Strip */} + {images.length > 1 && ( +
+ {images.map((image, index) => ( + + ))} +
+ )} +
+ ); +}; diff --git a/src/components/gallery/__tests__/Gallery.test.tsx b/src/components/gallery/__tests__/Gallery.test.tsx new file mode 100644 index 00000000..e5d405b9 --- /dev/null +++ b/src/components/gallery/__tests__/Gallery.test.tsx @@ -0,0 +1,80 @@ +import React from 'react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { Gallery } from '../Gallery'; + +jest.mock('next/image', () => ({ + __esModule: true, + default: ({ priority: _priority, ...props }: React.ImgHTMLAttributes & { priority?: boolean }) => , +})); + +const images = ['/one.jpg', '/two.jpg', '/three.jpg', '/four.jpg', '/five.jpg', '/six.jpg'] as const; + +describe('Gallery', () => { + it('explains the empty state without interactive image controls', () => { + render(); + expect(screen.getByRole('status')).toHaveTextContent('No images available'); + expect(screen.getByRole('status')).toHaveTextContent('Images will appear here when they are added.'); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + expect(screen.queryByRole('img')).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('supports custom empty copy outside the property domain', () => { + render(); + expect(screen.getByRole('heading', { name: 'No artwork yet' })).toBeInTheDocument(); + expect(screen.getByRole('status')).toHaveTextContent('Add your first artwork.'); + }); + + it('opens the main image with the keyboard and returns focus on close', async () => { + const user = userEvent.setup(); + render(); + await user.tab(); + const trigger = screen.getByRole('button', { name: 'View all 6 images of Exhibition' }); + expect(trigger).toHaveFocus(); + await user.keyboard('{Enter}'); + expect(screen.getByRole('dialog', { name: 'Exhibition' })).toBeInTheDocument(); + expect(screen.getByText('1 / 6')).toBeInTheDocument(); + await user.keyboard('{Escape}'); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(trigger).toHaveFocus(); + }); + + it('opens the first hidden image using the more control', () => { + render(); + fireEvent.click(screen.getByRole('button', { name: 'View 1 more images of Exhibition' })); + expect(screen.getByText('6 / 6')).toBeInTheDocument(); + }); + + it('omits thumbnails and more controls for a single image', () => { + render(); + expect(screen.getAllByRole('button')).toHaveLength(1); + }); + + it('resets the viewer and loading state when the collection changes', () => { + const { rerender, container } = render(); + fireEvent.load(screen.getByAltText('Exhibition')); + expect(container.querySelector('.animate-pulse')).toBeNull(); + fireEvent.click(screen.getByRole('button', { name: 'View 1 more images of Exhibition' })); + rerender(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(container.querySelector('.animate-pulse')).toBeInTheDocument(); + fireEvent.error(screen.getByAltText('Exhibition')); + expect(container.querySelector('.animate-pulse')).toBeNull(); + rerender(); + expect(screen.getByRole('status')).toBeInTheDocument(); + }); + + it.each(['light', 'dark'])('uses shared theme tokens in the %s theme', (theme) => { + document.documentElement.classList.toggle('dark', theme === 'dark'); + try { + const { rerender } = render(); + expect(screen.getByRole('region', { name: 'Exhibition' })).toHaveClass('bg-muted', 'text-foreground', 'border-border'); + rerender(); + expect(screen.getByText('View all 6 images')).toHaveClass('bg-background/95', 'text-foreground'); + expect(screen.getByRole('button', { name: 'View 1 more images of Exhibition' })).toHaveClass('bg-muted', 'text-foreground'); + } finally { + document.documentElement.classList.remove('dark'); + } + }); +}); diff --git a/src/components/property/ImageGallery.tsx b/src/components/property/ImageGallery.tsx index 54cbcf47..a9ff8ed7 100644 --- a/src/components/property/ImageGallery.tsx +++ b/src/components/property/ImageGallery.tsx @@ -1,144 +1,13 @@ 'use client'; -import React, { useState } from 'react'; -import Image from 'next/image'; -import { ImageLightbox } from './ImageLightbox'; -import { ErrorBoundary } from '@/components/ui/SimpleErrorBoundary'; +import { Gallery } from '@/components/gallery/Gallery'; -interface ImageGalleryProps { - images: string[]; - propertyName: string; +export interface ImageGalleryProps { + readonly images: readonly string[]; + readonly propertyName: string; } -/** - * Issue #1254 — the gallery renders the lightbox and a grid of thumbnails; a - * failure in either used to take the whole property page down with it. The - * boundary below keeps the failure inside the gallery, where it degrades to a - * retryable fallback. - */ -export const ImageGallery: React.FC = (props) => ( - - - -); - -const ImageGalleryContent: React.FC = ({ - images, - propertyName -}) => { - const [lightboxIndex, setLightboxIndex] = useState(null); - // Issue #1253 — the gallery had no loading state, so an image that was still - // being fetched left an empty grey box. The skeleton is drawn over the - // image until the browser reports it has loaded. - const [mainImageLoaded, setMainImageLoaded] = useState(false); - - const openLightbox = (index: number) => { - setLightboxIndex(index); - }; - - const closeLightbox = () => { - setLightboxIndex(null); - }; - - if (!images || images.length === 0) { - return ( -
-
- - - -

No images available

-
-
- ); - } - - return ( - <> -
- {/* Main Image */} -
- {propertyName} openLightbox(0)} - onLoad={() => setMainImageLoaded(true)} - /> - - {/* Issue #1253 — loading skeleton, drawn over the image until it has - loaded. It is decorative, so it is hidden from assistive tech. */} - {!mainImageLoaded && ( - - - {/* Thumbnail Gallery */} - {images.length > 1 && ( -
- {images.slice(1, 5).map((image, index) => { - const actualIndex = index + 1; - return ( -
openLightbox(actualIndex)} - > - {`${propertyName} - - {/* Hover overlay */} -
- - - -
-
- ); - })} - - {/* Show more indicator if there are more than 5 images */} - {images.length > 5 && ( -
-
-
- +{images.length - 5} -
-
- more -
-
-
-
- )} -
- )} -
- - {/* Lightbox */} - - - ); -}; +/** Property-specific copy for the shared Gallery. */ +export function ImageGallery({ images, propertyName }: ImageGalleryProps) { + return ; +} diff --git a/src/components/property/ImageLightbox.tsx b/src/components/property/ImageLightbox.tsx index 9aabe59e..e9f31409 100644 --- a/src/components/property/ImageLightbox.tsx +++ b/src/components/property/ImageLightbox.tsx @@ -1,209 +1,3 @@ 'use client'; -import React, { useState, useEffect, useCallback, useRef } from 'react'; -import Image from 'next/image'; -import { X, ChevronLeft, ChevronRight } from 'lucide-react'; - -interface ImageLightboxProps { - images: string[]; - initialIndex?: number; - isOpen: boolean; - onClose: () => void; -} - -export const ImageLightbox: React.FC = ({ - images, - initialIndex = 0, - isOpen, - onClose, -}) => { - const [currentIndex, setCurrentIndex] = useState(initialIndex); - const [touchStart, setTouchStart] = useState(null); - const [touchEnd, setTouchEnd] = useState(null); - const closeButtonRef = useRef(null); - const previouslyFocused = useRef(null); - - // Reset index when lightbox opens with new initialIndex - useEffect(() => { - if (isOpen) { - setCurrentIndex(initialIndex); - } - }, [initialIndex, isOpen]); - - // Issue #1252 — the overlay was a plain div, so a screen reader had no way - // of knowing a dialog had opened, and focus stayed wherever it was behind - // the overlay. Focus now moves into the dialog and is handed back to the - // element that opened it. - useEffect(() => { - if (!isOpen) return; - - previouslyFocused.current = document.activeElement as HTMLElement | null; - closeButtonRef.current?.focus(); - - return () => { - previouslyFocused.current?.focus?.(); - }; - }, [isOpen]); - - // Keyboard navigation - useEffect(() => { - if (!isOpen) return; - - const handleKeyDown = (e: KeyboardEvent) => { - switch (e.key) { - case 'ArrowLeft': - navigatePrevious(); - break; - case 'ArrowRight': - navigateNext(); - break; - case 'Escape': - onClose(); - break; - } - }; - - document.addEventListener('keydown', handleKeyDown); - document.body.style.overflow = 'hidden'; - - return () => { - document.removeEventListener('keydown', handleKeyDown); - document.body.style.overflow = 'unset'; - }; - }, [isOpen, currentIndex]); - - // Touch handlers for mobile swipe - const handleTouchStart = useCallback((e: React.TouchEvent) => { - setTouchEnd(null); - setTouchStart(e.targetTouches[0].clientX); - }, []); - - const handleTouchMove = useCallback((e: React.TouchEvent) => { - setTouchEnd(e.targetTouches[0].clientX); - }, []); - - const handleTouchEnd = useCallback(() => { - if (!touchStart || !touchEnd) return; - - const distance = touchStart - touchEnd; - const isLeftSwipe = distance > 50; - const isRightSwipe = distance < -50; - - if (isLeftSwipe) { - navigateNext(); - } - if (isRightSwipe) { - navigatePrevious(); - } - }, [touchStart, touchEnd]); - - const navigateNext = () => { - setCurrentIndex((prev) => (prev + 1) % images.length); - }; - - const navigatePrevious = () => { - setCurrentIndex((prev) => (prev - 1 + images.length) % images.length); - }; - - const handleThumbnailClick = (index: number) => { - setCurrentIndex(index); - }; - - if (!isOpen || images.length === 0) return null; - - return ( - // Issue #1252 — `role="dialog"` + `aria-modal` tell assistive tech that - // the rest of the page is behind a modal, and the label names it. -
- {/* Close Button */} - - - {/* Main Image Container */} -
- {`Property - - {/* Navigation Buttons */} - {images.length > 1 && ( - <> - - - - )} - - {/* Image Counter */} - {/* Issue #1252 — `aria-live` so the position is announced when it - changes. */} -
- {currentIndex + 1} / {images.length} -
-
- - {/* Thumbnail Strip */} - {images.length > 1 && ( -
- {images.map((image, index) => ( - - ))} -
- )} -
- ); -}; +export { ImageLightbox, type ImageLightboxProps } from '@/components/gallery/ImageLightbox'; diff --git a/src/components/property/__tests__/ImageGallery.test.tsx b/src/components/property/__tests__/ImageGallery.test.tsx index 37ba2dc6..deb1c4f4 100644 --- a/src/components/property/__tests__/ImageGallery.test.tsx +++ b/src/components/property/__tests__/ImageGallery.test.tsx @@ -65,6 +65,8 @@ describe('ImageGallery', () => { render(); expect(screen.getByText('No images available')).toBeInTheDocument(); + expect(screen.getByRole('status')).toHaveTextContent('Photos of Sunset Villa have not been added yet.'); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); expect(screen.queryByAltText('Sunset Villa')).not.toBeInTheDocument(); }); });