fix(gallery): decide the overlay by pointer capability, not viewport width

Codex review round 1 on #1272. Both findings are consequences of extending the
Grid/Justified tap-to-reveal model to every layout: what those two layouts got
away with, because they were the only ones using it, becomes wrong once
Masonry, Mosaic and Timeline inherit it.

The hover variants no longer hide behind `md:`. On a fine pointer under 768px
`isTouchDevice` is false, so nothing reveals the overlay, and the `md:` prefix
disabled the only hover variants there were -- the controls stayed
`opacity-0 pointer-events-none` with no way to reach them. Grid and Justified
already behaved that way, but Masonry, Mosaic and Timeline had unprefixed
`group-hover:` and revealed at any width, so this was a regression for them.
Width was never the real question: what the breakpoint was standing in for is
that :hover latches on a touchscreen once a tile is tapped. So the variants are
emitted for pointer devices only and withheld on touch, which says that
directly.

detectCoarsePointer no longer ORs the touch fallbacks over matchMedia.
matchMedia describes the PRIMARY pointer; `ontouchstart` and `maxTouchPoints`
only say a touchscreen exists somewhere, which is equally true of a touchscreen
laptop or a docked tablet being driven by its mouse. OR-ing them classified
those as touch-only, so an ordinary click merely revealed the overlay and
opening a photo took two clicks. The fallbacks now stand in only where
matchMedia is absent, which is what the comment already claimed.

3 more tests, all 3 failing before this commit.
This commit is contained in:
Paul Nothaft
2026-09-02 14:15:05 +02:00
parent c0d34796cd
commit d0274886e6
2 changed files with 67 additions and 11 deletions
+26 -10
View File
@@ -17,16 +17,21 @@ const COARSE_POINTER_QUERY = '(hover: none) and (pointer: coarse)';
* 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.
* matchMedia is authoritative wherever it exists, because it describes the
* PRIMARY pointer. `ontouchstart` and `maxTouchPoints` only say a touchscreen
* is present somewhere, which is equally true of a touchscreen laptop being
* driven by its mouse -- OR-ing them in would classify that as touch-only and
* turn every ordinary click into a two-step reveal. They stand in only where
* matchMedia is absent (old embedded webviews, some test environments).
*/
function detectCoarsePointer(): boolean {
if (typeof window === 'undefined') return false;
if (typeof window.matchMedia === 'function') {
return window.matchMedia(COARSE_POINTER_QUERY).matches;
}
const hasNavigator = typeof navigator !== 'undefined';
const fallback = ('ontouchstart' in window)
return ('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 {
@@ -241,11 +246,22 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
// 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';
//
// The hover variants are emitted for pointer devices only, rather than being
// gated behind `md:`. Width is the wrong proxy for hover: a mouse user with
// a window under 768px got no overlay at all, and in Masonry, Mosaic and
// Timeline -- whose `group-hover:` used to be unprefixed -- that made
// download and like unreachable at any narrow width. Withholding the classes
// on touch is what the breakpoint was really for, since :hover latches on a
// touchscreen once a tile has been tapped.
const revealed = (visible: boolean) => {
const base = visible
? 'opacity-100 pointer-events-auto'
: 'opacity-0 pointer-events-none';
return isTouchDevice
? base
: `${base} group-hover:opacity-100 group-hover:pointer-events-auto`;
};
const overlayClassName = `${overlayBaseClassName} ${revealed(overlayVisible)}`;
@@ -168,6 +168,46 @@ describe('PhotoCard overlay hit-testing (#1263)', () => {
fireEvent.click(container.querySelector('.tile')!);
expect(onClick).toHaveBeenCalledTimes(1);
expect(tokens(overlayOf(container))).toContain('md:group-hover:pointer-events-auto');
expect(tokens(overlayOf(container))).toContain('group-hover:pointer-events-auto');
});
it('reaches the hover overlay below the md breakpoint', () => {
// Codex review round 1. The hover variants used to be `md:group-hover:*`,
// so a mouse user with a window under 768px saw no overlay at all — and in
// Masonry, Mosaic and Timeline, whose `group-hover:` was unprefixed before
// this branch, that made download and like unreachable. Viewport width is
// not what decides whether a device can hover.
stubTouchDevice(false);
const { container } = renderCard();
const overlay = tokens(overlayOf(container));
expect(overlay).toContain('group-hover:opacity-100');
expect(overlay).toContain('group-hover:pointer-events-auto');
expect(overlay.some((c) => c.startsWith('md:'))).toBe(false);
});
it('withholds the hover variants on touch, where :hover latches after a tap', () => {
// The reason the breakpoint was there in the first place. Emitting
// `group-hover:` on a touchscreen leaves the overlay stuck open on the
// last tile tapped.
const { container } = renderCard();
expect(tokens(overlayOf(container)).some((c) => c.startsWith('group-hover:'))).toBe(false);
});
it('treats a touchscreen laptop driven by a mouse as a pointer device', () => {
// Codex review round 1. matchMedia describes the PRIMARY pointer;
// maxTouchPoints only says a touchscreen exists. OR-ing them classified a
// hybrid device as touch-only, so an ordinary click merely revealed the
// overlay and opening a photo took two clicks — a regression for the three
// layouts that had no tap-to-reveal step before.
stubTouchDevice(false);
Object.defineProperty(navigator, 'maxTouchPoints', { configurable: true, value: 10 });
(window as any).ontouchstart = null;
const onClick = vi.fn();
const { container } = renderCard({ onClick });
fireEvent.click(container.querySelector('.tile')!);
expect(onClick).toHaveBeenCalledTimes(1);
});
});