Fix terminal blank when last pane closes (#2665)#2670
Fix terminal blank when last pane closes (#2665)#2670austinywang wants to merge 2 commits intomainfrom
Conversation
When closing panes down to the last remaining one, the surviving terminal went blank because the SwiftUI split-to-single-pane rebuild transiently detached the terminal surface, causing the first responder to be lost. The one-shot async focus reconcile could fire before the view was reattached, leaving no further retry. Include the surviving panel in the layout follow-up as terminalFocusPanelId so the existing retry loop keeps calling ensureFocus until the surface view successfully becomes first responder after reattachment. Closes #2665 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
📝 WalkthroughWalkthroughAdjusted Workspace/Bonsplit pane-close handling to avoid unconditional terminal geometry reconciliation. Now, when a pane collapse closes a pane and a surviving focused panel maps to a terminal, an event-driven layout follow-up targeting that terminal is scheduled; otherwise the code falls back to the existing geometry reconcile path. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR fixes a terminal blank/input-loss bug (#2665) that occurred when closing split panes down to the last remaining pane. The root cause was the SwiftUI split-to-single-pane rebuild transiently detaching the terminal surface mid-focus-reconcile; the fix passes the surviving panel as Confidence Score: 5/5Safe to merge; fix is surgical, well-commented, and handles all relevant edge cases No P0 or P1 findings. The two change sites are symmetric and correct: pane-collapse detection in didCloseTab uses the explicit !bonsplitController.allPaneIds.contains(pane) guard while didClosePane implicitly implies a pane was removed. The includeGeometry flag preserves the prior geometry-reconcile behavior. No double-fire risk is introduced—the trailing scheduleFocusReconcile() calls were already present before this PR. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant BonsplitController
participant Workspace
participant SwiftUI
participant TerminalSurface
User->>BonsplitController: Close last tab in pane
BonsplitController->>Workspace: didCloseTab(pane collapsed)
Workspace->>Workspace: applyTabSelection(survivingPane)
note over Workspace: isDetaching=false,<br/>pane no longer in allPaneIds
Workspace->>Workspace: beginEventDrivenLayoutFollowUp(<br/>terminalFocusPanelId=survivingPanelId,<br/>includeGeometry=true)
Workspace->>SwiftUI: layout observers installed
SwiftUI-->>Workspace: split view rebuilds (detaches surface)
TerminalSurface-->>Workspace: terminalSurfaceHostedViewDidMoveToWindow
Workspace->>Workspace: scheduleLayoutFollowUpAttempt()
Workspace->>TerminalSurface: ensureFocus() [retry loop]
TerminalSurface-->>User: first responder restored ✓
Reviews (1): Last reviewed commit: "Fix terminal blank when last pane closes..." | Re-trigger Greptile |
The focusPanel layout follow-up with terminalFocusPanelId was gated to .terminalFirstResponder trigger only. When creating a new workspace via Cmd+T, the trigger is .standard, so no focus retry loop was set up. The new terminal surface goes through portal attach/detach cycles and the one-shot ensureFocus fires before the view is ready. Extend the follow-up to fire for any trigger when the terminal is not yet first responder, so the retry loop handles portal churn on workspace creation the same way it handles split-close churn. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
terminalFocusPanelId, so the existing retry loop (triggered byterminalSurfaceHostedViewDidMoveToWindow) keeps callingensureFocusuntil the surface view successfully becomes first responder after reattachmentTest plan
Closes #2665
🤖 Generated with Claude Code
Note
Medium Risk
Moderate risk because it changes focus/first-responder and layout-follow-up timing around SwiftUI split-view rebuilds, which could impact keyboard focus behavior across panel types.
Overview
Prevents the surviving terminal from going blank/losing input when split panes collapse to a single pane by re-running the existing event-driven layout follow-up until the terminal surface is reattached and becomes first responder.
This also broadens when terminal focus follow-ups are started during
focusPanel: they now run whenever the terminal isn’t first responder (or for.terminalFirstRespondertriggers), and pane-close paths now preferbeginEventDrivenLayoutFollowUp(..., includeGeometry: true)for surviving terminals instead of a one-shotscheduleTerminalGeometryReconcile.Reviewed by Cursor Bugbot for commit 9c7cb55. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes blank terminals when collapsing to a single pane and when creating a new workspace (Cmd+T), ensuring the terminal keeps focus and accepts input (fixes #2665).
terminalFocusPanelIdso the retry loop re-applies focus after the split view rebuild; applies to both tab-close and pane-collapse; skipped on detach..terminalFirstResponder), covering new workspace creation where the surface isn’t attached yet.Written for commit 9c7cb55. Summary will update on new commits.
Summary by CodeRabbit