paul/picpeak
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(gallery): follow the input in use, not the device's primary pointer

Browse Source 
Closes #1275. Follow-up to #1263.

`matchMedia('(hover: none) and (pointer: coarse)')` answers "what is this
device's primary pointer", which on anything with both inputs is the wrong
question. A touchscreen laptop reports fine+hover, so a finger tap was handled
as a click: the photo opened with no reveal step and the tile's own actions
needed a hover a finger cannot produce. An iPad with a trackpad reports the
opposite, so a mouse click was handled as a tap and opening a photo took two of
them while hovering did nothing.

Pointer events carry the answer per interaction. useInputMode holds one
module-level mode fed by a single window-level listener pair, so every tile
agrees and the listener count does not scale with the grid. The primary-pointer
query stays as the opening guess -- it is right for the two single-input cases
that are most of the traffic, a phone and a desktop -- and the first real
interaction corrects it on a hybrid.

pointermove matters as much as pointerdown: a mouse announces itself by
approaching, and the mode has to be right BEFORE the click, not as a
consequence of it. A pen is grouped with touch, since it taps rather than
hovers on most hardware and being wrong that way costs only a reveal step.

GalleryPremiumLayout's touch rules move off the media query onto a
data-input-mode attribute the layout sets, for the same reason: on a
touchscreen laptop the query stayed false and a finger could never reach the
checkbox or like button, and on an iPad with a trackpad it stayed true and both
were stuck on permanently.

The #1263 guarantee is unchanged and pinned by a test that walks all three
modes: a control that cannot be seen cannot be hit, whichever input is in use.

Verified in the running app under Chrome touch emulation, on a device
advertising a coarse primary pointer -- the iPad-with-trackpad case. A mouse
merely moving switched the grid to hover semantics and revealed the overlay,
and a subsequent tap switched it back; the premium layout's attribute followed,
with its checkbox reachable under touch and hidden-but-inert under mouse. The
mirror case (finger on a fine-primary device) cannot be staged in Chrome, which
couples touch emulation to a coarse primary pointer, so it rests on the jsdom
tests.

12 tests: 8 on the store, 4 more on PhotoCard. 3 of the 4 fail without the
per-interaction mode; the fourth is the #1263 no-regression guard and holds on
both sides by design.
This commit is contained in:
Paul Nothaft
2026-09-02 20:04:15 +02:00
parent b9c29fcf9b
commit 0b6b8fbdb0
6 changed files with 348 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/components/gallery/PhotoCard.tsx
+11 -58
View File
@@ -7,33 +7,9 @@ import { FeedbackIdentityModal } from './FeedbackIdentityModal';
import { feedbackService } from '../../services/feedback.service';
import { ColorLabelBadge } from './ColorLabelBadge';
import { useGuestIdentityOptional } from '../../contexts/GuestIdentityContext';
import { useInputMode } from '../../hooks/useInputMode';
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 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';
return ('ontouchstart' in window)
|| (hasNavigator && navigator.maxTouchPoints > 0);
}
export interface PhotoCardFeedbackOptions {
allowLikes?: boolean;
allowFavorites?: boolean;
@@ -133,7 +109,10 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
}) => {
const guestIdentity = useGuestIdentityOptional();
const [overlayVisible, setOverlayVisible] = useState(false);
const [isTouchDevice, setIsTouchDevice] = useState(detectCoarsePointer);
// #1275 — the input in use right now, not what the device is capable of.
// On a hybrid the two disagree, and acting on the device's primary pointer
// handles one of its two inputs as if it were the other.
const isTouchDevice = useInputMode() === 'touch';
const overlayTimeoutRef = useRef<number | null>(null);
// Self-managed identity modal state (identityMode === 'self')
@@ -143,31 +122,6 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
const savedIdentityValue = identityMode === 'self' ? selfIdentity : savedIdentity;
// 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 (typeof window === 'undefined' || typeof window.matchMedia !== 'function') return;
const mediaQuery = window.matchMedia(COARSE_POINTER_QUERY);
const listener = (event: MediaQueryListEvent) => {
setIsTouchDevice(event.matches);
};
if (mediaQuery.addEventListener) {
mediaQuery.addEventListener('change', listener);
} else if (mediaQuery.addListener) {
mediaQuery.addListener(listener);
}
return () => {
if (mediaQuery.removeEventListener) {
mediaQuery.removeEventListener('change', listener);
} else if (mediaQuery.removeListener) {
mediaQuery.removeListener(listener);
}
};
}, []);
const hideOverlay = useCallback(() => {
if (overlayTimeoutRef.current !== null && typeof window !== 'undefined') {
window.clearTimeout(overlayTimeoutRef.current);
@@ -247,13 +201,12 @@ export const PhotoCard: React.FC<PhotoCardProps> = ({
// opening the photo. Every visibility toggle below therefore moves
// pointer-events with it, in both the tap-to-reveal and the hover branch.
//
// 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.
// The hover variants are emitted while a mouse is in use, and withheld while
// a finger is. Not behind `md:`: width is the wrong proxy for hover, and a
// mouse user with a window under 768px got no overlay at all. Not behind the
// device's primary pointer either (#1275) — on a hybrid that answers for the
// wrong input. Withholding them 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'
frontend/src/components/gallery/__tests__/PhotoCard.touchTargets.test.tsx
+85 -1
View File
@@ -14,9 +14,10 @@
*/
import React from 'react';
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { render, screen, fireEvent } from '@testing-library/react';
import { render, screen, fireEvent, act } from '@testing-library/react';
import { PhotoCard } from '../PhotoCard';
import { __inputModeTesting } from '../../../hooks/useInputMode';
import type { Photo } from '../../../types';
vi.mock('../../common', () => ({
@@ -65,6 +66,24 @@ function stubTouchDevice(isTouch: boolean) {
// signal — remove it so the pointer-device case is actually exercised.
if (isTouch) (window as any).ontouchstart = null;
else delete (window as any).ontouchstart;
// The input mode is a module-level store now (#1275), so it has to re-read
// the stubbed device rather than keep what it decided on first import.
__inputModeTesting.reset();
}
/**
* Dispatch a real pointer event, the way a finger or a mouse announces itself.
*
* Wrapped in act() because the store notifies outside React's own event
* system: without it the re-render has not landed by the time the following
* click is dispatched, which is precisely the ordering the fix depends on.
*/
function pointer(type: 'pointerdown' | 'pointermove', pointerType: string) {
act(() => {
const event = new Event(type, { bubbles: true }) as any;
event.pointerType = pointerType;
window.dispatchEvent(event);
});
}
/** Class tokens, so `md:group-hover:pointer-events-auto` isn't mistaken for the bare one. */
@@ -194,6 +213,71 @@ describe('PhotoCard overlay hit-testing (#1263)', () => {
expect(tokens(overlayOf(container)).some((c) => c.startsWith('group-hover:'))).toBe(false);
});
it('switches to tap-to-reveal when a finger arrives on a mouse-primary device', () => {
// #1275. A touchscreen laptop reports a fine primary pointer, so before
// this the finger was handled as a click: the photo opened with no reveal
// step and the tile's own actions needed a hover a finger cannot produce.
stubTouchDevice(false);
const onClick = vi.fn();
const { container } = renderCard({ onClick });
const tile = () => container.querySelector('.tile')!;
// Mouse first — one click opens, as it should on this device.
fireEvent.click(tile());
expect(onClick).toHaveBeenCalledTimes(1);
// Now a finger. The tap announces itself before the click lands.
pointer('pointerdown', 'touch');
fireEvent.click(tile());
expect(onClick).toHaveBeenCalledTimes(1); // revealed, did not open
expect(tokens(overlayOf(container))).toContain('pointer-events-auto');
});
it('switches back to one-click open when the mouse returns', () => {
// The other direction, on the same device in the same session: an iPad
// with a trackpad reports a coarse primary pointer, so a mouse click was
// being handled as a tap and opening a photo took two of them.
stubTouchDevice(true);
const onClick = vi.fn();
const { container } = renderCard({ onClick });
const tile = () => container.querySelector('.tile')!;
// Finger: reveal, then open.
pointer('pointerdown', 'touch');
fireEvent.click(tile());
expect(onClick).not.toHaveBeenCalled();
// A mouse approaching is enough — the mode must be right BEFORE the click.
pointer('pointermove', 'mouse');
fireEvent.click(tile());
expect(onClick).toHaveBeenCalledTimes(1);
expect(tokens(overlayOf(container))).toContain('group-hover:opacity-100');
});
it('keeps hidden controls inert in every mode', () => {
// The #1263 guarantee has to survive the mode switching: whichever input
// is in use, a control that cannot be seen cannot be hit.
stubTouchDevice(false);
const { container } = renderCard();
expect(tokens(overlayOf(container))).toContain('pointer-events-none');
pointer('pointerdown', 'touch');
expect(tokens(overlayOf(container))).toContain('pointer-events-none');
pointer('pointermove', 'mouse');
expect(tokens(overlayOf(container))).toContain('pointer-events-none');
});
it('treats a pen like a finger, since it taps rather than hovers', () => {
stubTouchDevice(false);
const onClick = vi.fn();
const { container } = renderCard({ onClick });
pointer('pointerdown', 'pen');
fireEvent.click(container.querySelector('.tile')!);
expect(onClick).not.toHaveBeenCalled();
});
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
frontend/src/components/gallery/layouts/GalleryPremiumLayout.css
+19 -15
View File
@@ -339,24 +339,28 @@
pointer-events: auto;
}
/* Touch devices never get :hover, so the two controls above would be
/* A finger never produces :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;
}
and give them a finger-sized target (#1263).
.gallery-premium-checkbox {
height: 2.25rem;
width: 2.25rem;
}
Keyed off the input in use, not a media query (#1275). `(hover: none) and
(pointer: coarse)` describes the device's PRIMARY pointer, so on a
touchscreen laptop it stayed false and a finger could never reach these,
while on an iPad with a trackpad it stayed true and they were stuck on
permanently. The attribute is set by the layout from useInputMode. */
[data-input-mode='touch'] .gallery-premium-checkbox,
[data-input-mode='touch'] .gallery-premium-like-btn {
opacity: 1;
pointer-events: auto;
}
.gallery-premium-like-btn {
padding: 0.75rem;
}
[data-input-mode='touch'] .gallery-premium-checkbox {
height: 2.25rem;
width: 2.25rem;
}
[data-input-mode='touch'] .gallery-premium-like-btn {
padding: 0.75rem;
}
.gallery-premium-like-btn:hover {
frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx
+6 -1
View File
@@ -24,6 +24,7 @@ import { thumbnailUrlForTile } from '../imageTiers';
import { feedbackService } from '../../../services/feedback.service';
import { PhotoReactions } from '../PhotoReactions';
import { useGuestIdentityOptional } from '../../../contexts/GuestIdentityContext';
import { useInputMode } from '../../../hooks/useInputMode';
import { FeedbackIdentityModal } from '../FeedbackIdentityModal';
import { galleryService } from '../../../services/gallery.service';
import { analyticsService } from '../../../services/analytics.service';
@@ -251,6 +252,7 @@ export const GalleryPremiumLayout: React.FC<GalleryPremiumLayoutProps> = ({
counts: Record<string, number>;
} | null>(null);
const guestIdentity = useGuestIdentityOptional();
const inputMode = useInputMode();
const [showIdentityModal, setShowIdentityModal] = useState(false);
const [pendingLikePhotoId, setPendingLikePhotoId] = useState<number | null>(null);
@@ -492,7 +494,10 @@ export const GalleryPremiumLayout: React.FC<GalleryPremiumLayoutProps> = ({
}
return (
<div className="gallery-premium-layout">
// #1275 — the stylesheet keys its touch rules off this rather than a
// primary-pointer media query, so the checkbox and like button follow the
// input actually in use on a device that has both.
<div className="gallery-premium-layout" data-input-mode={inputMode}>
{/* Hero Section */}
<div className="gallery-premium-hero">
<div
frontend/src/hooks/__tests__/useInputMode.test.tsx
+127
View File
@@ -0,0 +1,127 @@
/**
* The input-mode store (#1275).
*
* The bug it exists to fix is that `matchMedia('(hover: none) and (pointer:
* coarse)')` answers "what is this device's primary pointer", which on
* anything with both inputs is the wrong question. These pin the answer it
* gives instead: whichever input was used last, corrected before the click.
*/
import React from 'react';
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { render, screen, act } from '@testing-library/react';
import { useInputMode, __inputModeTesting } from '../useInputMode';
/** Report a primary pointer, the way a device advertises itself. */
function stubPrimaryPointer(coarse: boolean) {
Object.defineProperty(window, 'matchMedia', {
configurable: true,
writable: true,
value: (query: string) => ({
matches: coarse && query.includes('pointer: coarse'),
media: query,
addEventListener: () => {}, removeEventListener: () => {},
addListener: () => {}, removeListener: () => {},
onchange: null, dispatchEvent: () => false,
}),
});
__inputModeTesting.reset();
}
function pointer(type: 'pointerdown' | 'pointermove', pointerType: string) {
act(() => {
const event = new Event(type, { bubbles: true }) as any;
event.pointerType = pointerType;
window.dispatchEvent(event);
});
}
const Probe: React.FC = () => <span data-testid="mode">{useInputMode()}</span>;
const mode = () => screen.getByTestId('mode').textContent;
beforeEach(() => stubPrimaryPointer(false));
afterEach(() => vi.restoreAllMocks());
describe('useInputMode', () => {
it('starts from the primary pointer, which is right for single-input devices', () => {
render(<Probe />);
expect(mode()).toBe('mouse');
stubPrimaryPointer(true);
render(<Probe />);
expect(screen.getAllByTestId('mode')[1].textContent).toBe('touch');
});
it('follows the input in use, in both directions', () => {
render(<Probe />);
expect(mode()).toBe('mouse');
pointer('pointerdown', 'touch');
expect(mode()).toBe('touch');
pointer('pointermove', 'mouse');
expect(mode()).toBe('mouse');
pointer('pointerdown', 'touch');
expect(mode()).toBe('touch');
});
it('classifies an approaching mouse before it clicks', () => {
// The whole point of listening to pointermove as well: a mouse announces
// itself by moving, and the mode has to be right BEFORE the click, not as
// a consequence of it.
stubPrimaryPointer(true);
render(<Probe />);
expect(mode()).toBe('touch');
pointer('pointermove', 'mouse');
expect(mode()).toBe('mouse');
});
it('groups a pen with touch, since it taps rather than hovers', () => {
render(<Probe />);
pointer('pointerdown', 'pen');
expect(mode()).toBe('touch');
});
it('ignores a pointer type it does not recognise', () => {
render(<Probe />);
pointer('pointerdown', '');
expect(mode()).toBe('mouse');
});
it('shares one mode, and one listener pair, across every subscriber', () => {
// A gallery renders this hook once per tile. The listeners must not scale
// with the tile count, and two tiles must never disagree about the input.
const { unmount } = render(<><Probe /><Probe /><Probe /></>);
expect(__inputModeTesting.subscriberCount()).toBe(3);
pointer('pointerdown', 'touch');
expect(screen.getAllByTestId('mode').map((n) => n.textContent))
.toEqual(['touch', 'touch', 'touch']);
unmount();
expect(__inputModeTesting.subscriberCount()).toBe(0);
});
it('stops listening once nothing is subscribed', () => {
const add = vi.spyOn(window, 'addEventListener');
const remove = vi.spyOn(window, 'removeEventListener');
const { unmount } = render(<Probe />);
const pointerAdds = add.mock.calls.filter(([type]) => String(type).startsWith('pointer'));
expect(pointerAdds.map(([type]) => type).sort()).toEqual(['pointerdown', 'pointermove']);
unmount();
const pointerRemoves = remove.mock.calls.filter(([type]) => String(type).startsWith('pointer'));
expect(pointerRemoves.map(([type]) => type).sort()).toEqual(['pointerdown', 'pointermove']);
});
it('falls back to touch signals where matchMedia is missing', () => {
delete (window as any).matchMedia;
Object.defineProperty(navigator, 'maxTouchPoints', { configurable: true, value: 5 });
__inputModeTesting.reset();
render(<Probe />);
expect(mode()).toBe('touch');
});
});
frontend/src/hooks/useInputMode.ts
+100
View File
@@ -0,0 +1,100 @@
/**
* Which input the visitor is using RIGHT NOW — not what the device is (#1275).
*
* `matchMedia('(hover: none) and (pointer: coarse)')` answers a different
* question: it describes the device's PRIMARY pointer. On anything with both
* — a touchscreen laptop, an iPad with a trackpad, a Surface — one of the two
* inputs is then handled as if it were the other:
*
* - fine-primary + finger: the tap is treated as a click, so a photo opens
* with no tap-to-reveal and the tile's own actions need a hover that a
* finger cannot produce.
* - coarse-primary + mouse: the click is treated as a tap, so opening a
* photo takes two clicks and hovering does nothing.
*
* Pointer events carry the answer per interaction. One window-level listener
* pair feeds a module-level mode that every subscriber shares, so all cards
* agree and the listener count does not scale with the number of tiles.
*
* `pointermove` matters as much as `pointerdown`: a mouse announces itself by
* approaching, and the mode has to be right BEFORE the click lands, not as a
* consequence of it.
*/
import { useSyncExternalStore } from 'react';
export type InputMode = 'touch' | 'mouse';
const COARSE_POINTER_QUERY = '(hover: none) and (pointer: coarse)';
/**
* The reading to start from, before anything has been touched or moved.
* The primary-pointer query is the best guess available at that moment, and it
* is right for the two single-input cases that make up most traffic — a phone
* and a desktop. On a hybrid it is a coin toss that the first real interaction
* corrects.
*/
function initialMode(): InputMode {
if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') {
// No matchMedia (old embedded webviews, some test environments). A
// touchscreen signal is the only thing left to go on.
const hasNavigator = typeof navigator !== 'undefined';
const touchish = (typeof window !== 'undefined' && 'ontouchstart' in window)
|| (hasNavigator && navigator.maxTouchPoints > 0);
return touchish ? 'touch' : 'mouse';
}
return window.matchMedia(COARSE_POINTER_QUERY).matches ? 'touch' : 'mouse';
}
let mode: InputMode = initialMode();
const subscribers = new Set<() => void>();
function setMode(next: InputMode) {
if (next === mode) return;
mode = next;
subscribers.forEach((notify) => notify());
}
function handlePointer(event: PointerEvent) {
// A pen is grouped with touch: it taps rather than hovers on most hardware,
// and being wrong in that direction only costs a reveal step, where being
// wrong the other way puts an action under a pointer that cannot see it.
if (event.pointerType === 'touch' || event.pointerType === 'pen') setMode('touch');
else if (event.pointerType === 'mouse') setMode('mouse');
// Anything else (an unknown or empty pointerType) leaves the mode alone.
}
function subscribe(notify: () => void) {
if (subscribers.size === 0 && typeof window !== 'undefined') {
// Capture phase, so the mode is settled before any component's own handler
// for the same interaction runs. Passive: this never calls preventDefault.
window.addEventListener('pointerdown', handlePointer, { capture: true, passive: true });
window.addEventListener('pointermove', handlePointer, { capture: true, passive: true });
}
subscribers.add(notify);
return () => {
subscribers.delete(notify);
if (subscribers.size === 0 && typeof window !== 'undefined') {
window.removeEventListener('pointerdown', handlePointer, { capture: true });
window.removeEventListener('pointermove', handlePointer, { capture: true });
}
};
}
const getSnapshot = () => mode;
// The server has no pointer; 'mouse' keeps hover markup in the initial HTML.
const getServerSnapshot = (): InputMode => 'mouse';
/** The input in use right now. Re-renders the caller when it changes. */
export function useInputMode(): InputMode {
return useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot);
}
export const __inputModeTesting = {
/** Re-read the device and drop any interaction history. */
reset() {
mode = initialMode();
subscribers.forEach((notify) => notify());
},
current: () => mode,
subscriberCount: () => subscribers.size,
};