refactor(web): chat shortcuts live with the hooks that own their actions#23097

merged1
refactor/split3-chatview-controls-bar
refactor/split3-chatview-shortcuts
opened2d agobyJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 2d ago

ChatView built the whole keyboard shortcut table inline: about 130 lines of handlers that reached into the terminal actions, the right panel actions, thread settlement and pinning, and the composer handle. ChatView had to pull every one of those pieces out of their hooks just to hand them to useChatShortcuts. The hook itself kept its input in a ref assigned from a layout effect.

Each owner now returns its own chords:

  • useChatTerminalActions returns shortcuts for terminal.toggle/split/splitVertical/close/new. It checks terminal permissions when the key is pressed instead of ChatView reading two scopes on every render.
  • useRightPanelActions returns rightPanel.toggle/toggleMaximized/close, threadPanel.toggle and diff.toggle.
  • useThreadLifecycle owns thread.settle and thread.pin, together with the settled and pinned state they read. ChatView no longer calls useThreadActions or exports that state.
  • The composer and branch toolbar chords are composerShortcutCommands in chat/composerShortcuts.ts. useChatShortcuts takes composerRef and branchToolbarRef and resolves those chords inside its key handler, which is the only place a ref read is allowed. It also uses the refs for type-to-focus, replacing the typeIntoComposer callback.
  • useChatShortcuts reads its input through useEffectEvent instead of useLayoutEffect(() => { latest.current = input }). The listeners still attach once.

ChatView keeps the chords that combine state from several owners (copy reference, find, cycle host, the queue chords, stop, and the project-script fallback). The behavior is unchanged.

Filebeforeafter
ChatView.tsx4,0073,899
useChatShortcuts.ts106111
composerShortcuts.ts38
useChatTerminalActions.ts408451
useRightPanelActions.ts313330
useThreadLifecycle.tsx582616

useStableCallback calls in ChatView: 33 to 30. No render boundaries change; the composer render counts match #23093.

Tested:

  • apps/web typecheck and lint on the touched files.
  • React Compiler coverage: 0 bailouts on ChatView and the touched hooks. ChatView stays in the slow set.
  • Hands-on in a dev build, server thread:
    • Ctrl+J opens and closes the terminal drawer.
    • With the terminal focused: Ctrl+N adds a terminal, Ctrl+D splits it, Ctrl+W asks for confirmation and closes it.
    • Ctrl+Alt+B toggles the right panel, Ctrl+Shift+B toggles the thread panel.
    • Ctrl+D opens the Diff surface and Ctrl+W closes it.
    • Ctrl+Shift+M opens the model picker, Ctrl+Shift+E the reasoning menu, Ctrl+Shift+A the mode list.
    • Typing a letter with nothing focused lands it in the composer.
    • Ctrl+Shift+P pins and then unpins (undo toast), and Ctrl+Shift+C copies the thread ID.
    • Ctrl+F opens find and Escape closes it, returning focus to the composer.
    • Ctrl+Shift+G does nothing on a server thread, same as main: the branch toolbar only mounts in the composer context strip.

🤖 Generated with Claude Code

Checks

All checks passed

Added 1 commit

  • refactor(web): chat shortcuts live with the hooks that own their actions300d3ce
Synced from GitHub

Approvability

Verdict: Approved at 300d3ce

Macroscope's review found this PR approvable — This PR reorganizes existing chat shortcut handlers into the hooks that own their actions, while preserving the prior permission checks, routing, and keyboard-event behavior. It introduces no new user-facing capability, schema change, product-default change, or static-analysis override.

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

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: 20c4d860-6213-4fd2-8409-f303fcdbfa74
📥 Commits

Reviewing files that changed from the base of the PR and between 4d5d85a12e0d632a56dacb4849a956d4ec1d87b8 and 300d3ced33bc5b8afd5a823cd7bd27e9102db4c3.

📒 Files selected for processing: 6
  • apps/web/src/components/ChatView.tsx
  • apps/web/src/components/chat/composerShortcuts.ts
  • apps/web/src/components/chat/useChatShortcuts.ts
  • apps/web/src/components/chat/useChatTerminalActions.ts
  • apps/web/src/components/chat/useRightPanelActions.ts
  • apps/web/src/components/chat/useThreadLifecycle.tsx

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

📝 Walkthrough

Chat shortcut handling now uses command maps returned by composer, terminal, right-panel, and thread-lifecycle hooks. useChatShortcuts accepts composer and branch-toolbar refs and uses effect events to access current input.

Changes: Chat shortcuts

Layer / File(s)Summary
Shortcut contract and dispatch
apps/web/src/components/chat/useChatShortcuts.ts
The hook accepts composer and branch-toolbar refs, resolves commands from command maps, and uses effect events for current input and shortcut handling.
Composer and branch commands
apps/web/src/components/chat/composerShortcuts.ts
The new factory handles model selection, reasoning effort, composer controls, branch selection, and previous-worktree navigation. Handlers ignore repeated events.
Terminal and right-panel commands
apps/web/src/components/chat/useChatTerminalActions.ts, apps/web/src/components/chat/useRightPanelActions.ts
The hooks return command maps for terminal and right-panel actions. Terminal commands check access and route actions to the focused panel or drawer.
Thread lifecycle commands
apps/web/src/components/chat/useThreadLifecycle.tsx
The hook returns settle and pin commands with authorization, capability, and thread-state checks. Failed actions can produce error toasts.
ChatView shortcut wiring
apps/web/src/components/ChatView.tsx
ChatView passes the hook-provided command maps and composer references to the shortcut hook. Thread-find, reference-copy, queued-message, and thread-stop handling remains.

Priority: ⬇️ Low

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

Change: Refactor

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant ChatView
3 participant useChatShortcuts
4 participant ChatShortcutCommands
5 participant ShortcutHandler
6 ChatView->>useChatShortcuts: Pass command maps and shortcut context
7 useChatShortcuts->>ChatShortcutCommands: Resolve command for keyboard event
8 ChatShortcutCommands->>ShortcutHandler: Invoke mapped handler

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to 300d3

This refactor moves keyboard shortcut definitions into the hooks that own their actions. Inspection found no concrete behavior regression, so it looks safe to merge after the usual checks.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description explains the refactor and provides detailed verification results, but it does not include the required Scope and approval information. It also does not use the required Problem and Cha…Add the required Problem, Change, Scope and approval, and Verification sections. State the underlying problem and expected behavior, link the triaged issue or maintainer approval with the approval comment, or explain why an exemption applie…
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the main refactor: moving chat shortcut definitions into the hooks and modules that own their actions.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the refactor and provides detailed verification results, but it does not include the required Scope and approval information. It also does not use the required Problem and Change section headings explicitly.

Resolution

Add the required Problem, Change, Scope and approval, and Verification sections. State the underlying problem and expected behavior, link the triaged issue or maintainer approval with the approval comment, or explain why an exemption applies. Keep the existing verification details and report any checks that were not completed.

✨ 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.