From 4c2eeab2f482a04304b7110876f7ccd10fbeee76 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Tue, 1 Sep 2026 18:45:49 +0200 Subject: [PATCH] refactor(gallery): drop the unreachable Story feedback sheet StoryFeedbackSheet could never open: handleOpenFeedback was the only caller of setSelectedPhotoForFeedback and was itself never called. This was the last remaining build:check error (TS6133). Removed rather than wired up, on three findings: - The sheet offered nothing PhotoLightbox does not, and was strictly worse. It held comments and ratings in layout-local useState and never called feedbackService.getPhotoFeedback, so existing server-side feedback was invisible; it rendered stars and a comment form unconditionally, ignoring allow_ratings/allow_comments; and it had no reactions, colour labels, identity modal or rate-limit handling. This layout already renders PhotoLightbox with feedbackEnabled, which does all of that against the server. - It was not a mobile affordance. The CSS styled it as a fixed right-edge desktop drawer (right: 0; max-width: 28rem) with no media query. - Every sibling layout routes feedback through the lightbox. Grid, Masonry, Timeline, Mosaic and Carousel expose a per-card onQuickComment that calls onOpenPhotoWithFeedback to open the parent's lightbox on the feedback tab; none has a standalone feedback surface. The closest sibling, GalleryPremiumLayout, renders its own lightbox and deliberately voids _onOpenPhotoWithFeedback with no per-card control -- exactly the shape Story now has. Drops the component, its state and handlers, the feedbackOptions destructure (only the sheet read it) and 251 lines of orphaned CSS. savedIdentity also fed guest_name/guest_email into the like call; those were always undefined at runtime since the unreachable sheet was their only writer, so no behaviour changes. Also widens the Story nav search input, which clipped its placeholder. At the input's computed 14px the placeholder measures en 121px, de 145, ru 152, fr 174 against a 128px box -- so German was 17px over and French 46px over. 8rem -> 13rem collapsed, 12rem -> 17rem focused, keeping expand-on-focus; verified at 1280px and at the 768px breakpoint where the search appears. Refs testplan REPORT.md A1 and the gallery-story placeholder warning. --- .../gallery/layouts/GalleryStoryLayout.css | 251 +----------------- .../gallery/layouts/GalleryStoryLayout.tsx | 97 +------ .../GalleryStoryLayout.feedback.test.tsx | 131 +++++++++ .../layouts/story/StoryFeedbackSheet.tsx | 172 ------------ .../components/gallery/layouts/story/index.ts | 1 - 5 files changed, 141 insertions(+), 511 deletions(-) create mode 100644 frontend/src/components/gallery/layouts/__tests__/GalleryStoryLayout.feedback.test.tsx delete mode 100644 frontend/src/components/gallery/layouts/story/StoryFeedbackSheet.tsx diff --git a/frontend/src/components/gallery/layouts/GalleryStoryLayout.css b/frontend/src/components/gallery/layouts/GalleryStoryLayout.css index f63107a8..51a232d6 100644 --- a/frontend/src/components/gallery/layouts/GalleryStoryLayout.css +++ b/frontend/src/components/gallery/layouts/GalleryStoryLayout.css @@ -99,18 +99,21 @@ } } +/* 13rem collapsed so the placeholder still fits in the longest locale — + "Rechercher des souvenirs..." (fr) measures ~11rem at 0.875rem, and the + old 8rem clipped even the German string. */ .story-nav-search input { background: transparent; border: none; outline: none; font-size: 0.875rem; - width: 8rem; + width: 13rem; color: var(--story-foreground); transition: width 0.2s ease; } .story-nav-search input:focus { - width: 12rem; + width: 17rem; } .story-nav-search input::placeholder { @@ -437,226 +440,6 @@ } } -/* Feedback Sheet */ -.story-feedback-sheet { - position: fixed; - top: 0; - right: 0; - bottom: 0; - width: 100%; - max-width: 28rem; - background-color: rgba(13, 13, 13, 0.95); - backdrop-filter: blur(24px); - -webkit-backdrop-filter: blur(24px); - border-left: 1px solid rgba(255, 255, 255, 0.1); - z-index: 60; - transform: translateX(100%); - transition: transform 0.3s ease; - padding: 1.5rem; - overflow-y: auto; -} - -.story-feedback-sheet.open { - transform: translateX(0); -} - -.story-feedback-sheet-header { - margin-bottom: 1.5rem; -} - -.story-feedback-sheet-title { - font-size: 1.5rem; - margin-bottom: 0.25rem; -} - -.story-feedback-sheet-description { - color: var(--story-muted-foreground); - font-size: 0.875rem; -} - -.story-feedback-rating { - margin-bottom: 2rem; -} - -.story-feedback-rating-label { - display: block; - font-size: 0.75rem; - font-weight: 500; - text-transform: uppercase; - letter-spacing: 0.1em; - color: var(--story-muted-foreground); - margin-bottom: 0.5rem; -} - -.story-feedback-rating-stars { - display: flex; - gap: 0.5rem; -} - -.story-feedback-rating-star { - background: none; - border: none; - cursor: pointer; - padding: 0; - transition: transform 0.2s ease; -} - -.story-feedback-rating-star:hover { - transform: scale(1.1); -} - -.story-feedback-rating-star.active svg { - fill: var(--story-primary); - color: var(--story-primary); -} - -.story-feedback-rating-star svg { - color: var(--story-muted-foreground); - transition: color 0.2s ease; -} - -.story-feedback-rating-star:hover svg { - color: rgba(201, 169, 97, 0.5); -} - -.story-feedback-comments { - max-height: 40vh; - overflow-y: auto; - padding-right: 0.5rem; - margin-bottom: 2rem; -} - -.story-feedback-comments-label { - display: block; - font-size: 0.75rem; - font-weight: 500; - text-transform: uppercase; - letter-spacing: 0.1em; - color: var(--story-muted-foreground); - margin-bottom: 1rem; -} - -.story-feedback-comment { - display: flex; - gap: 0.75rem; - font-size: 0.875rem; - margin-bottom: 1rem; -} - -.story-feedback-comment-avatar { - width: 2rem; - height: 2rem; - border-radius: 9999px; - background-color: var(--story-muted); - display: flex; - align-items: center; - justify-content: center; - font-size: 0.75rem; - font-weight: 500; - flex-shrink: 0; - border: 1px solid rgba(255, 255, 255, 0.1); -} - -.story-feedback-comment-content { - flex: 1; -} - -.story-feedback-comment-header { - display: flex; - justify-content: space-between; - align-items: baseline; - margin-bottom: 0.25rem; -} - -.story-feedback-comment-author { - font-weight: 500; -} - -.story-feedback-comment-date { - font-size: 0.625rem; - color: var(--story-muted-foreground); -} - -.story-feedback-comment-text { - color: var(--story-muted-foreground); - line-height: 1.5; -} - -.story-feedback-empty { - font-size: 0.875rem; - color: rgba(163, 163, 163, 0.5); - font-style: italic; -} - -.story-feedback-form { - padding-top: 1rem; - border-top: 1px solid rgba(255, 255, 255, 0.1); -} - -.story-feedback-textarea { - width: 100%; - min-height: 100px; - padding: 0.75rem; - background-color: rgba(26, 26, 26, 0.5); - border: 1px solid rgba(255, 255, 255, 0.05); - border-radius: 0.25rem; - color: var(--story-foreground); - font-size: 0.875rem; - resize: none; - margin-bottom: 0.75rem; -} - -.story-feedback-textarea:focus { - outline: none; - border-color: rgba(201, 169, 97, 0.3); -} - -.story-feedback-textarea::placeholder { - color: var(--story-muted-foreground); -} - -.story-feedback-submit { - width: 100%; - padding: 0.75rem 1rem; - background-color: var(--story-primary); - color: var(--story-primary-foreground); - border: none; - border-radius: 0.125rem; - font-weight: 500; - letter-spacing: 0.05em; - cursor: pointer; - display: flex; - align-items: center; - justify-content: center; - gap: 0.5rem; - transition: background-color 0.2s ease; -} - -.story-feedback-submit:hover { - background-color: white; -} - -.story-feedback-submit:disabled { - opacity: 0.5; - cursor: not-allowed; -} - -.story-feedback-close { - position: absolute; - top: 1rem; - right: 1rem; - padding: 0.5rem; - background: none; - border: none; - color: var(--story-muted-foreground); - cursor: pointer; - transition: color 0.2s ease; -} - -.story-feedback-close:hover { - color: var(--story-foreground); -} - /* Scroll to Top */ .story-scroll-to-top { position: fixed; @@ -717,27 +500,3 @@ .story-footer-btn:hover { background-color: white; } - -/* Backdrop Overlay */ -.story-backdrop { - position: fixed; - inset: 0; - background-color: rgba(0, 0, 0, 0.5); - z-index: 55; -} - -/* Animation */ -@keyframes storyFadeIn { - from { - opacity: 0; - transform: translateY(20px); - } - to { - opacity: 1; - transform: translateY(0); - } -} - -.story-animate-fade-in { - animation: storyFadeIn 0.8s ease-out forwards; -} diff --git a/frontend/src/components/gallery/layouts/GalleryStoryLayout.tsx b/frontend/src/components/gallery/layouts/GalleryStoryLayout.tsx index a4a073f3..2866284b 100644 --- a/frontend/src/components/gallery/layouts/GalleryStoryLayout.tsx +++ b/frontend/src/components/gallery/layouts/GalleryStoryLayout.tsx @@ -14,7 +14,6 @@ import { StoryScene, StoryPhotoCard, StoryCarousel, - StoryFeedbackSheet, StoryScrollToTop } from './story'; import { PhotoLightbox } from '../PhotoLightbox'; @@ -60,7 +59,6 @@ export const GalleryStoryLayout: React.FC = ({ useEnhancedProtection = false, useCanvasRendering = false, feedbackEnabled = false, - feedbackOptions, heroPhotoOverride, welcomeMessage, onLogout, @@ -80,11 +78,7 @@ export const GalleryStoryLayout: React.FC = ({ const [scrolled, setScrolled] = useState(false); const [searchQuery, setSearchQuery] = useState(''); const [favorites, setFavorites] = useState>(new Set()); - const [selectedPhotoForFeedback, setSelectedPhotoForFeedback] = useState(null); const [lightboxIndex, setLightboxIndex] = useState(null); - const [comments, setComments] = useState>>({}); - const [ratings, setRatings] = useState>({}); - const [savedIdentity, setSavedIdentity] = useState<{ name: string; email: string } | null>(null); // Track scroll for nav background useEffect(() => { @@ -161,87 +155,18 @@ export const GalleryStoryLayout: React.FC = ({ try { await feedbackService.submitFeedback(slug, String(photoId), { feedback_type: 'like', - guest_name: savedIdentity?.name, - guest_email: savedIdentity?.email, }); onFeedbackChange?.(); } catch (err) { console.warn('Like submit failed', err); } - }, [favorites, slug, savedIdentity, onFeedbackChange]); - - const handleOpenFeedback = useCallback((photo: Photo) => { - setSelectedPhotoForFeedback(photo); - }, []); + }, [favorites, slug, onFeedbackChange]); const handleOpenLightbox = useCallback((photo: Photo) => { const index = photos.findIndex(p => p.id === photo.id); setLightboxIndex(index >= 0 ? index : 0); }, [photos]); - const handleCloseFeedback = useCallback(() => { - setSelectedPhotoForFeedback(null); - }, []); - - const handleAddComment = useCallback(async (text: string, name?: string, email?: string) => { - if (!selectedPhotoForFeedback) return; - - if (name && email) { - setSavedIdentity({ name, email }); - } - - const newComment = { - id: `${Date.now()}`, - author: name || savedIdentity?.name || t('gallery.feedback.anonymous', 'Anonymous'), - text, - date: new Date().toLocaleDateString() - }; - - setComments(prev => ({ - ...prev, - [selectedPhotoForFeedback.id]: [...(prev[selectedPhotoForFeedback.id] || []), newComment] - })); - - try { - await feedbackService.submitFeedback(slug, String(selectedPhotoForFeedback.id), { - feedback_type: 'comment', - comment_text: text, - guest_name: name || savedIdentity?.name, - guest_email: email || savedIdentity?.email, - }); - onFeedbackChange?.(); - } catch (err) { - console.warn('Comment submit failed', err); - } - }, [selectedPhotoForFeedback, slug, savedIdentity, onFeedbackChange, t]); - - const handleRate = useCallback(async (rating: number) => { - if (!selectedPhotoForFeedback) return; - - // Clicking the star you already gave clears the rating (#884) — - // 0 tells the backend to delete it. Session-local `ratings` is the - // source of truth for "my rating" here, never the photo's average. - const current = ratings[selectedPhotoForFeedback.id] || 0; - const next = rating === current ? 0 : rating; - - setRatings(prev => ({ - ...prev, - [selectedPhotoForFeedback.id]: next - })); - - try { - await feedbackService.submitFeedback(slug, String(selectedPhotoForFeedback.id), { - feedback_type: 'rating', - rating: next, - guest_name: savedIdentity?.name, - guest_email: savedIdentity?.email, - }); - onFeedbackChange?.(); - } catch (err) { - console.warn('Rating submit failed', err); - } - }, [selectedPhotoForFeedback, ratings, slug, savedIdentity, onFeedbackChange]); - const handleDownloadAll = useCallback(async () => { // Whole-gallery path when available: posting ids would hit the server's // 500-id cap and silently truncate a large gallery (#1160). @@ -399,7 +324,10 @@ export const GalleryStoryLayout: React.FC = ({ )} - {/* Lightbox */} + {/* Lightbox. It owns the whole feedback surface on this theme — ratings, + comments, reactions and colour labels — the same way the Premium + layout routes feedback through its own lightbox instead of a + per-card affordance. */} {lightboxIndex !== null && ( = ({ onSelectPerson={onSelectPerson} /> )} - - {/* Feedback Sheet */} - {feedbackEnabled && ( - - )} ); }; diff --git a/frontend/src/components/gallery/layouts/__tests__/GalleryStoryLayout.feedback.test.tsx b/frontend/src/components/gallery/layouts/__tests__/GalleryStoryLayout.feedback.test.tsx new file mode 100644 index 00000000..b43beee7 --- /dev/null +++ b/frontend/src/components/gallery/layouts/__tests__/GalleryStoryLayout.feedback.test.tsx @@ -0,0 +1,131 @@ +/** + * The Story theme routes ALL feedback through its own PhotoLightbox. + * + * It used to also render a `StoryFeedbackSheet`, but the only setter for the + * state that opened it was never called, so the sheet was unreachable — a + * second, weaker feedback surface (local-only comments, never fetched from the + * server, no allow_comments/allow_ratings gating) shadowing the lightbox's. + * The sheet was removed; this pins the layout to the lightbox-only shape the + * Premium theme also uses, so the dead surface cannot come back unnoticed. + */ +import React from 'react'; +import fs from 'fs'; +import path from 'path'; +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; + +import { GalleryStoryLayout } from '../GalleryStoryLayout'; +import type { Photo } from '../../../../types'; + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string, fallback?: unknown) => (typeof fallback === 'string' ? fallback : key), + }), +})); + +// framer-motion's useInView needs IntersectionObserver, and the animation +// props would leak onto the DOM node. +vi.mock('framer-motion', () => { + const stub = (tag: string) => + React.forwardRef>(({ children, className, onClick }, ref) => + React.createElement(tag, { ref, className, onClick }, children as React.ReactNode) + ); + return { + motion: new Proxy({} as Record, { + get: (cache, tag: string) => (cache[tag] ??= stub(tag)), + }), + AnimatePresence: ({ children }: { children?: React.ReactNode }) => <>{children}, + useInView: () => true, + }; +}); + +vi.mock('../../../common', () => ({ + AuthenticatedImage: ({ src, alt }: { src: string; alt?: string }) => {alt}, + PoweredBy: () => null, +})); + +vi.mock('../../PhotoLightbox', () => ({ + PhotoLightbox: ({ feedbackEnabled, initialIndex }: { feedbackEnabled?: boolean; initialIndex: number }) => ( +
+ ), +})); + +const submitFeedback = vi.fn().mockResolvedValue({}); +vi.mock('../../../../services/feedback.service', () => ({ + feedbackService: { submitFeedback: (...args: unknown[]) => submitFeedback(...args) }, +})); +vi.mock('../../../../services/gallery.service', () => ({ + galleryService: { downloadSelectedPhotos: vi.fn() }, +})); +vi.mock('../../../../services/analytics.service', () => ({ + analyticsService: { trackGalleryEvent: vi.fn() }, +})); + +const photos: Photo[] = [1, 2, 3].map((i) => ({ + id: i, + filename: `IMG_${i}.jpg`, + url: `/api/gallery/x/photo/${i}`, + thumbnail_url: `/api/gallery/x/thumbnail/${i}`, + type: 'individual', + size: 1, + uploaded_at: '2026-01-01T00:00:00Z', + category_name: 'Ceremony', +} as Photo)); + +const props = { + photos, + slug: 'x', + eventName: 'Sarah & Tom', + onPhotoClick: () => {}, + onDownload: () => {}, + selectedPhotos: new Set(), + isSelectionMode: false, + allowDownloads: true, + feedbackEnabled: true, + feedbackOptions: { requireNameEmail: true, allowComments: true, allowRatings: true }, +} as never; + +describe('GalleryStoryLayout — feedback lives in the lightbox only', () => { + it('renders no feedback sheet even with feedback fully enabled', () => { + const { container } = render(); + + expect(container.querySelector('.story-feedback-sheet')).toBeNull(); + expect(container.querySelector('.story-backdrop')).toBeNull(); + expect(screen.queryByText('Rate this moment')).toBeNull(); + expect(screen.queryByPlaceholderText('Write a lovely note...')).toBeNull(); + }); + + it('gives each card a like button and nothing else — no per-card feedback affordance', () => { + const { container } = render(); + + const actions = container.querySelectorAll('.story-photo-card-actions'); + expect(actions.length).toBe(photos.length); + actions.forEach((row) => expect(row.querySelectorAll('button')).toHaveLength(1)); + }); + + it('opens the lightbox with feedback enabled when a card is clicked', () => { + const { container } = render(); + + expect(screen.queryByTestId('lightbox')).toBeNull(); + fireEvent.click(container.querySelector('a[data-photo-id="2"]')!); + + const lightbox = screen.getByTestId('lightbox'); + expect(lightbox.getAttribute('data-feedback-enabled')).toBe('true'); + expect(lightbox.getAttribute('data-index')).toBe('1'); + }); + + it('keeps no unreachable feedback state in the source', () => { + const layouts = path.resolve(process.cwd(), 'src/components/gallery/layouts'); + + const layout = fs.readFileSync(path.join(layouts, 'GalleryStoryLayout.tsx'), 'utf8'); + expect(layout).not.toMatch(/StoryFeedbackSheet|selectedPhotoForFeedback/); + + const storyIndex = fs.readFileSync(path.join(layouts, 'story/index.ts'), 'utf8'); + expect(storyIndex).not.toMatch(/StoryFeedbackSheet/); + expect(fs.existsSync(path.join(layouts, 'story/StoryFeedbackSheet.tsx'))).toBe(false); + }); +}); diff --git a/frontend/src/components/gallery/layouts/story/StoryFeedbackSheet.tsx b/frontend/src/components/gallery/layouts/story/StoryFeedbackSheet.tsx deleted file mode 100644 index b05cfe83..00000000 --- a/frontend/src/components/gallery/layouts/story/StoryFeedbackSheet.tsx +++ /dev/null @@ -1,172 +0,0 @@ -import React, { useState } from 'react'; -import { Star, Send, X } from 'lucide-react'; -import { useTranslation } from 'react-i18next'; -import type { Photo } from '../../../../types'; - -interface Comment { - id: string; - author: string; - text: string; - date: string; -} - -interface StoryFeedbackSheetProps { - isOpen: boolean; - onClose: () => void; - photo: Photo | null; - comments: Comment[]; - rating: number; - onAddComment: (text: string, name?: string, email?: string) => void; - onRate: (rating: number) => void; - requireNameEmail?: boolean; - savedIdentity?: { name: string; email: string } | null; -} - -export const StoryFeedbackSheet: React.FC = ({ - isOpen, - onClose, - photo, - comments, - rating, - onAddComment, - onRate, - requireNameEmail = false, - savedIdentity -}) => { - const { t } = useTranslation(); - const [commentText, setCommentText] = useState(''); - const [guestName, setGuestName] = useState(savedIdentity?.name || ''); - const [guestEmail, setGuestEmail] = useState(savedIdentity?.email || ''); - - const handleSubmit = (e: React.FormEvent) => { - e.preventDefault(); - if (commentText.trim()) { - if (requireNameEmail && (!guestName.trim() || !guestEmail.trim())) { - return; - } - onAddComment(commentText, guestName || undefined, guestEmail || undefined); - setCommentText(''); - } - }; - - if (!photo) return null; - - return ( - <> - {/* Backdrop */} - {isOpen && ( -
- )} - - {/* Sheet */} -
- - -
-

- {t('gallery.feedback.title', 'Feedback')} -

-

- {t('gallery.feedback.shareThoughts', 'Share your thoughts on "{{name}}"', { name: photo.filename })} -

-
- -
- {/* Rating Section */} -
- -
- {[1, 2, 3, 4, 5].map((star) => ( - - ))} -
-
- - {/* Comments List */} -
- - -
- {comments.length === 0 ? ( -

- {t('gallery.feedback.noComments', 'No comments yet. Be the first!')} -

- ) : ( - comments.map((comment) => ( -
-
- {comment.author[0]?.toUpperCase() || '?'} -
-
-
- {comment.author} - {comment.date} -
-

{comment.text}

-
-
- )) - )} -
-
- - {/* Comment Form */} -
- {requireNameEmail && !savedIdentity && ( -
- setGuestName(e.target.value)} - className="story-feedback-textarea" - style={{ minHeight: 'auto', padding: '0.5rem 0.75rem' }} - required - /> - setGuestEmail(e.target.value)} - className="story-feedback-textarea" - style={{ minHeight: 'auto', padding: '0.5rem 0.75rem' }} - required - /> -
- )} -