Repository navigation
Conversation
Dialogs focused Cancel on open, so pressing Enter cancelled even though Confirm is the highlighted action. Popups now focus the AlertDialogAction or DialogAction when they open, so Enter and Space confirm and Tab reaches Cancel, with native button semantics. Destructive actions aren't marked, so those dialogs keep focus on Cancel. Touch opens still focus the popup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR changes keyboard focus and Enter/Space behavior in shared web dialog primitives and several existing update, installation, and import flows. The resulting product interaction defaults warrant human review. You can add or adjust custom eligibility rules. Learn more. |
Dismissing prior approval to re-evaluate 894463f
ConfirmDialogHost serves every requestConfirmDialog and dialogs.confirm caller, so focusing Confirm there also made Enter approve MCP tool requests, roll back checkpoints and sign out providers. The host now keeps Cancel focused unless the caller passes initialFocus: "confirm", and only the desktop and server update prompts do. DialogAction now rejects destructive Button variants at compile time instead of matching the variant name, which missed ghost-destructive. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Dismissing prior approval to re-evaluate 6f46e56
|
Re CodeRabbit's security note: good catch. |
|
@coderabbitai run pre-merge checks |
✅ Actions performedRunning Pre-merge checks. Results will be posted in the summary comment once complete. |
Replaces #18176, which was closed for missing evidence. The before/after screenshots and the recording are now included below; the code is unchanged.
Problem
Confirmation dialogs focus Cancel when they open, so Enter cancels even though Confirm is the highlighted action. The desktop update prompt is the clearest case. Fixes #18167.
Repro: wait for the desktop update prompt ("Install update … and restart T3 Code?") and press Enter. The dialog closes as if Cancel was pressed.
Change
DialogAction(also exported asAlertDialogAction) incomponents/ui: aButtonthat marks itself as the dialog's primary action. Itsvariantprop excludes the destructiveButtonvariants at compile time, so a destructive action can't take focus by mistake; destructive confirms keep a plainButton.DialogPopupandAlertDialogPopuppass Base UI'sinitialFocusand focus that action when they open. Enter and Space then confirm it with native button behavior, Esc cancels, and Tab reaches Cancel.initialFocusstill wins.ConfirmDialogHostis shared by everyrequestConfirmDialog/dialogs.confirmcaller, so it stays Cancel-first by default. A new optionalinitialFocus: "confirm" | "cancel"onConfirmDialogOptions(packages/contracts, a local-shell TS type; nothing crosses the wire) lets a caller opt in, and destructive confirms keep Cancel even then. Only the desktop and server update prompts opt in. Every other host confirm (MCP tool approvals, checkpoint rollback, provider sign-out, archive, …) behaves exactly as today.Buttonto action swaps, no visual change. None of these dialogs has inputs before the footer that the new focus would skip.apps/mobile) is React Native and doesn't use them.Scope
Follows the direction approved in #18167 (comment): Base UI
initialFocusin the two dialog primitives plus targeted call sites, no Enter interceptor.variant="destructive".Verification
Automated:
vp test runonui/dialog.test.tsx, the newConfirmDialogHost.test.tsx, and the existing tests for the touched flows (confirmDialog,localApi,ServerUpdateAction,relayClientInstallDialog,SidebarUpdatePill,SettingsPanels.restore): 54 tests pass. The new tests cover:AlertDialogandDialog, Cancel when it's disabled, and a caller'sinitialFocuswinning;initialFocus: "confirm", and Cancel for a destructive confirm even when asked for Confirm.Static checks:
tsc --noEmitinapps/webandpackages/contractsis clean.vp lintandvp fmt --checkon the changed files are clean.Manual: I ran
vp run devwith an isolated--home-dirand opened the confirm dialog with the real update message, driving the keys in headless Chrome onmainand on this branch. The table is from the first run, on98beed1a22. The screenshots and recording below repeat the key rows on the current base,8d199b4f7b. The rebase didn't touch the dialog primitives. The recording predates the opt-in commit; the update prompts now passinitialFocus: "confirm", so what they show is unchanged.mainNot checked: the packaged Electron app (same web component), and real screen readers.
Evidence
Initial focus, update prompt (before → after):


Destructive confirm keeps Cancel focused (before → after):


Recording:
mainfirst, then this PR. The caption bar is an overlay added by the recording script to show the key pressed and the result. For themainpass, only the web files were switched while the same dev server kept running, so the footer still shows the branch name.dialog-focus-before-after.mp4
Model and harness: Claude Opus 5.5 in Claude Code (T3 Code).
🤖 Generated with Claude Code