feat(confirmations): ask in a modal instead of an action toast - #199
Merged
Merged
Conversation
Artmann
force-pushed
the
feat/confirm-modal
branch
from
September 18, 2026 04:31
d642002 to
deabb5c
Compare
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
Artmann
force-pushed
the
feat/confirm-modal
branch
from
September 18, 2026 04:40
deabb5c to
da02ca5
Compare
Deleting a connection, a worksheet selection, or an environment asked in a sonner toast carrying a Delete action. A toast is a poor place to ask: it dismisses itself, it never takes focus, it is not announced as a dialog, and the question is gone by the time the user looks back at it. All three now go through one AlertDialog. `useConfirm()` takes the same shape the action toast did -- a title, a description and the callback the confirm button runs -- so no caller keeps "is the dialog open" state of its own, and the worksheet explorer still confirms a whole selection in one question. The success and error toasts are unchanged; only the question moved. The primitive is written by hand rather than generated: the shadcn CLI writes to a literal `@/` directory here and pulls in the umbrella `radix-ui` package, so `ui/alert-dialog.tsx` follows `ui/popover.tsx` instead -- one `@radix-ui/react-alert-dialog` import and the app's own tokens. A Radix modal sets `pointer-events: none` on `<body>` and restores it when the last layer closes. A test that leaves the dialog open is unmounted rather than closed, so the restore never runs and the style leaks to the next test in the file, which then fails on `user.click` pointing at whatever it clicked. `vitest.setup.ts` clears it on the way into each test -- `beforeEach`, because vitest runs `afterEach` hooks in reverse registration order and one registered there would fire before the unmount that leaks it.
…ectory `npx shadcn@latest add alert-dialog` put both files under a literal `./@/` directory, silently rewrote this repository's `button.tsx` inside it, and added `cn` and the umbrella `radix-ui` to package.json. Nothing landed in `src/`.
Artmann
force-pushed
the
feat/confirm-modal
branch
from
September 18, 2026 04:43
da02ca5 to
f93738e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deleting a connection, a worksheet selection, or an environment asked in a sonner toast carrying a
Deleteaction. A toast is a poor place to ask a question: it dismisses itself, it never takes focus, it is not announced as a dialog, and it is gone by the time the user looks back at it.All three now go through one
AlertDialog.What changed
src/app/components/ui/alert-dialog.tsx— the Radix primitive, written by hand in the shapeui/popover.tsxalready establishes: one@radix-ui/react-alert-dialogimport and the app's own tokens (bg-panel,text-text,z-(--z-portal)). The shadcn CLI was tried first and is not usable in this repository — see the friction entry in the second commit.src/app/components/ConfirmDialogProvider.tsx— one dialog for the whole app, asked for imperatively.useConfirm()takes the same shape the action toast did (a title, a description, and the callback the confirm button runs), so no caller keeps "is the dialog open" state of its own and the worksheet explorer still confirms a whole selection in one question. Closing drops the options rather than flipping anopenflag, so the callback — and the selection it closes over — is released as soon as the question is answered.DatabaseExplorer,WorksheetExplorer,SettingsScreen) calluseConfirm(). Their success and error toasts are untouched; only the question moved.renderer.tsxand intest-utils.tsx.One test-infra fix
A Radix modal sets
pointer-events: noneon<body>and restores it when the last layer closes. A test that leaves the dialog open is unmounted rather than closed, so the restore never runs and the style leaks to the next test in the file, which then fails onuser.clickwith "the element has pointer-events: none" — pointing at whatever it clicked rather than at the dialog.vitest.setup.tsclears it on the way into each test.beforeEachrather thanafterEach, because vitest runsafterEachhooks in reverse registration order, so one registered there fires before Testing Library's cleanup — that is, before the unmount that leaks the style.Verification
yarn typecheck,yarn lintandyarn format:checkare clean.yarn testpasses 1607 tests across 129 files, including six newConfirmDialogProvidertests covering confirm, Cancel, Escape, and the missing-provider assertion; the existing delete tests now click Cancel rather than walking away from the question.Not verified: the visual check in the running app.
yarn startexits as soon as it is not attached to an interactive terminal — the Vite dev server dies with it and the window loads nothing — so it could not be driven from a background shell. Worth a look at the modal inyarn startbefore merging, particularly opening one from inside the connection editor overlay (the--z-portalvs--z-overlaycheck).🤖 Generated with Claude Code
https://claude.ai/code/session_0156WamccrozzRDnhxs9nRH7