Skip to content

Commit e5a625d

Browse files
committed
fix: prevent synthesised click from closing overlays opened by touch
Move onTap's touchFired guard from per-element to module-level scope. After any touchend, all onTap click handlers are suppressed for 400ms, preventing the browser's synthesised click from hitting newly-visible overlays at the same coordinates. Removes drawer's justOpened guard — now redundant.
1 parent b0f78a2 commit e5a625d

5 files changed

Lines changed: 110 additions & 36 deletions

File tree

src/drawer/drawer.ts

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ export function createDrawer(
4242
drawer.appendChild(grid)
4343

4444
let drawerOpen = false
45-
let justOpened = false
4645

4746
for (const buttonDef of buttons) {
4847
const button = el('button')
@@ -95,14 +94,6 @@ export function createDrawer(
9594
backdrop.style.display = 'block'
9695
drawer.classList.add('open')
9796
drawerOpen = true
98-
// Guard against synthesised click on backdrop. When onTap fires on
99-
// touchend (no preventDefault), the browser synthesises mousedown/click
100-
// ~4ms later at the same coordinates. By then the backdrop is visible
101-
// and intercepts the click, immediately closing the drawer.
102-
justOpened = true
103-
setTimeout(() => {
104-
justOpened = false
105-
}, 300)
10697
}
10798

10899
function close(): void {
@@ -115,20 +106,6 @@ export function createDrawer(
115106
return drawerOpen
116107
}
117108

118-
// Backdrop dismisses drawer. The click listener ignores the first click
119-
// after open() to avoid synthesised click from the triggering touch.
120-
// touchend is never synthesised, so it always works immediately.
121-
backdrop.addEventListener(
122-
'click',
123-
(e: Event) => {
124-
if (justOpened) {
125-
justOpened = false
126-
e.stopImmediatePropagation()
127-
}
128-
},
129-
{ capture: true },
130-
)
131-
132109
onTap(backdrop, () => {
133110
const kbWasOpen = isKeyboardOpen()
134111
haptic()

src/util/tap.ts

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,48 @@
11
/**
22
* Register a tap handler that works on both touch and non-touch devices.
33
*
4-
* iOS Safari can fail to synthesise `click` after touch events on dynamically
5-
* created elements. This listens for `touchend` (fires on finger lift — correct
6-
* button press-then-release UX) and `click` (fallback for mouse/keyboard).
7-
* A guard prevents double-fire when both events occur.
4+
* ## Why module-level touchFired (not per-element)
5+
*
6+
* On touch devices, after touchend fires, the browser synthesises a click
7+
* ~4ms later at the same coordinates. When a touchend handler opens an
8+
* overlay (higher z-index) at those coordinates, the synthesised click
9+
* hit-tests against the overlay instead of the original button — closing
10+
* it immediately. A per-element guard only prevents double-fire on the
11+
* SAME element; module-level state prevents cross-element synthesised clicks.
12+
*
13+
* ## Why not preventDefault() on touchend
14+
*
15+
* The W3C Touch Events spec says preventDefault() on touchend suppresses
16+
* synthesised mouse events — the "correct" solution. But it was removed
17+
* (d40fa46) because without synthesised mousedown, focus stays on the
18+
* terminal textarea and Android re-shows the keyboard when toolbar buttons
19+
* are pressed. Restoring it would require blur() + reworking keyboard state
20+
* preservation (isKeyboardOpen/conditionalFocus) across 13 call sites.
21+
*
22+
* ## Why not Pointer Events (pointerup)
23+
*
24+
* preventDefault() on pointerup does NOT suppress compatibility mouse
25+
* events per the Pointer Events spec. The browser still synthesises
26+
* mousedown/mouseup/click from the underlying touch events.
827
*/
9-
export function onTap(element: HTMLElement, handler: (e: Event) => void): void {
10-
let touchFired = false
1128

29+
let touchFired = false
30+
let touchGuardTimer: ReturnType<typeof setTimeout> | null = null
31+
32+
export function onTap(element: HTMLElement, handler: (e: Event) => void): void {
1233
element.addEventListener('touchend', (e: TouchEvent) => {
1334
// No preventDefault — allow the browser to synthesise mousedown/click,
1435
// which transfers focus to the button and away from the terminal textarea.
1536
// Without this, Android re-shows the keyboard when buttons are pressed.
16-
// The touchFired guard below prevents the handler from double-firing.
37+
// The module-level touchFired guard below prevents the handler from
38+
// double-firing on this element AND prevents cross-element synthesised
39+
// clicks from closing overlays that just opened at the same coordinates.
1740
touchFired = true
1841
handler(e)
19-
setTimeout(() => {
42+
if (touchGuardTimer !== null) clearTimeout(touchGuardTimer)
43+
touchGuardTimer = setTimeout(() => {
2044
touchFired = false
45+
touchGuardTimer = null
2146
}, 400)
2247
})
2348

@@ -26,3 +51,12 @@ export function onTap(element: HTMLElement, handler: (e: Event) => void): void {
2651
handler(e)
2752
})
2853
}
54+
55+
/** Reset module-level touch guard state — test-only. */
56+
export function _resetTouchGuard(): void {
57+
touchFired = false
58+
if (touchGuardTimer !== null) {
59+
clearTimeout(touchGuardTimer)
60+
touchGuardTimer = null
61+
}
62+
}

tests/integration.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { createHelpOverlay } from '../src/controls/help'
88
import { createDrawer } from '../src/drawer/drawer'
99
import { createHookRegistry } from '../src/hooks/registry'
1010
import { createToolbar } from '../src/toolbar/toolbar'
11+
import { _resetTouchGuard } from '../src/util/tap'
1112
import { mockTerminal } from './fixtures'
1213

1314
beforeEach(() => {
@@ -138,6 +139,10 @@ describe('font controls integration', () => {
138139
})
139140

140141
describe('help overlay integration', () => {
142+
beforeEach(() => {
143+
_resetTouchGuard()
144+
})
145+
141146
test('creates help overlay', () => {
142147
const term = mockTerminal()
143148
const { helpButton } = createFontControls(term, defaultConfig.font)
@@ -178,6 +183,28 @@ describe('help overlay integration', () => {
178183
expect(element.querySelector('.wt-help-version')).toBeNull()
179184
})
180185

186+
test('synthesised click after touchend does not immediately close overlay', () => {
187+
const term = mockTerminal()
188+
const { element: fontControls, helpButton } = createFontControls(term, defaultConfig.font)
189+
const { element: overlay } = createHelpOverlay(term, helpButton, defaultConfig)
190+
191+
document.body.appendChild(fontControls)
192+
document.body.appendChild(overlay)
193+
194+
// Simulate touch on ? button — touchend opens the overlay
195+
helpButton.dispatchEvent(new Event('touchend', { bubbles: true }))
196+
expect(overlay.style.display).toBe('block')
197+
198+
// Browser synthesises click ~4ms later. On a real device the overlay
199+
// (higher z-index) now covers the ? button area, so hit-testing targets
200+
// the overlay element itself. Simulate this by dispatching click on the
201+
// overlay with target === overlay.
202+
overlay.dispatchEvent(new Event('click', { bubbles: true }))
203+
204+
// Overlay should still be open — the synthesised click must be ignored
205+
expect(overlay.style.display).toBe('block')
206+
})
207+
181208
test('renders configured button descriptions and no stale Claude section', () => {
182209
const term = mockTerminal()
183210
const { helpButton } = createFontControls(term, defaultConfig.font)

tests/playwright/touch.spec.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,8 @@ test('drawer open → close → re-open cycle', async ({ page }) => {
9393
test('synthesised click from tap() hits backdrop (regression guard)', async ({ page }) => {
9494
// Proves the mechanism that caused the open-then-close bug still exists:
9595
// after touchend opens the drawer, synthesised mousedown/click land on the
96-
// backdrop. The justOpened guard in drawer.ts must block these.
97-
// Listen at document level (capture phase) because the fix uses
98-
// stopImmediatePropagation on the backdrop element.
96+
// backdrop. The module-level touch guard in tap.ts suppresses the onTap
97+
// click handler, so the drawer stays open despite the click reaching it.
9998
await page.evaluate(() => {
10099
const w = window as unknown as { __backdropClicks: { isTrusted: boolean }[] }
101100
w.__backdropClicks = []

tests/tap.test.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1-
import { describe, expect, test, vi } from 'vitest'
2-
import { onTap } from '../src/util/tap'
1+
import { beforeEach, describe, expect, test, vi } from 'vitest'
2+
import { _resetTouchGuard, onTap } from '../src/util/tap'
33

44
describe('onTap', () => {
5+
beforeEach(() => {
6+
_resetTouchGuard()
7+
})
8+
59
test('fires handler on click', () => {
610
const element = document.createElement('button')
711
const handler = vi.fn()
@@ -50,4 +54,37 @@ describe('onTap', () => {
5054

5155
vi.useRealTimers()
5256
})
57+
58+
test('touchend on element A suppresses click on element B', () => {
59+
const a = document.createElement('button')
60+
const b = document.createElement('button')
61+
const handlerA = vi.fn()
62+
const handlerB = vi.fn()
63+
onTap(a, handlerA)
64+
onTap(b, handlerB)
65+
66+
a.dispatchEvent(new TouchEvent('touchend'))
67+
b.dispatchEvent(new Event('click'))
68+
69+
expect(handlerA).toHaveBeenCalledOnce()
70+
expect(handlerB).not.toHaveBeenCalled()
71+
})
72+
73+
test('cross-element click works after guard timeout', () => {
74+
vi.useFakeTimers()
75+
const a = document.createElement('button')
76+
const b = document.createElement('button')
77+
const handlerA = vi.fn()
78+
const handlerB = vi.fn()
79+
onTap(a, handlerA)
80+
onTap(b, handlerB)
81+
82+
a.dispatchEvent(new TouchEvent('touchend'))
83+
vi.advanceTimersByTime(400)
84+
b.dispatchEvent(new Event('click'))
85+
86+
expect(handlerB).toHaveBeenCalledOnce()
87+
88+
vi.useRealTimers()
89+
})
5390
})

0 commit comments

Comments
 (0)