Repository navigation
Fix keyboard focus when revealing a chat in another pane - #28987
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 12 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly summarizes the primary change: fixing keyboard focus when revealing a chat in another pane. Description check The description is detailed and covers the user impact, implementation, rationale, testing, scope, risks, and known coverage limits. It includes visual proof and completed checklist items, but current…
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments: 1
src/renderer/src/components/native-chat/native-chat-composer-reveal-focus.test.tsx: 1
212-228: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe remaining tests use outside buttons, not an editable outside the destination root and retained-pane hosts. The only editable-preservation test focuses the
inputinsideHarness, so it is already protected by the destination-root containment check. The sibling test does not cover external editable focus. This is a concrete test-coverage gap, not a demonstrated runtime defect.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
2fc111b1-fed8-4d08-b22e-104f7ab87cfd
📒 Files selected for processing: 2
src/renderer/src/components/native-chat/native-chat-composer-reveal-focus.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-composer-reveal-focus.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
d29b3ca5-62ed-4e52-9211-705e75ff509c
📒 Files selected for processing: 12
src/renderer/src/components/native-chat/native-chat-composer-reveal-focus.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-composer-reveal-focus.tssrc/renderer/src/components/tab-bar/QuickLaunchButton.tsxsrc/renderer/src/components/tab-bar/use-tab-bar-create-menu-controller.tssrc/renderer/src/components/use-terminal-create-actions.tssrc/renderer/src/lib/launch-agent-in-new-tab-host-agents.test.tssrc/renderer/src/lib/launch-agent-in-new-tab-host-agents.tssrc/renderer/src/lib/launch-agent-in-new-tab-structured-route.tssrc/renderer/src/lib/launch-agent-in-new-tab.test.tssrc/renderer/src/lib/launch-agent-in-new-tab.tssrc/renderer/src/lib/structured-agent-session-provisional-tab.test.tssrc/renderer/src/lib/structured-agent-session-provisional-tab.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
|
A selected chat could leave typing in another pane’s composer. Explicit chat activation and approved New Agent actions now move typing to the destination composer while preserving unrelated editable fields. Round 5 fixes slow startup and remote launches: the one-second focus claim window started before routing and admission, which can take three seconds. Commit 00f6ff2 starts that window when the tab opens. Pointer or Tab navigation, a newer action, launch refusal, and failure still cancel their own phase. The CodeRabbit thread is resolved. The existing focused test file passes all 23 tests. Fresh independent narrow review found no proven issue, and CI’s static analysis and typecheck passed on this head. All five unit-test shards, relay integration and both package jobs passed. Test-selection evidence also passed, and the full PR Checks workflow completed successfully on this exact head. A short remote rendered recheck remains pending. Current main intentionally skips the aggregate when all required results match; the coordinator confirmed that this satisfies the gate only after every required job succeeds on this exact head. Earlier Grok comparison screenshots cover previous head 6167d52; they are not current-head proof. Grok needs sign-in in the required isolated driver home and the approved fallbacks failed to start. Windows allows this worker to drive CDP directly, but both Windows hosts are currently below the required 30% free-memory gate; no test app has been launched this round. Previously disclosed gaps remain: actual upper-PR Split Chat UI, Review Notes provider selection, question-field renderer protection, controlled host delay, delayed composer readiness, Windows, and SSH were not exercised in the earlier comparison. Resume and initial-workspace agents retain prior behavior and remain outside this PR’s scope. No merge has been performed. |
ELI5
Switching to a chat in another pane could highlight the new pane while typing still went into the old chat. The reveal rule protected every editable field, including the composer the user meant to leave. Explicit chat activation and new-chat actions now carry keyboard focus to the requested composer; Incidental reveal preserves editing in Find-in-chat, question answers, and fields outside the chat. Deliberate tab navigation keeps its existing focus behavior.
What Changed
The tab-bar + menu/search, New Agent shortcut, and shared New Agent menu (including Review Notes) capture a cancellable focus request before waiting for the execution host. The existing launch path carries it to the real tab, and the focus manager keeps it until the composer is ready, the user clicks or tabs elsewhere, another action supersedes it, or one second passes after the tab opens. A successful launch does not cancel a request already handed to the focus manager. Only another chat composer may yield to an explicit request; ordinary reveal keeps the existing editable-field protection.
Why
Pane visibility alone cannot tell whether the user intended to move keyboard focus. Separating an explicit action from incidental reveal follows the common pattern and preserves other edits. The request is temporary and local to the renderer; no saved state or messages between hosts change. Terminal outcomes discard it. The shared launch API lets the stacked Split Chat feature use the same handoff.
Linked Issue
Prerequisite for #28950, which adds Split Chat. This PR owns composer focus; the stacked PR owns creating the split pane.
Visual Proof
Round 5 current head
00f6ff2c9fbcorrects when the focus claim timer starts. Current-head remote QA is pending; the captures below are explicitly from previous head6167d52.Grok 4.7 ran main (
74ad2309d6) and the exact candidate (6167d52) on M4Air through the hidden renderer, with isolated homes, a setup script, and default Notes/Setup tabs. Whole-window captures below show a real cross-pane tab click: the source draft stays intact and the typing marker lands in the destination on both builds. This is a regression check, not a reproduced difference on that click path.Main:
Final candidate:
Matching shortcut runs on both builds received an unsent marker after New Agent without manually focusing the destination composer:
Matching follow-up runs also preserve focused Find-in-chat and Find files during nonpointer activation. Deliberate chat-tab clicks move the caret to the composer on both builds. After New Agent followed by a Search click, the candidate accepts typing in Jump while main leaves it on the sidebar button and drops it; accepting text in the field the user clicked is intended.
Main Search after the click:
Final candidate Search after the click:
Rendered comparison and the bounded fixture attempt are complete. Real + menu, tab activation, shortcut, incidental reveal and focused Find/external-field cases were checked on both builds. Review Notes menus opened on both, but provider selection was not exercised because it can submit a note. A real question card mounted from a fake read owner, then its synthetic tab disappeared before the focus check; question protection remains inconclusive. Safe mocked admission was not installed, so controlled one-second expiry, held-request cancellation and delayed readiness remain unverified. Cmd+J/sidebar rows, terminal-backed chat, Windows and SSH were not exercised. No real provider message or question answer was sent. All owned apps exited, real config hashes stayed unchanged, and the QA checkouts were clean before removal. The final status comment records these coverage limits.
Testing
Round 5 existing focused test file: 23 tests passed through the test queue; no existing expiry test, so no added cases.
Current-head full PR workflow and fresh narrow review.
Current-head remote hidden-renderer recheck.
Previous-head PR workflow 38694287760 passes on previous commit
6167d52041119c97a546a4222ec8159353940a2c, including the aggregate gate.Previous-head remote hidden-renderer comparison of main and
6167d52, with coverage limits above.Existing tests updated for protected fields, cancelled intent, late composer readiness, supplied intent through host routing, and terminal cancellation; all five CI test shards passed.
Review
No dependencies, saved data, remote payloads, process launching, filesystem paths, or keyboard shortcut definitions change. Chat admission still belongs to its execution host, including SSH-backed and folder workspaces. Source review and runtime limitations will be recorded in the final status comment.
Agent skill upstream boundary
Notes
Deferred by scope decision: Resume in New Chat keeps its existing focus behavior and receives no new focus request. Workspace creation with an initial agent also keeps the existing reveal behavior. Intended difference from the common pattern: a new chat may wait for the machine running it to create its tab, so launch keeps the request until the tab exists, then the composer has a one-second claim window. Requests that expire do not override later input.
Author X: @BrennanKB5.
Checklist