From c0d34796cdb1b541d501734afd929fa7089832e3 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 2 Sep 2026 12:49:55 +0200 Subject: [PATCH] fix(gallery): stop invisible overlay controls swallowing mobile taps Closes #1263. A tap on a photo tile did one of three things depending on where the finger landed: opened the photo, downloaded it, or liked it. The cause is that `opacity-0` hides pixels but not hit-testing. The overlay's View/Download/Like buttons and the selection checkbox were rendered at opacity 0 and left fully tappable; each one calls stopPropagation, so hitting an unseen button both fired its action and suppressed the tile's own open. On a pointer device hover reveals the controls before anyone can click them, so the gap never showed. On a touchscreen there is no hover, so in Masonry, Mosaic and Timeline the controls were invisible for good and tappable for good. Visibility and hit-testing now move together. PhotoCard computes both from one place, so every layout that uses it gets the same rule instead of passing its own opacity classes: - `touchAware` is gone. It gated the tap-to-reveal state machine, and only Grid and Justified opted in -- which is why those two behaved and the other three did not. Every PhotoCard layout is touch-aware now: first tap reveals the controls, second tap on a control acts, second tap elsewhere opens the photo. Pointer devices keep hover semantics unchanged. - The pointer reading moved from an effect into the initial state. As an effect it landed a mount-time render between the tile measurement in useLayoutEffect and the image mount that measurement gates, remounting every card once -- caught by the #1095 regression test, which is the reason that test exists. It also now degrades to ontouchstart/maxTouchPoints where matchMedia is absent, since every layout runs this path now. Two more instances of the same class, outside PhotoCard: - GalleryPremiumLayout's checkbox and like button are CSS-hidden the same way. They get pointer-events alongside opacity, and because that layout has no reveal gesture, a `(hover: none)` block shows both outright at a finger-sized target rather than leaving them unreachable. - PhotoGrid's download button called `onClick={onDownload}` with no stopPropagation, so downloading also opened the lightbox. Verified on a mobile viewport with real touch emulation: at rest the tile centre now hits the image rather than an unseen Download button, and one tap reveals the controls instead of downloading the file. 5 tests, all 5 failing before the change. --- frontend/src/components/gallery/PhotoCard.tsx | 85 +++++---- frontend/src/components/gallery/PhotoGrid.tsx | 14 +- .../__tests__/PhotoCard.touchTargets.test.tsx | 173 ++++++++++++++++++ .../gallery/layouts/GalleryPremiumLayout.css | 29 +++ .../gallery/layouts/GridGalleryLayout.tsx | 1 - .../layouts/JustifiedGalleryLayout.tsx | 1 - .../gallery/layouts/MasonryGalleryLayout.tsx | 8 +- .../gallery/layouts/MosaicGalleryLayout.tsx | 2 +- .../gallery/layouts/TimelineGalleryLayout.tsx | 2 +- 9 files changed, 264 insertions(+), 51 deletions(-) create mode 100644 frontend/src/components/gallery/__tests__/PhotoCard.touchTargets.test.tsx diff --git a/frontend/src/components/gallery/PhotoCard.tsx b/frontend/src/components/gallery/PhotoCard.tsx index 4eb17eda..9b53fb2b 100644 --- a/frontend/src/components/gallery/PhotoCard.tsx +++ b/frontend/src/components/gallery/PhotoCard.tsx @@ -9,6 +9,26 @@ import { ColorLabelBadge } from './ColorLabelBadge'; import { useGuestIdentityOptional } from '../../contexts/GuestIdentityContext'; import type { Photo } from '../../types'; +const COARSE_POINTER_QUERY = '(hover: none) and (pointer: coarse)'; + +/** + * Does this device lack hover? Read synchronously at first render rather than + * in an effect: every layout runs this now (#1263), and a mount-time state + * change here would land an extra render between the tile measurement in + * useLayoutEffect and the image mount it gates, remounting each card once. + * + * matchMedia is missing in some test environments and old embedded webviews, + * so the other two signals stand in for it. + */ +function detectCoarsePointer(): boolean { + if (typeof window === 'undefined') return false; + const hasNavigator = typeof navigator !== 'undefined'; + const fallback = ('ontouchstart' in window) + || (hasNavigator && navigator.maxTouchPoints > 0); + if (typeof window.matchMedia !== 'function') return fallback; + return window.matchMedia(COARSE_POINTER_QUERY).matches || fallback; +} + export interface PhotoCardFeedbackOptions { allowLikes?: boolean; allowFavorites?: boolean; @@ -37,9 +57,11 @@ export interface PhotoCardProps { skeletonClassName?: string; /** Keep container at opacity 0 until in view (only meaningful with `lazy`). */ fadeInWhenVisible?: boolean; - /** Tap-to-reveal overlay state machine for touch devices (Grid/Justified). */ - touchAware?: boolean; - /** Static overlay classes; `touchAware` appends computed visibility classes. */ + /** + * Static overlay classes — positioning, backdrop, spacing. Visibility and + * hit-testing are owned by this component for every layout (#1263), so a + * layout must NOT pass its own `opacity-*` / `group-hover:*` here. + */ overlayBaseClassName: string; /** 'light' = white/90 buttons with dark icons; 'dark' = white/20 buttons with white icons. */ actionVariant?: 'light' | 'dark'; @@ -84,7 +106,6 @@ export const PhotoCard: React.FC = ({ inViewRootMargin, skeletonClassName = 'skeleton w-full h-full rounded-lg', fadeInWhenVisible = false, - touchAware = false, overlayBaseClassName, actionVariant = 'light', allowDownloads = true, @@ -107,7 +128,7 @@ export const PhotoCard: React.FC = ({ }) => { const guestIdentity = useGuestIdentityOptional(); const [overlayVisible, setOverlayVisible] = useState(false); - const [isTouchDevice, setIsTouchDevice] = useState(false); + const [isTouchDevice, setIsTouchDevice] = useState(detectCoarsePointer); const overlayTimeoutRef = useRef(null); // Self-managed identity modal state (identityMode === 'self') @@ -117,22 +138,12 @@ export const PhotoCard: React.FC = ({ const savedIdentityValue = identityMode === 'self' ? selfIdentity : savedIdentity; - // Detect touch device (touch-aware overlay only) + // Keep the initial reading in step when the pointer changes under us — + // a tablet docked to a mouse, a browser window moved to another screen. useEffect(() => { - if (!touchAware || typeof window === 'undefined') return; - - const mediaQuery = window.matchMedia('(hover: none) and (pointer: coarse)'); - const updateTouchState = () => { - const hasNavigator = typeof navigator !== 'undefined'; - setIsTouchDevice( - mediaQuery.matches || - ('ontouchstart' in window) || - (hasNavigator && navigator.maxTouchPoints > 0) - ); - }; - - updateTouchState(); + if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') return; + const mediaQuery = window.matchMedia(COARSE_POINTER_QUERY); const listener = (event: MediaQueryListEvent) => { setIsTouchDevice(event.matches); }; @@ -150,7 +161,7 @@ export const PhotoCard: React.FC = ({ mediaQuery.removeListener(listener); } }; - }, [touchAware]); + }, []); const hideOverlay = useCallback(() => { if (overlayTimeoutRef.current !== null && typeof window !== 'undefined') { @@ -225,23 +236,22 @@ export const PhotoCard: React.FC = ({ const showFeedbackActions = feedbackEnabled && Boolean(feedbackOptions); - const overlayVisibilityClass = overlayVisible - ? 'opacity-100 md:opacity-100' - : 'opacity-0 md:opacity-0'; + // #1263 - opacity hides pixels, not hit-testing. An `opacity-0` control is + // still tappable, and on a touchscreen (no hover) it is invisible for good, + // so a tap in the middle of a tile silently downloaded or liked instead of + // opening the photo. Every visibility toggle below therefore moves + // pointer-events with it, in both the tap-to-reveal and the hover branch. + const revealed = (visible: boolean) => + (visible + ? 'opacity-100 md:opacity-100 pointer-events-auto md:pointer-events-auto' + : 'opacity-0 md:opacity-0 pointer-events-none md:pointer-events-none') + + ' md:group-hover:opacity-100 md:group-hover:pointer-events-auto'; - const overlayClassName = touchAware - ? `${overlayBaseClassName} ${overlayVisibilityClass} md:group-hover:opacity-100` - : overlayBaseClassName; + const overlayClassName = `${overlayBaseClassName} ${revealed(overlayVisible)}`; - const checkboxVisibilityClass = touchAware - ? `${ - isSelected || isSelectionMode || overlayVisible - ? 'opacity-100 md:opacity-100' - : 'opacity-0 md:opacity-0' - } md:group-hover:opacity-100` - : isSelected - ? 'opacity-100' - : 'opacity-0 group-hover:opacity-100'; + const checkboxVisibilityClass = revealed( + isSelected || isSelectionMode || overlayVisible, + ); const buttonType = actionVariant === 'dark' ? ('button' as const) : undefined; const actionButtonClass = @@ -251,11 +261,6 @@ export const PhotoCard: React.FC = ({ const actionIconClass = actionVariant === 'dark' ? 'w-5 h-5 text-white' : 'w-5 h-5 text-neutral-800'; const handlePhotoClick = (e: React.MouseEvent) => { - if (!touchAware) { - onClick(e); - return; - } - if (isTouchDevice && !overlayVisible && !isSelectionMode) { e.preventDefault(); e.stopPropagation(); diff --git a/frontend/src/components/gallery/PhotoGrid.tsx b/frontend/src/components/gallery/PhotoGrid.tsx index 436342e4..2379cb94 100644 --- a/frontend/src/components/gallery/PhotoGrid.tsx +++ b/frontend/src/components/gallery/PhotoGrid.tsx @@ -311,8 +311,11 @@ const PhotoThumbnail: React.FC = ({ )} - {/* Overlay on hover/tap - Always visible on mobile for better UX */} -
+ {/* Overlay on hover/tap - Always visible on mobile for better UX. + #1263: `md:opacity-0` hides the pixels but not the hit area, so + on a narrow pointer-device window the buttons stayed tappable + while invisible. pointer-events tracks opacity. */} +
{!isSelectionMode && ( <>