chore(e2e): review screenshots for #2551 - #2562
Closed
copse-release-bot[bot] wants to merge 3 commits into
Closed
copse-release-bot[bot] wants to merge 3 commits into
copse-release-bot[bot] wants to merge 3 commits into
Conversation
Cmd/Ctrl+W deletes the active thread. Nothing checked whether the user was looking at a dialog when they pressed it, so closing Settings with the keystroke that closes things everywhere else destroyed a conversation instead. There is no undo. The guard reads the DOM — is any `<dialog>` open — rather than asking a list of `isXOpen()` predicates. The renderer has seventeen dialogs and the one hand-maintained list of them, three lines above this in the Cmd/Ctrl+F handler, named four: settings, the command palette, file search, keyboard shortcuts. That list is the failure mode the issue anticipates when it says "or any other dialog for that matter", so the fix is shaped so a new dialog is covered the moment it exists. The find bar now uses the same check, which is a behaviour change worth noticing: it also defers to the thirteen the list missed — an approval, an SSH passphrase, a confirm — each a question it should not open underneath. `show()` counts as well as `showModal()`. The approval prompt appears inline over the chat rather than modally, and it is still something the user is answering. `preventDefault()` deliberately stays outside the guard. It is what keeps the keystroke from also reaching macOS's File ▸ Close accelerator, and moving it inside would hand Cmd+W back to the menu exactly when a dialog is up. So the keystroke is still swallowed; only the delete is skipped. Doing nothing is the right answer rather than closing the dialog for them — Esc already does that, in the handler immediately below, and every dialog honours it. Left alone deliberately: the shortcut is matched inline as `meta && e.key === 'w'` rather than through a `matchXShortcut` helper like its neighbours, so Caps Lock (which makes `e.key` `'W'`) silently disables it. Routing it through a matcher would be tidier, but it would also widen which keystrokes delete a thread, and that is the wrong direction to move a destructive shortcut without being asked. main.ts binds its shortcuts inside a boot function with no seam to call, so the wiring is pinned at the source level — the file's existing test does the same for the layout boot — while the guard itself is unit-tested against real `<dialog>` elements. The wiring assertions check the property rather than the text: every `confirmDeleteThread()` call site is guarded, not just the one that exists today. Verified locally: 8641 tests, 8631 pass, 10 skipped, 0 fail, plus typecheck, lint, format, dead-code, oracle, e2e-syntax and demo:site. All three wiring assertions were confirmed to fail with the guard removed. Refs #2474 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hpk1gEma9LhMUuvr2TwUj6
Contributor
🖥️ PR preview
|
Contributor
|
Closing automatically: parent PR #2551 is closed, so these candidates have no review context left. Reference screenshots only ever move through a live parent PR. |
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
2 participants
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.
Screenshot candidates rendered for parent PR
#2551 at
79f212ae878a36dc9faedded00d955c4af2d8e34byCI run 34152645664.
Review GitHub's image diffs, then merge this PR (or enable auto-merge) to apply the
accepted references to
claude/dialog-guard-cmd-w-2474. This branch containsonly PNG candidates from the immutable
reference-screenshot-candidates-34152645664artifact and never targetsmain.If the parent branch has advanced beyond the source SHA above, do not merge this PR;
the successful CI run for the new head will replace it.