Sign up
View stargazersSign up

fix(web): ⌘Z undoes a settle even when the composer has focus#22608

merged3
main
fix/thread-undo-from-composer
opened13d agobyTtheo
6 files changed
View on GitHubTwo-way sync enabledPR last imported 12d ago

Fixes #16159.

After you settle, snooze, archive, or unpin a thread, the sidebar says "⌘Z to undo". But ⌘Z did nothing. The thread.undo binding skips focused text fields, and the composer is almost always focused: ChatView focuses it on every thread change, and settling the open thread moves you to the next one. So ⌘Z went to the composer's own (empty) undo.

Fix

A new notice claims ⌘Z, even inside a text field, until you press another key or edit text. After that, ⌘Z is the field's own undo again. So the most recent thing you did is what ⌘Z undoes:

  • Settle, then ⌘Z: the thread comes back, even with a draft in the composer.
  • Settle, type, then ⌘Z: your typing is undone and the thread stays settled.

The handler moves from the _chat route to the root sidebar layout, so it works on every route that shows the notice, and listens in the capture phase so the editor's undo keymap can't eat the key first. Holding ⌘Z undoes once. Terminals keep their shortcut. The claim only applies while the sidebar (and so the notice) is visible. The stored thread.undo binding is unchanged, so remaps keep working.

This is @ishaanko's change from #18714 (and #16172), rebased onto current main with his authorship kept. @saphid's #18382 (built on #14339) took a different route: the composer yields ⌘Z only when it is empty with no editor history, plus a history reset on thread switch. I did not take that part:

  • The reset is already on main: #18466 remounts the composer editor per draft, which fixes the cross-thread text restore reported in #16159.
  • The "empty composer only" rule still fails when the next thread has a saved draft.

Before / after

Real web client, headless Chromium, this branch's dev server. Open a thread, focus the composer, Ctrl+Shift+S to settle it, then Ctrl+Z from the composer. Each image: left is after settle, right is after Ctrl+Z.

Before (main): the thread stays settled and the notice stays up.

before

After: "Simplify thread service discovery" is back and the notice is gone.

after

Verification

  • Same live run, text undo: settle a thread with Ctrl+Shift+S, type "draft reply", press Ctrl+Z. The composer is empty again, the thread stays settled, and the notice is still up.
  • vp test run src/hooks/showThreadUndoNotice.test.ts src/hooks/useThreadActions.undo.test.ts src/keybindings.test.ts: 53 passed. Scoped lint clean, web typecheck clean.
  • Not verified: ⌘Z on macOS desktop (the Electron Edit menu uses role: "undo"; Chromium gives the page first chance at the key, and the handler calls preventDefault), IME, and Windows. Mobile has no undo notice, so it is unchanged.

Credit: @ishaanko wrote the fix. @saphid and @juliusmarminge did the earlier composer-history work in #14339 and #18382.

Co-authored-by: ishaanko ishaanko.mail@gmail.com

Reviewed with sol-loop: 3 rounds with GPT-6.1 Sol on high. Sol verdict: approved

Made with Claude Opus 5.5 in Claude Code, running in T3 Code.

🤖 Generated with Claude Code

Checks

23 successful · 11 skipped · 1 neutral
All checks passed
9dd9426

Added 2 commits

  • docs: shorten the thread undo shortcut notes0f18e37
  • fix(web): mod+z undoes thread actions from the composerb4e761e
apps/web/src/components/AppSidebarLayout.tsxline 279Outdated · 0f18e37Open thread
……@@ -245,6 +251,64 @@ function NavigationHistoryShortcuts() {
276+ // A modifier tapped mid-hold leaves the held key repeating.
277+ if (!MODIFIER_KEYS.has(event.key)) heldUndoKey = null;
278+ const command = resolveShortcutCommand(event, keybindings, {
279+ context: {
MmacroscopeappBoton line 27913d ago

🟠 High components/AppSidebarLayout.tsx:279

Custom thread.undo rules are resolved as though both panels are closed: when: "terminalOpen && !terminalFocus" never matches with the terminal open, while rules requiring !terminalOpen or !previewOpen can incorrectly allow undo when those panels are open. ThreadUndoShortcut omits terminalOpen and previewOpen from the resolver context, so they default to false; pass the route’s actual panel-open state, as NavigationHistoryShortcuts does.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @apps/web/src/components/AppSidebarLayout.tsx around line 279:
2
3Custom `thread.undo` rules are resolved as though both panels are closed: `when: "terminalOpen && !terminalFocus"` never matches with the terminal open, while rules requiring `!terminalOpen` or `!previewOpen` can incorrectly allow undo when those panels are open. `ThreadUndoShortcut` omits `terminalOpen` and `previewOpen` from the resolver context, so they default to `false`; pass the route’s actual panel-open state, as `NavigationHistoryShortcuts` does.
Ttheo13d ago

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Fixed in 9dd9426. The old _chat handler passed both flags, so leaving them out was a regression for custom rules. routePanelsOpen(routeThreadRef) now builds terminalOpen/previewOpen for both NavigationHistoryShortcuts and ThreadUndoShortcut. heldUndoKey moved to a ref so the new routeThreadRef dependency can't reset it mid-hold when an archive undo navigates.

MmacroscopeappBot13d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This PR changes the default mod+z interaction in the composer by temporarily prioritizing thread undo after sidebar actions, using a new global capture-phase shortcut handler. The change is focused and tested, but it alters a product-wide default interaction.

You can add or adjust custom eligibility rules. Learn more.

Added 1 commit

  • fix(web): custom thread undo rules see the open terminal and preview9dd9426
CcoderabbitaiBot13d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 693bf9e7-94ff-4be9-8926-5411d1ac0cc8
📥 Commits

Reviewing files that changed from the base of the PR and between ecfe4f7955bd826a4d465aa52200ea4354a0cb7f and 9dd9426afd3fcbf0fe02ead6e1e5b92be36fba7c.

📒 Files selected for processing: 6
  • apps/web/src/components/AppSidebarLayout.tsx
  • apps/web/src/hooks/showThreadUndoNotice.test.ts
  • apps/web/src/hooks/showThreadUndoNotice.ts
  • apps/web/src/routes/_chat.tsx
  • docs/user/keybindings.md
  • docs/user/thread-sidebar.md
💤 Files with no reviewable changes: 1
  • apps/web/src/routes/_chat.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The thread undo shortcut now handles eligible undo notices while an editable field is focused. The change adds notice claim state, moves shortcut handling to AppSidebarLayout, removes the chat-route handler, and updates the keyboard shortcut documentation.

Changes: Thread undo shortcut

Layer / File(s)Summary
Undo notice shortcut claim
apps/web/src/hooks/showThreadUndoNotice.ts, apps/web/src/hooks/showThreadUndoNotice.test.ts		Undo notices claim mod+z. New helpers query and release the claim. Tests cover claim, release, a new notice, undo completion, and expiry.
Shortcut handling and documented behavior
apps/web/src/components/AppSidebarLayout.tsx, apps/web/src/routes/_chat.tsx, docs/user/keybindings.md, docs/user/thread-sidebar.md		AppSidebarLayout handles the configured undo shortcut outside Settings and releases the claim on unrelated keys or input. It also uses routePanelsOpen for navigation shortcut panel state. The chat route no longer handles thread.undo. The docs describe when the shortcut applies in text fields and terminals.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant User
3 participant ThreadUndoShortcut
4 participant threadUndoClaimsShortcut
5 participant undoLatestThreadAction
6 User->>ThreadUndoShortcut: Press configured undo shortcut
7 ThreadUndoShortcut->>threadUndoClaimsShortcut: Check whether the notice claims the shortcut
8 ThreadUndoShortcut->>undoLatestThreadAction: Undo the latest thread action when applicable

Possibly related PRs

  • pingdotgg/t3code#12848: Introduced the shared thread undo actions, thread.undo command, and default mod+z binding that this change extends.

Suggested reviewers: juliusmarminge, flamboh

Merge Risk: ⚪ Minimal · up to 9dd94

The shortcut change is mergeable after normal checks. Terminal shortcuts remain protected by the existing binding condition.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely identifies the main fix: ⌘Z now undoes a settled thread when the composer has focus.
Description checkPassedThe description includes the problem, implementation details, issue reference, scope, screenshots, focused verification results, and unverified environments. It satisfies the required template.
Linked Issues checkPassedIssue #16159 requires the sidebar undo shortcut to restore settle, snooze, archive, and unpin actions while the composer has focus. ThreadUndoShortcut now runs in AppSidebarLayout during capture, …
Out of Scope Changes checkPassedThe changes stay within issue #16159. The route-handler move, shared panel-state helper, shortcut lifecycle tests, and user documentation support the thread undo behavior. No unrelated product behavio…
Full details: Linked Issues check

Explanation

Issue #16159 requires the sidebar undo shortcut to restore settle, snooze, archive, and unpin actions while the composer has focus. ThreadUndoShortcut now runs in AppSidebarLayout during capture, temporarily treats the focused editor as non-editable while the visible notice owns the shortcut, and calls undoLatestThreadAction. The notice state releases the claim after another key or input, and clears it after undo or expiry. Existing undo-action coverage includes settle, snooze, archive, and unpin. New tests cover shortcut claim, release, re-claim, undo completion, and expiry. Terminal and command-palette guards remain in place.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #16159. The route-handler move, shared panel-state helper, shortcut lifecycle tests, and user documentation support the thread undo behavior. No unrelated product behavior or unrelated files are changed.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.