diff --git a/frontend/src/components/gallery/PhotoCard.tsx b/frontend/src/components/gallery/PhotoCard.tsx index 24c16d5f..5e27a33b 100644 --- a/frontend/src/components/gallery/PhotoCard.tsx +++ b/frontend/src/components/gallery/PhotoCard.tsx @@ -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 = ({ }) => { 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(null); // Self-managed identity modal state (identityMode === 'self') @@ -143,31 +122,6 @@ export const PhotoCard: React.FC = ({ 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 = ({ // 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' diff --git a/frontend/src/components/gallery/__tests__/PhotoCard.touchTargets.test.tsx b/frontend/src/components/gallery/__tests__/PhotoCard.touchTargets.test.tsx index b8f39517..fe777835 100644 --- a/frontend/src/components/gallery/__tests__/PhotoCard.touchTargets.test.tsx +++ b/frontend/src/components/gallery/__tests__/PhotoCard.touchTargets.test.tsx @@ -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 diff --git a/frontend/src/components/gallery/layouts/GalleryPremiumLayout.css b/frontend/src/components/gallery/layouts/GalleryPremiumLayout.css index c6a2ef92..181e80ed 100644 --- a/frontend/src/components/gallery/layouts/GalleryPremiumLayout.css +++ b/frontend/src/components/gallery/layouts/GalleryPremiumLayout.css @@ -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 { diff --git a/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx b/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx index c45ec555..e2dedbb9 100644 --- a/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx +++ b/frontend/src/components/gallery/layouts/GalleryPremiumLayout.tsx @@ -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 = ({ counts: Record; } | null>(null); const guestIdentity = useGuestIdentityOptional(); + const inputMode = useInputMode(); const [showIdentityModal, setShowIdentityModal] = useState(false); const [pendingLikePhotoId, setPendingLikePhotoId] = useState(null); @@ -492,7 +494,10 @@ export const GalleryPremiumLayout: React.FC = ({ } return ( -
+ // #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. +
{/* Hero Section */}
({ + 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 = () => {useInputMode()}; +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(); + expect(mode()).toBe('mouse'); + + stubPrimaryPointer(true); + render(); + expect(screen.getAllByTestId('mode')[1].textContent).toBe('touch'); + }); + + it('follows the input in use, in both directions', () => { + render(); + 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(); + expect(mode()).toBe('touch'); + + pointer('pointermove', 'mouse'); + expect(mode()).toBe('mouse'); + }); + + it('groups a pen with touch, since it taps rather than hovers', () => { + render(); + pointer('pointerdown', 'pen'); + expect(mode()).toBe('touch'); + }); + + it('ignores a pointer type it does not recognise', () => { + render(); + 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(<>); + 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(); + 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(); + expect(mode()).toBe('touch'); + }); +}); diff --git a/frontend/src/hooks/useInputMode.ts b/frontend/src/hooks/useInputMode.ts new file mode 100644 index 00000000..8a095c7b --- /dev/null +++ b/frontend/src/hooks/useInputMode.ts @@ -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, +};