fix(core): stop popup triggers from fighting their own light dismiss (#5004)

[cixzhang]

Fixes the trigger side of #5004.

The mechanism

A light dismiss and the trigger's own click are one press. The browser dismisses an auto popover on pointerup and queues the toggle event; the click follows a beat later. Which of the two React sees first is a race:

order	what the trigger's handler reads	result
click first, then toggle	still open	closes — correct, by luck
toggle first, then click	already closed	reopens — the button looks like it does nothing

DropdownMenu and Popover each carried a private 50 ms timing guard against this; Selector, MultiSelector and ComplexSelector carried nothing. A guard is also the wrong shape: a window is a guess about how long the two halves of one press can be apart, and the case where the race is lost — a blocked main thread — is exactly the case where the window is too short. I saw that directly: at 50 ms the new tests pass alone and fail when the suites run in parallel.

The fix

Count gestures, don't time them. A tiny module counts pointerdown and keydown on the document; the layer records which gesture the browser dismissed it in, and wasJustDismissed() is true only while that same gesture is still in flight. A click from the dismissing press is absorbed however long the main thread was blocked; a deliberate second press is a new gesture and always acts. No constant to tune. Both hand-rolled copies collapse into it.

The same dismissal also fires for controls that sit on the trigger — the clear ✕ and the status button. They live outside the popover, so pressing one dismissed the popup, which means the affordance and the popup it belongs to could never be used together. keepOpenProps names such a control an invoker of the popover, which puts it inside the layer for that decision. The attribute is stamped for the press only: a permanent invoker reports itself as expanded to assistive tech, which I confirmed in Chrome's AX tree — a Clear button must not.

Test plan

Unit — 8 new tests, all red against unmodified source (verified by restoring the source files from main and rerunning): the trigger click absorbed when the dismissal lands first, a deliberate second press still acting, a programmatic hide left unguarded, the invoker stamped for the press and removed after, the invoker's own toggle cancelled, and one regression test each for Selector, MultiSelector and ComplexSelector driving the losing order directly. packages/core: 6318 passing (the one failure is the Table 500-row perf budget, which flakes under parallel load here and passes in isolation).

Real browsers — Playwright, Chromium and WebKit iPhone touch, pressing the clear ✕ and the status button of an open menu:

	before	after
MultiSelector clear ✕	menu closes	menu stays open, value cleared
Selector clear ✕ (placement="below")	menu closes	menu stays open
Selector status button	menu closes	menu stays open

Trigger toggling is unchanged in both engines: one press, one state change, and the third press reopens.

On the race itself: I could not make Chromium or WebKit lose it on this machine — both consistently deliver the click first, which is why the bug reads as intermittent and environment-dependent. The regression tests therefore drive the losing order explicitly rather than hoping for it. If you can reproduce the reopen somewhere concrete, that environment is the real check on this.

Notes

• useSelectedItemOffset and the overlay geometry are untouched here.
• Other clearable inputs that render their own popup — Typeahead, Tokenizer, DateInput, DateRangeInput, TimeInput — have the same shape and are not wired here; their clear buttons should take keepOpenProps too once someone confirms which of them can have a clear button visible with the popup open.
• With the default selected-item overlay, Selector's clear and status buttons are covered by the menu entirely, so this fix is not visible there — that is the geometry half of #5004, left alone deliberately.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
astryx	Ready	Preview	Aug 25, 2026 4:22am

[github-actions]

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

Modified Components

ComplexSelector _{(@astryxdesign/core)} · View in Storybook

Metric	Before	After	Delta
Bundle Size (ESM)	N/A	N/A	N/A
Lines of Code	N/A	457	-
Complexity	N/A	Very High (41)	-

DropdownMenu _{(@astryxdesign/core)} · View in Storybook

Metric	Before	After	Delta
Bundle Size (ESM)	N/A	N/A	N/A
Lines of Code	N/A	1409	-
Complexity	N/A	Very High (152)	-

Field _{(@astryxdesign/core)} · View in Storybook

Metric	Before	After	Delta
Bundle Size (ESM)	N/A	N/A	N/A
Lines of Code	N/A	764	-
Complexity	N/A	Very High (48)	-

Layer _{(@astryxdesign/core)} · View in Storybook

Metric	Before	After	Delta
Bundle Size (ESM)	N/A	N/A	N/A
Lines of Code	N/A	1301	-
Complexity	N/A	Very High (95)	-

MultiSelector _{(@astryxdesign/core)} · View in Storybook

Metric	Before	After	Delta
Bundle Size (ESM)	N/A	N/A	N/A
Lines of Code	N/A	1511	-
Complexity	N/A	Very High (188)	-

Popover _{(@astryxdesign/core)} · View in Storybook

Metric	Before	After	Delta
Bundle Size (ESM)	N/A	N/A	N/A
Lines of Code	N/A	562	-
Complexity	N/A	Very High (52)	-

Selector _{(@astryxdesign/core)} · View in Storybook

Metric	Before	After	Delta
Bundle Size (ESM)	N/A	N/A	N/A
Lines of Code	N/A	1607	-
Complexity	N/A	Very High (168)	-

Bundle Size Summary

Package	Size (ESM)	Size (CJS)	Gzipped
@astryxdesign/core	N/A	4.8KB	1.2KB

Accessibility Audit

Status: No accessibility violations detected.

---

_{Generated by PR Enrichment workflow | Storybook | Sandbox | View full report}

[cixzhang]

Thanks — the gesture counter is the right primitive, and having one place that knows a dismissal just happened is a good direction.

Needs a merge with main; #4769 landed today and this will conflict.

Three things:

The guard only reaches callers that go through toggle(). Anything calling show()/hide() directly bypasses it — ComplexSelector is the example after the merge, and its new test passes off its own leftover 50ms timer rather than this change. Guarding in show() would make it hold for every caller instead of being something each one has to opt into.

dismissedByGestureRef is never cleared (useLayer.tsx:485), so a click with no pointerdown before it — AT activation, .click() — is read as part of the dismissing gesture indefinitely. Make it one-shot.

keepOpenProps isn't finished. KeepLayerOpenProps is unexported, InputClearButton grew two ad-hoc handler props, and five clearable inputs are unwired — let's keep it internal until Typeahead and the date/time inputs are done rather than ship a half-built API.

[Reviewed by Robohands and Cindy]

[cixzhang]

Merged main and rewrote the guard against your three points. Pushed to this branch (4947e62).

The guard moved into show(). Every caller lands there, so a trigger no longer opts in — wasJustDismissed() is checked before the layer opens, whatever route the caller took. ComplexSelector's private 50 ms timer is deleted with it. Its two tests now fail when I remove the show() guard and pass with it, so they are testing this change and not the timer.

The dismissal is one-shot. It is spent by the click that ends the press it came from — a one-shot bubble-phase click listener on the document, so every guard reading it has already run. A click with no press of its own (AT activation, element.click()) therefore acts. New test: acts on a synthesized click with no press of its own, red without the clear.

One more defect the guard exposed. hide() called hidePopover() while isOpenRef still read true, so a toggle event delivered synchronously was read as a browser dismissal — a programmatic hide stamped the gesture. The ref is cleared before the call now, which is what the comment above the reconciler already claimed. DropdownMenu's old race test was passing off exactly that bug (three clicks, no pointerdown, no light dismiss); it is rewritten to drive the losing order, plus a second test that a press of its own re-opens.

Real Chromium

Playwright, trusted presses, pnpm storybook. Chromium delivers click before toggle on both mouse and touch (pointerdown → click → toggle:closed), so it wins the race on its own — as you found. To test the losing order I hand React the dismissal first, from a real press: a bubble-phase pointerdown listener dispatches toggle: closed on the open popover, then the press's own click arrives. Before/after is the same browser, same driver, main's files swapped in for "before":

trigger press while open	before	after
DropdownMenu	closes (its own 50 ms timer)	closes
Popover	re-opens	closes
Selector	re-opens	closes
MultiSelector	re-opens	closes
ComplexSelector	closes (its own 50 ms timer)	closes

Also in Chromium, with the fix: third press re-opens everywhere; a synthesized .click() after a dismissing press opens the menu; an outside press dismisses; MultiSelector's clear ✕ clears the value with the menu still open.

Tests

Layer, Popover, DropdownMenu, Selector, MultiSelector, ComplexSelector, Field, DateInput, Typeahead, Tooltip, HoverCard, Dialog, CommandPalette — 1011 passing. Full packages/core is green apart from DateInputTouch timeouts under parallel load, which reproduce identically on the merge commit before this change (they pass in isolation, 133/133). Typecheck clean, lint 0 errors.

Against the shared dismissal stack

Merging this branch into #4881 auto-merges usePopover.tsx cleanly — the one file both touch. That merge does conflict in Layer/index.ts, Tooltip/useTooltip.tsx and useHoverCard.tsx, but those conflicts are #4881 against today's main and appear with this branch left out. Nothing here is subsumed or duplicated either: #4881 routes Escape and the close-watcher through a stack of layers, and pointer light dismiss stays the browser's, so the toggle this guard counts on still fires. Order can be whichever suits you.

Not done

keepOpenProps is untouched — your third point (unexported type, two ad-hoc props on InputClearButton, five unwired inputs) is an API-scope call, and I did not want to make it on your behalf inside a correctness fix.

[Pushed by Robohands]

[cixzhang]

Thanks — the two you took from last round hold up. In Chromium, with the dismissal handed to React before the click, main re-opens the menu and this closes it:

	before	after
press the trigger of an open menu, dismissal first
press the clear ✕ with the menu open
press the status ⚠ with the menu open

One to fix before it lands: after a touch the browser cancels — a press turned into a scroll, a long-press — Chrome's a11y tree has the clear ✕ as an expanded pop-up button, the state your note rules out. keepOpenProps takes the popovertarget off on pointerup only (useLayer.tsx:691), and a press has two ends. Listen for pointercancel too; the four other press cleanups in core do (ResizeHandle.tsx:439, useSheetGestures.ts).

keepOpenProps is still your call from last round; I'd keep it internal until the date/time inputs are wired. Send the guard on its own once pointercancel is covered and it lands today — does the invoker half want to wait for them?

MERGE: hold(new public surface on a shipped hook — your call; plus one a11y fix outstanding)

[Reviewed by Robohands]

[cixzhang]

Fixed the pointercancel one in e6dcd04. keepOpenProps now hangs one cleanup off both ends of the press, so whichever arrives first takes popovertarget off and removes both listeners — the shape ResizeHandle.tsx, useTableColumnResize.tsx, usePointerDragScroll.ts and useSheetGestures.ts already use.

Driven the way you found it: CDP touchStart → touchCancel, no touchEnd, on the clear ✕ of an open MultiSelector, read out of Chrome's own AX tree.

before  role=button  name="Clear all Technologies"
        props=["invalid=false","focusable=true","expanded=true","details="]   popovertarget=_r_9_
after   role=button  name="Clear all Technologies"
        props=["invalid=false","focusable=true"]                              popovertarget absent

Both sides still report expanded=true mid-press, which is the point of the stamp; only the cancelled press used to keep it. An ordinary tap is unchanged — value cleared, menu still open. New unit test for the cancel path (red without the fix), and 589 tests across Layer, Selector, MultiSelector, Popover, DropdownMenu, ComplexSelector and Field pass.

Chromium only: WebKit will not launch on this machine today, and WebKit is the engine most likely to claim the gesture, so that row is still unverified.

keepOpenProps as public surface is untouched here — still the open question.