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.
This commit is contained in:
Paul Nothaft
2026-09-02 12:49:55 +02:00
parent f722bdaf4b
commit c0d34796cd
9 changed files with 264 additions and 51 deletions
+45 -40
View File
@@ -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<PhotoCardProps> = ({
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<PhotoCardProps> = ({
}) => {
const guestIdentity = useGuestIdentityOptional();
const [overlayVisible, setOverlayVisible] = useState(false);
const [isTouchDevice, setIsTouchDevice] = useState(false);
const [isTouchDevice, setIsTouchDevice] = useState(detectCoarsePointer);
const overlayTimeoutRef = useRef<number | null>(null);
// Self-managed identity modal state (identityMode === 'self')
@@ -117,22 +138,12 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
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<PhotoCardProps> = ({
mediaQuery.removeListener(listener);
}
};
}, [touchAware]);
}, []);
const hideOverlay = useCallback(() => {
if (overlayTimeoutRef.current !== null && typeof window !== 'undefined') {
@@ -225,23 +236,22 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
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<PhotoCardProps> = ({
const actionIconClass = actionVariant === 'dark' ? 'w-5 h-5 text-white' : 'w-5 h-5 text-neutral-800';
const handlePhotoClick = (e: React.MouseEvent<HTMLDivElement>) => {
if (!touchAware) {
onClick(e);
return;
}
if (isTouchDevice && !overlayVisible && !isSelectionMode) {
e.preventDefault();
e.stopPropagation();
+11 -3
View File
@@ -311,8 +311,11 @@ const PhotoThumbnail: React.FC<PhotoThumbnailProps> = ({
</div>
)}
{/* Overlay on hover/tap - Always visible on mobile for better UX */}
<div className="absolute inset-0 bg-black/40 opacity-100 md:opacity-0 md:group-hover:opacity-100 transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2">
{/* 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. */}
<div className="absolute inset-0 bg-black/40 opacity-100 pointer-events-auto md:opacity-0 md:pointer-events-none md:group-hover:opacity-100 md:group-hover:pointer-events-auto transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2">
{!isSelectionMode && (
<>
<button
@@ -328,7 +331,12 @@ const PhotoThumbnail: React.FC<PhotoThumbnailProps> = ({
{allowDownloads && (
<button
className="p-2 sm:p-2 bg-white/90 rounded-full hover:bg-white transition-colors"
onClick={onDownload}
// #1263 — without stopPropagation the tap also reached the
// tile's own onClick, so downloading opened the lightbox too.
onClick={(e) => {
e.stopPropagation();
onDownload(e);
}}
aria-label="Download photo"
>
<Download className="w-5 h-5 text-theme" />
@@ -0,0 +1,173 @@
/**
* Mobile tap determinism (#1263).
*
* The overlay actions (open / download / like) are hidden with `opacity-0`,
* which hides pixels but not hit-testing. On a pointer device hover reveals
* them before anyone can click, so the gap never shows; on a touchscreen there
* is no hover, so the buttons stayed permanently invisible AND permanently
* tappable. A tap near the middle of a tile hit an unseen button, whose
* stopPropagation then suppressed the tile's own open — so the same gesture
* downloaded, liked or opened depending on where the finger landed.
*
* Visibility and hit-testing have to move together. These tests pin the
* pointer-events half, which is the half that was missing.
*/
import React from 'react';
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { render, screen, fireEvent } from '@testing-library/react';
import { PhotoCard } from '../PhotoCard';
import type { Photo } from '../../../types';
vi.mock('../../common', () => ({
AuthenticatedImage: ({ src, alt }: { src: string; alt?: string }) => (
<img data-testid="tile" src={src} alt={alt} />
),
}));
vi.mock('../../../contexts/GuestIdentityContext', () => ({
useGuestIdentityOptional: () => null,
}));
const PHOTO = {
id: 7,
filename: 'IMG_0001.jpg',
url: '/api/gallery/x/photo/7',
thumbnail_url: '/api/gallery/x/thumbnail/7',
type: 'individual',
size: 1,
uploaded_at: '2026-01-01T00:00:00Z',
width: 4000,
height: 3000,
} as Photo;
/** Report a coarse, hover-less pointer — a phone. */
function stubTouchDevice(isTouch: boolean) {
Object.defineProperty(window, 'matchMedia', {
configurable: true,
writable: true,
value: (query: string) => ({
matches: isTouch && query.includes('pointer: coarse'),
media: query,
addEventListener: () => {},
removeEventListener: () => {},
addListener: () => {},
removeListener: () => {},
onchange: null,
dispatchEvent: () => false,
}),
});
Object.defineProperty(navigator, 'maxTouchPoints', {
configurable: true,
value: isTouch ? 5 : 0,
});
// jsdom defines `ontouchstart`, which PhotoCard reads as a third touch
// signal — remove it so the pointer-device case is actually exercised.
if (isTouch) (window as any).ontouchstart = null;
else delete (window as any).ontouchstart;
}
/** Class tokens, so `md:group-hover:pointer-events-auto` isn't mistaken for the bare one. */
function tokens(el: HTMLElement) {
return Array.from(el.classList);
}
function renderCard(props: Partial<React.ComponentProps<typeof PhotoCard>> = {}) {
return render(
<PhotoCard
photo={PHOTO}
isSelected={false}
isSelectionMode={false}
onClick={() => {}}
onDownload={() => {}}
onToggleSelect={() => {}}
className="group tile"
overlayBaseClassName="absolute inset-0 flex items-center justify-center gap-2"
imageProps={{ src: PHOTO.thumbnail_url!, alt: PHOTO.filename }}
allowDownloads
{...props}
/>,
);
}
/** The overlay is the element the action buttons live in. */
function overlayOf(container: HTMLElement) {
const button = container.querySelector('[aria-label="View full size"]');
return button?.parentElement as HTMLElement;
}
beforeEach(() => {
Object.defineProperty(HTMLElement.prototype, 'offsetWidth', {
configurable: true,
get() { return 320; },
});
stubTouchDevice(true);
});
afterEach(() => {
vi.restoreAllMocks();
delete (HTMLElement.prototype as any).offsetWidth;
});
describe('PhotoCard overlay hit-testing (#1263)', () => {
it('makes the hidden overlay inert, so a tap cannot reach an unseen button', () => {
const { container } = renderCard();
const overlay = overlayOf(container);
expect(tokens(overlay)).toContain('opacity-0');
expect(tokens(overlay)).toContain('pointer-events-none');
expect(tokens(overlay)).not.toContain('pointer-events-auto');
});
it('arms the overlay once a tap has revealed it', () => {
const onClick = vi.fn();
const { container } = renderCard({ onClick });
// First tap on the tile body reveals the actions; it must not open.
fireEvent.click(container.querySelector('.tile')!);
expect(onClick).not.toHaveBeenCalled();
const overlay = overlayOf(container);
expect(tokens(overlay)).toContain('opacity-100');
expect(tokens(overlay)).toContain('pointer-events-auto');
expect(tokens(overlay)).not.toContain('pointer-events-none');
// Second tap on the tile body now opens the photo.
fireEvent.click(container.querySelector('.tile')!);
expect(onClick).toHaveBeenCalledTimes(1);
});
it('keeps the selection checkbox inert while it is invisible', () => {
const onToggleSelect = vi.fn();
const { container } = renderCard({ onToggleSelect });
const checkbox = screen.getByRole('checkbox');
expect(tokens(checkbox)).toContain('opacity-0');
expect(tokens(checkbox)).toContain('pointer-events-none');
fireEvent.click(container.querySelector('.tile')!);
expect(tokens(screen.getByRole('checkbox'))).toContain('pointer-events-auto');
});
it('reveals on touch without a layout having to opt in', () => {
// Masonry / Mosaic / Timeline previously passed their own
// `opacity-0 group-hover:opacity-100` and never opted into the touch
// state machine, so on a phone their overlay was unreachable-but-tappable
// forever. PhotoCard owns visibility for every layout now.
const onClick = vi.fn();
const { container } = renderCard({ onClick });
fireEvent.click(container.querySelector('.tile')!);
expect(onClick).not.toHaveBeenCalled();
expect(tokens(overlayOf(container))).toContain('pointer-events-auto');
});
it('leaves a pointer device on hover semantics — no tap-to-reveal step', () => {
stubTouchDevice(false);
const onClick = vi.fn();
const { container } = renderCard({ onClick });
fireEvent.click(container.querySelector('.tile')!);
expect(onClick).toHaveBeenCalledTimes(1);
expect(tokens(overlayOf(container))).toContain('md:group-hover:pointer-events-auto');
});
});
@@ -292,11 +292,16 @@
transition: all 0.2s ease;
cursor: pointer;
opacity: 0;
/* #1263 - opacity 0 still hit-tests. On a touchscreen there is no hover to
reveal this, so the top-left corner of every tile silently toggled
selection. Hit-testing tracks visibility. */
pointer-events: none;
}
.gallery-premium-photo-card:hover .gallery-premium-checkbox,
.gallery-premium-checkbox.visible {
opacity: 1;
pointer-events: auto;
}
.gallery-premium-checkbox:hover {
@@ -323,11 +328,35 @@
cursor: pointer;
transition: all 0.2s ease;
opacity: 0;
/* #1263 - same as the checkbox: invisible must also mean untappable, or the
top-right corner of every tile likes the photo instead of opening it. */
pointer-events: none;
}
.gallery-premium-photo-card:hover .gallery-premium-like-btn,
.gallery-premium-like-btn.liked {
opacity: 1;
pointer-events: auto;
}
/* Touch devices never get :hover, so the two controls above would be
permanently unreachable rather than merely accidental. Show them outright
and give them a finger-sized target (#1263). */
@media (hover: none) and (pointer: coarse) {
.gallery-premium-checkbox,
.gallery-premium-like-btn {
opacity: 1;
pointer-events: auto;
}
.gallery-premium-checkbox {
height: 2.25rem;
width: 2.25rem;
}
.gallery-premium-like-btn {
padding: 0.75rem;
}
}
.gallery-premium-like-btn:hover {
@@ -86,7 +86,6 @@ const GridPhoto: React.FC<GridPhotoProps> = ({
lazy
fadeInWhenVisible={animationType === 'fade'}
skeletonClassName="skeleton aspect-square w-full rounded-lg"
touchAware
imageProps={{
src: photo.thumbnail_url || photo.url,
alt: photo.filename,
@@ -127,7 +127,6 @@ const JustifiedPhoto: React.FC<JustifiedPhotoProps> = ({
inViewRootMargin="100px"
fadeInWhenVisible={animationType === 'fade'}
skeletonClassName="skeleton w-full h-full rounded-lg"
touchAware
imageProps={{
src: photo.thumbnail_url || photo.url,
alt: photo.filename,
@@ -122,7 +122,7 @@ const MasonryPhoto: React.FC<MasonryPhotoProps> = ({
isGallery: true,
protectFromDownload: !allowDownloads,
}}
overlayBaseClassName="absolute inset-0 bg-black/40 opacity-0 group-hover:opacity-100 transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2"
overlayBaseClassName="absolute inset-0 bg-black/40 transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2"
allowDownloads={allowDownloads}
feedbackEnabled={feedbackEnabled}
feedbackOptions={feedbackOptions}
@@ -378,7 +378,7 @@ export const MasonryGalleryLayout: React.FC<BaseGalleryLayoutProps> = ({
isGallery: true,
protectFromDownload: !allowDownloads,
}}
overlayBaseClassName="absolute inset-0 bg-black/40 opacity-0 group-hover:opacity-100 transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2"
overlayBaseClassName="absolute inset-0 bg-black/40 transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2"
actionVariant="dark"
allowDownloads={allowDownloads}
beforeOverlay={feedbackEnabled ? <FeedbackCountIndicators photo={photo} /> : undefined}
@@ -436,7 +436,7 @@ export const MasonryGalleryLayout: React.FC<BaseGalleryLayoutProps> = ({
isGallery: true,
protectFromDownload: !allowDownloads,
}}
overlayBaseClassName="absolute inset-0 bg-black/40 opacity-0 group-hover:opacity-100 transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2"
overlayBaseClassName="absolute inset-0 bg-black/40 transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2"
actionVariant="dark"
allowDownloads={allowDownloads}
beforeOverlay={feedbackEnabled ? <FeedbackCountIndicators photo={photo} /> : undefined}
@@ -503,7 +503,7 @@ export const MasonryGalleryLayout: React.FC<BaseGalleryLayoutProps> = ({
isGallery: true,
protectFromDownload: !allowDownloads,
}}
overlayBaseClassName="absolute inset-0 bg-black/40 opacity-0 group-hover:opacity-100 transition-opacity duration-200 flex items-center justify-center gap-2"
overlayBaseClassName="absolute inset-0 bg-black/40 transition-opacity duration-200 flex items-center justify-center gap-2"
actionVariant="dark"
allowDownloads={allowDownloads}
beforeOverlay={feedbackEnabled ? <FeedbackCountIndicators photo={photo} /> : undefined}
@@ -85,7 +85,7 @@ const MosaicPhoto: React.FC<MosaicPhotoProps> = ({
isGallery: true,
protectFromDownload: !allowDownloads,
}}
overlayBaseClassName="absolute inset-0 bg-black/40 opacity-0 group-hover:opacity-100 transition-opacity duration-200 flex items-center justify-center gap-2"
overlayBaseClassName="absolute inset-0 bg-black/40 transition-opacity duration-200 flex items-center justify-center gap-2"
allowDownloads={allowDownloads}
feedbackEnabled={feedbackEnabled}
feedbackOptions={feedbackOptions}
@@ -132,7 +132,7 @@ export const TimelineGalleryLayout: React.FC<BaseGalleryLayoutProps> = ({
isGallery: true,
protectFromDownload: !allowDownloads,
}}
overlayBaseClassName="absolute inset-0 bg-black/40 opacity-0 group-hover:opacity-100 transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2"
overlayBaseClassName="absolute inset-0 bg-black/40 transition-opacity duration-200 rounded-lg flex items-center justify-center gap-2"
allowDownloads={allowDownloads}
feedbackEnabled={feedbackEnabled}
feedbackOptions={feedbackOptions}