From d115f8d9606930b453ad8ca97fa7d6d4083503a5 Mon Sep 17 00:00:00 2001 From: weishu Date: Mon, 27 Jul 2026 12:59:40 +0800 Subject: [PATCH] fix(web): stop touch taps from double-firing session navigation (#1185) useLongPress binds both touch and mouse handlers. After a tap, touch browsers emit compatibility mouse events (~300ms later) that the page did not preventDefault, so onClick fired twice: once from touchend, once from the synthesized mouseup. On the wide tablet sidebar layout the list stays under the finger, so the second onClick lands on whatever row slid into that position and navigates to the wrong session. preventDefault() on touchend for every handled tap, and additionally swallow mouse events that arrive within 700ms of a touch so browsers that still dispatch the compatibility sequence cannot re-trigger onClick. Based on the fix by RiriAgent in the fork (commits 1bbfcc20, 5e3d135a). Co-authored-by: RiriAgent <39219425+RiriAgent@users.noreply.github.com> --- web/src/hooks/useLongPress.test.tsx | 120 ++++++++++++++++++++++++++++ web/src/hooks/useLongPress.ts | 36 +++++++-- 2 files changed, 150 insertions(+), 6 deletions(-) create mode 100644 web/src/hooks/useLongPress.test.tsx diff --git a/web/src/hooks/useLongPress.test.tsx b/web/src/hooks/useLongPress.test.tsx new file mode 100644 index 00000000..de55c440 --- /dev/null +++ b/web/src/hooks/useLongPress.test.tsx @@ -0,0 +1,120 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, fireEvent, render } from '@testing-library/react' +import { useLongPress } from './useLongPress' + +function Probe(props: { onClick: () => void; onLongPress?: () => void }) { + const handlers = useLongPress({ + onClick: props.onClick, + onLongPress: props.onLongPress ?? (() => {}), + }) + return ( + + ) +} + +describe('useLongPress', () => { + let now = 10_000 + + beforeEach(() => { + vi.useFakeTimers() + // Start well past the ghost-mouse window so a plain mouse tap (no prior + // touch, lastTouchAt = 0) is not mistaken for a touch-synthesized event. + now = 10_000 + vi.spyOn(Date, 'now').mockImplementation(() => now) + }) + + afterEach(() => { + cleanup() + vi.useRealTimers() + }) + + it('fires onClick once for a mouse tap', () => { + const onClick = vi.fn() + const { getByTestId } = render() + const row = getByTestId('row') + + fireEvent.mouseDown(row, { button: 0, clientX: 10, clientY: 10 }) + fireEvent.mouseUp(row, { button: 0, clientX: 10, clientY: 10 }) + + expect(onClick).toHaveBeenCalledTimes(1) + }) + + it('fires onClick once for a touch tap (ignores the browser-synthesized mouse events that follow)', () => { + // Real touch browsers (Android Chrome, etc.) emit a compatibility + // mousedown/mouseup/click ~300ms after touchend for any touch the page + // did not preventDefault. Because useLongPress binds BOTH touch and + // mouse handlers, those synthesized events must not trigger a second + // onClick — otherwise a tap navigates twice (and the second navigation + // lands on whatever row slid under the finger meanwhile). + const onClick = vi.fn() + const { getByTestId } = render() + const row = getByTestId('row') + + fireEvent.touchStart(row, { touches: [{ clientX: 10, clientY: 10 }] }) + const touchEndPrevented = !fireEvent.touchEnd(row, { changedTouches: [{ clientX: 10, clientY: 10 }] }) + + // Browser-synthesized compatibility mouse events for the same tap, + // ~300ms later. + act(() => { + now += 300 + vi.advanceTimersByTime(300) + }) + fireEvent.mouseDown(row, { button: 0, clientX: 10, clientY: 10 }) + fireEvent.mouseUp(row, { button: 0, clientX: 10, clientY: 10 }) + + expect(touchEndPrevented).toBe(true) + expect(onClick).toHaveBeenCalledTimes(1) + }) + + it('does not fire onClick when the touch moved (scroll gesture)', () => { + const onClick = vi.fn() + const { getByTestId } = render() + const row = getByTestId('row') + + fireEvent.touchStart(row, { touches: [{ clientX: 10, clientY: 10 }] }) + fireEvent.touchMove(row, { touches: [{ clientX: 10, clientY: 40 }] }) + fireEvent.touchEnd(row, { changedTouches: [{ clientX: 10, clientY: 40 }] }) + + expect(onClick).not.toHaveBeenCalled() + }) + + it('still fires onLongPress (and not onClick) for a touch long-press', () => { + const onClick = vi.fn() + const onLongPress = vi.fn() + const { getByTestId } = render() + const row = getByTestId('row') + + fireEvent.touchStart(row, { touches: [{ clientX: 10, clientY: 10 }] }) + act(() => { + now += 500 + vi.advanceTimersByTime(500) + }) + fireEvent.touchEnd(row, { changedTouches: [{ clientX: 10, clientY: 10 }] }) + + expect(onLongPress).toHaveBeenCalledTimes(1) + expect(onClick).not.toHaveBeenCalled() + }) + + it('still honors a genuine mouse click well after a touch', () => { + const onClick = vi.fn() + const { getByTestId } = render() + const row = getByTestId('row') + + // A touch interaction happens first. + fireEvent.touchStart(row, { touches: [{ clientX: 10, clientY: 10 }] }) + fireEvent.touchEnd(row, { changedTouches: [{ clientX: 10, clientY: 10 }] }) + expect(onClick).toHaveBeenCalledTimes(1) + + // Much later, a real mouse interaction must still work (hybrid devices). + act(() => { + now += 1_000 + vi.advanceTimersByTime(1_000) + }) + fireEvent.mouseDown(row, { button: 0, clientX: 10, clientY: 10 }) + fireEvent.mouseUp(row, { button: 0, clientX: 10, clientY: 10 }) + + expect(onClick).toHaveBeenCalledTimes(2) + }) +}) diff --git a/web/src/hooks/useLongPress.ts b/web/src/hooks/useLongPress.ts index 5673b2d7..4e9d372d 100644 --- a/web/src/hooks/useLongPress.ts +++ b/web/src/hooks/useLongPress.ts @@ -8,6 +8,11 @@ type UseLongPressOptions = { disabled?: boolean } +// How long after a touch interaction to keep ignoring synthesized mouse +// events. Android's compatibility mouse events fire ~300ms after touchend; +// 700ms covers that with margin without affecting genuine later mouse input. +const GHOST_MOUSE_WINDOW_MS = 700 + type UseLongPressHandlers = { onMouseDown: React.MouseEventHandler onMouseUp: React.MouseEventHandler @@ -26,6 +31,13 @@ export function useLongPress(options: UseLongPressOptions): UseLongPressHandlers const isLongPressRef = useRef(false) const touchMoved = useRef(false) const pressPointRef = useRef<{ x: number; y: number }>({ x: 0, y: 0 }) + // Timestamp of the most recent touch interaction. Touch browsers emit + // compatibility mouse events (mousedown/mouseup/click) after a tap for any + // touch the page did not preventDefault. Since we bind BOTH touch and + // mouse handlers, those "ghost" mouse events would fire onClick a second + // time — on a persistent list (tablet sidebar) the second click lands on + // whatever row slid under the finger, navigating to the wrong session. + const lastTouchAtRef = useRef(0) const clearTimer = useCallback(() => { if (timerRef.current) { @@ -59,28 +71,40 @@ export function useLongPress(options: UseLongPressOptions): UseLongPressHandlers touchMoved.current = false }, [clearTimer, onClick]) + // True when a mouse event is actually a touch-synthesized compatibility + // event firing right after a tap; such events must not re-trigger onClick. + const isGhostMouseEvent = useCallback( + () => Date.now() - lastTouchAtRef.current < GHOST_MOUSE_WINDOW_MS, + [] + ) + const onMouseDown = useCallback((e) => { if (e.button !== 0) return + if (isGhostMouseEvent()) return startTimer(e.clientX, e.clientY) - }, [startTimer]) + }, [startTimer, isGhostMouseEvent]) const onMouseUp = useCallback(() => { + if (isGhostMouseEvent()) return handleEnd(!isLongPressRef.current) - }, [handleEnd]) + }, [handleEnd, isGhostMouseEvent]) const onMouseLeave = useCallback(() => { + if (isGhostMouseEvent()) return handleEnd(false) - }, [handleEnd]) + }, [handleEnd, isGhostMouseEvent]) const onTouchStart = useCallback((e) => { + lastTouchAtRef.current = Date.now() const touch = e.touches[0] startTimer(touch.clientX, touch.clientY) }, [startTimer]) const onTouchEnd = useCallback((e) => { - if (isLongPressRef.current) { - e.preventDefault() - } + lastTouchAtRef.current = Date.now() + // Prevent the browser's compatibility mouse/click sequence from firing + // on the row that ends up under the finger after navigation/reordering. + e.preventDefault() handleEnd(!isLongPressRef.current) }, [handleEnd])