Skip to content

fix(overlays): keep a Dialog open when a closing popup is pressed - #1467

Merged
tenphi merged 4 commits into
mainfrom
andrew/cub-5293-picking-an-option-from-a-fading-dropdown-inside-a-dialog
Oct 5, 2026
Merged

tenphi merged 4 commits into
mainfrom
andrew/cub-5293-picking-an-option-from-a-fading-dropdown-inside-a-dialog

Conversation

@tenphi

@tenphi tenphi commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Describe changes

Fixes CUB-5293: clicking an option in a list that is fading out, inside a Dialog, closed the whole Dialog and lost the form.

Cause. Our overlays stay mounted, and clickable, through their exit transition, but React Aria takes a closed overlay off its visible-overlay stack at once. A press on a closing list therefore counted as a press outside the Dialog, which was topmost again, and dismissed it. A list closes on its own when the content around its trigger scrolls (useCloseOnScroll), so a trackpad's momentum or a test runner scrolling a field into view was enough.

Fix. A closing overlay is marked data-react-aria-top-layer, which React Aria never treats as "outside". The press now reaches the list, so the clicked option is picked.

  • DisplayTransition's render prop gets a new isExiting arg: true from the render where the isShown prop turns false until the exit finishes. It's needed because phase (and the render arg isShown) still report the shown state for two frames after close, and preserveContent freezes every value the render prop closes over, so there was no fresh way to tell a closing overlay from an open one.

  • Overlay (Popover, Modal, Tray: menus, Picker, FilterPicker, popover and modal dialogs), ListBoxPopover (ComboBox, SearchComboBox, TagInput, CommandTextArea) and Select mark themselves top layer while isExiting.

  • Modal and Tray now use useOverlayEscapeGuard, like Popover and Select. Pressing a closing nested modal leaves focus inside it, and its raw useOverlay handler would otherwise swallow the next Escape meant for the Dialog underneath.

  • Toasts and notifications: their container is portaled outside any open Dialog, so pressing a toast or notification action closed the Dialog and the action never ran (unrelated to exit transitions). The container is now permanently data-react-aria-top-layer, as React Aria's own toast region is. That also keeps it reachable by focus from a Dialog's contained scope, and visible to screen readers while a modal hides the rest of the page.

pointer-events: none during exit was rejected: a list hanging over the Dialog's backdrop would let the click through to the backdrop and still close the Dialog.

Once this ships, the console-ui workaround in cubedevinc/cubejs-enterprise#15818 (useDialogListPressGuard) can go.

Verified

  • New browser tests in DialogTrigger.browser.test.tsx (FilterPicker, Select, ComboBox, nested modal Dialog, Escape after pressing a closing nested modal or tray) and Notifications.browser.test.tsx (toast and notification actions over a Dialog). Each fails on main and passes here; reverting any one source file fails only its own case.
  • A scratch test with the issue's exact scenario (a DialogForm whose content really scrolls, a real scrollTop change, a real Playwright click on the fading option) fails on main and passes here for FilterPicker, Select and Picker. The form keeps its typed value and the option is picked.
  • isExiting unit test in DisplayTransition.test.tsx.
  • pnpm lint, pnpm diagnostics:compiler, pnpm audit-docs --component=DisplayTransition pass. tsc has no errors in changed files (the 22 pre-existing ones are elsewhere). The jsdom suite passes; under very high machine load some unrelated tests timed out and passed on rerun.
Checklist
  • Tests are added (including unit tests and stories in the storybook)
  • Tests are passed successfully
  • Changeset(s) is(are) added
  • Commit message follows commit guidelines

Closes: CUB-5293

🤖 Generated with Claude Code

tenphi and others added 2 commits October 5, 2026 15:57
A closed overlay leaves React Aria's visible-overlay stack at once but
stays on screen and clickable while it fades out, so a press on it
counted as a press outside the Dialog under it. A scroll closes a list
on its own, so clicking a still-visible option dismissed the whole
Dialog and lost the form. Mark a closing overlay
`data-react-aria-top-layer`, keyed on a new `isExiting` render arg of
`DisplayTransition`, in Overlay (popover, modal, tray), ListBoxPopover
and Select.

Closes CUB-5293

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A press on a closing nested modal or tray now reaches it, which leaves
focus inside it, and its raw `useOverlay` handler swallowed the next
`Escape` meant for the Dialog under it. Use `useOverlayEscapeGuard`
there, as `Popover` and `Select` already do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
cube-ui-kit Ready Ready Preview Oct 5, 2026 4:45pm UTC

Request Review

@changeset-bot

changeset-bot Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4249860

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cube-dev/ui-kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

🏋️ Size limit report

Name Size Passed?
All 598.92 KB (+0.02% 🔺) Yes 🎉
Tree shaking (just a Button) 129.75 KB (+0.05% 🔺) Yes 🎉

Compared against main at 04d2e87 — run 37342060991, 2026-10-05T16:35:52Z.

To see which modules changed, download the size-limit-statoscope-report artifact from this run and open report.html.

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

📦 NPM canary release

Deployed canary version 0.0.0-canary-51dc1fd.

@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

🧪 Storybook is successfully deployed!

The toast and notification container is portaled outside any open
Dialog, so React Aria counted a press on it as a press outside: the
Dialog closed and the action never ran. Mark the container
`data-react-aria-top-layer`, as React Aria's own toast region does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tenphi
tenphi merged commit 05f03e8 into main Oct 5, 2026
18 checks passed
@tenphi
tenphi deleted the andrew/cub-5293-picking-an-option-from-a-fading-dropdown-inside-a-dialog branch October 5, 2026 16:59
@tenphi tenphi mentioned this pull request Oct 5, 2026

This branch was successfully deployed

2 active deployments
Preview — 4249860e Deployed Oct 5, 2026 by vercel[bot]
Chromatic staging — 4249860e Deployed Oct 5, 2026 by tenphi via Prepare Storybook for review & tests #3979
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant