fix(web): confirm dialogs respond to Enter and await async actions - #1744
Conversation
The confirm dialog's initial focus was resolved from the Button component's $el, which is a text node in dev builds (the component has a template-root comment, so it renders as a fragment). Focus fell back to the header close button, so Enter cancelled instead of confirming. Resolve the initial focus with a CSS selector on the confirm button instead. ConfirmOptions now accepts an async action: the dialog stays open with a loading state (cancel/Esc/overlay suppressed) until the work settles. The archive-session, remove-workspace, and delete-provider confirms move from the menu components into App.vue so the dialog can await the actual client call.
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b12d14b78
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function settle(ok: boolean): void { | ||
| const req = current.value; | ||
| if (!req) return; | ||
| if (!req || busy.value) return; |
There was a problem hiding this comment.
Preserve supersede behavior while busy
Because confirm() still uses settle(false) to cancel an existing request before replacing it, this new busy guard makes superseding a dialog during an in-flight action a no-op: the old promise is not cancelled and can later resolve/reject from the old action, while the newly opened dialog inherits the global busy state and is temporarily inert. This affects any programmatic confirm opened while a destructive action is awaiting; either block replacement while busy or explicitly settle the previous request on the supersede path.
Useful? React with 👍 / 👎.
A second confirm() during an in-flight action would replace the busy dialog and inherit the global busy state, opening inert until the first action settled. Resolve the new request unconfirmed instead.
|
@codex review |
🦋 Changeset detectedLatest commit: fc9eb1e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc9eb1ea05
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| </Button> | ||
| <Button | ||
| ref="confirmButtonRef" | ||
| class="confirm-dialog__confirm" |
There was a problem hiding this comment.
Avoid relying on Button fallthrough for focus
In dev builds this class is applied to the Button component rather than guaranteed to the native <button>. Button.vue has a template-root comment, so Vue renders it as a fragment in the same dev scenario this change is trying to fix; fallthrough attrs such as class are not inherited onto the inner button. That leaves no .confirm-dialog__confirm in the DOM, so Dialog falls back to focusing the close button and Enter still cancels the dialog.
Useful? React with 👍 / 👎.
Related Issue
No linked issue — the problem is described below.
Problem
In the web UI, modal confirmation dialogs (e.g. archive session) had two issues:
$el, but Button has a template-root comment, so in dev builds it renders as a fragment whose$elis a text node. Focus fell back to the header close button, so Enter cancelled the dialog instead of confirming it.What changed
$el, so Enter confirms in both dev and production builds.useConfirmDialognow accepts an asyncaction: while it runs, the dialog stays open with the confirm button loading, and cancel / Esc / overlay-click are suppressed. A rejection closes the dialog and rethrows to the caller.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.