Repository navigation
(ui): replace native confirm/alert with the designed dialog everywhere, and ban them #585
Copy link
Copy link
Closed
Description
Decision (maintainer, 12/10): every confirmation and message in the renderer goes through the app's designed dialog, the same one as the close-session confirmation (showChoiceDialog, public/choice-dialog.js). Native confirm(), alert() and prompt() are banned. A lint rule enforces the ban, and a small substitution module is the only entry point.
Why
- The native dialogs look out of place: the resume, attach and live-elsewhere guards, deletes and save failures all use them.
- They block the renderer.
- Playwright auto-dismisses them, so e2e journeys cannot drive them. QA run 2 hit this.
Inventory on be8566c (grep -nE "\b(window\.)?(confirm|alert|prompt)\(" public/*.js)
resume-guard.js:33,42,56,62,68,72: scheduled task already running, live elsewhere, continuation choice. Injected asconfirmfromapp.js:307,1519,1551.app.js:1054: delete plan confirm.agents-view.js:421: delete background session.sidebar.js:1169,1236,1238,1241: archive messages.sidebar.js:1313,1316: remap.sidebar.js:1343,1360: hide or delete worktree.settings-panel.js:715: hide project.viewer-panel.js:283,290,293: delete file.viewer-panel.js:473: overwrite a file changed on disk.viewer-panel.js:594: discard unsaved edits.file-panel.js:438,448,451: overwrite and save failure.file-panel.js:1456: discard unsaved edits. This one is sync, inside the unsaved guard ((restore): save the open sessions when the app closes #479/(window): ask before a window close ends the app #478), so take care.activity-trace-panel.js:70,120,133: read, delete, delete failure.
Expected
- A module, for example
public/app-dialogs.js, exposing asyncappConfirm({ title, message, confirmLabel, danger })andappAlert({ title, message }), built onshowChoiceDialog.- It queues instead of resolving null when a dialog is already open.
showChoiceDialogcurrently returns null at once in that case, which would read as Cancel. - Focus follows the (a11y): New Session dialog opened from the keyboard leaves focus on the page body #583 rule: on open, the safe button for a destructive confirm, or the first control; on close, back to the opener.
- It queues instead of resolving null when a dialog is already open.
- Every call site above migrated. Sync callers become async. This matters most on the save and unsaved-edit paths, and wherever an injected
confirmis used:resume-guard.jstakes an async confirm. - ESLint
no-restricted-globals/no-restricted-propertiesforconfirm,alertandprompt(bare andwindow.) overpublic/**, with a message pointing to the module. - Tests:
- The module: queueing, focus, Escape = cancel.
- Each migrated guard keeps its semantics.
- One e2e journey drives a formerly native confirm, for example Delete on a background session, or a resume guard.
- Main-process native dialogs (
dialog.showMessageBox, for example the "Close Switchboard?" quit question) are out of scope. List them in the PR as a follow-up.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels
Activity
(ui): fix review findings on the app dialogs (#585)
(ui): one modal stack routes keys to the topmost dialog (#585)
(ui): keep terminal protocol replies and focus right under the modal …
(ui): trap focus in the topmost modal and give it back on close (#585)
(ui): fall back to the active terminal when a modal's opener cannot t…
(ui): confirmations and messages use the app's own dialog; native one…
(ui): fix review findings on the app dialogs (#585)
(ui): one modal stack routes keys to the topmost dialog (#585)
(ui): keep terminal protocol replies and focus right under the modal …
(ui): trap focus in the topmost modal and give it back on close (#585)
(ui): fall back to the active terminal when a modal's opener cannot t…
(ui): route #478's close question through the modal stack (#585)
(ui): a close question is never held behind another question (#585)
test(ui): settle a queued question before it opens (#585)
(ui): confirmations and messages use the app's own dialog; native one…
(ui): fix review findings on the app dialogs (#585)
(ui): one modal stack routes keys to the topmost dialog (#585)
(ui): keep terminal protocol replies and focus right under the modal …
(ui): trap focus in the topmost modal and give it back on close (#585)
(ui): fall back to the active terminal when a modal's opener cannot t…
(ui): route #478's close question through the modal stack (#585)
(ui): a close question is never held behind another question (#585)
test(ui): settle a queued question before it opens (#585)
(ui): follow #599's Archive dialogs under the modal stack (#585)
(ui): route every dialog through the in-app design, and ban native co…