Fix Dock sidebar render after reopen - #5437
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughCentralizes Dock activation into DockControlsStore.synchronizeSidebarLifecycle, moves lifecycle synchronization into RightSidebarPanelView, removes DockPanelView lifecycle hooks, and adds a UI test that verifies Dock re-renders after hiding and re-showing the right sidebar. ChangesDock Sidebar Lifecycle Synchronization and Rerender Fix
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 17 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (17 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 |
7d35407 to
f565e52
Compare
Greptile SummaryFixes Dock terminals staying blank after hiding and reopening the right sidebar (
Confidence Score: 5/5Safe to merge. The change centralises dock lifecycle in the sidebar view and adds a direct regression test that verifies the exact bug scenario. The production fix is logically sound: synchronizeSidebarLifecycle is idempotent (activate/deactivate both safe to call multiple times), all four call sites in RightSidebarPanelView pass the current SwiftUI-owned state, and removing DockPanelView's own hooks eliminates the original source-of-truth split. The regression test exercises process liveness and visual redraw end-to-end. No new data paths, auth boundaries, or persistence changes are introduced. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant SwiftUI
participant RSPV as RightSidebarPanelView
participant DCS as DockControlsStore
participant DPV as DockPanelView
Note over User,DPV: Sidebar Hide (⌥⌘B)
User->>SwiftUI: press ⌥⌘B
SwiftUI->>RSPV: onChange(isVisible: false)
RSPV->>DCS: synchronizeSidebarLifecycle(isVisible:false, ...)
DCS->>DCS: deactivate()
Note over RSPV,DPV: DockPanelView stays in portal hierarchy
Note over User,DPV: Sidebar Reopen (⌥⌘B)
User->>SwiftUI: press ⌥⌘B
SwiftUI->>RSPV: onChange(isVisible: true)
RSPV->>DCS: synchronizeSidebarLifecycle(isVisible:true, mode:.dock, ...)
DCS->>DCS: activate(rootDirectory:, workspaceId:)
DCS->>DPV: redraws portal-hosted terminal
Reviews (2): Last reviewed commit: "chore: update dock panel length budget" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxUITests/FeedSidebarUITests.swift`:
- Around line 391-399: The test is sampling the rightmost 12% of the window
which can include main terminal pixels; update dockTerminalBrightPixelCount(in:)
to locate the Dock panel element (e.g., the accessibility identifier or element
named "DockPanel") and take the screenshot/crop based on that element's frame
instead of using xFractionStart:0.88; compute the crop bounds from the DockPanel
XCUIElement (or its firstMatch) and pass that image region to brightPixelCount
so the check only inspects the Dock panel; apply the same change to the other
occurrences referenced in the range that currently use fractional window
cropping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 763da0d6-a148-412e-bd56-57adff5968c0
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (3)
Sources/DockPanelView.swiftSources/RightSidebarPanelView.swiftcmuxUITests/FeedSidebarUITests.swift
f565e52 to
405e825
Compare
Summary
Fixes #5435
Verification
top -s 1, hide/reopen right sidebar, confirmed terminal renders after reopen and portal returns toentryVisible=1 hostedHidden=0FeedSidebarUITests/testDockTerminalRerendersAfterRightSidebarHideShowviatest-e2e.ymlpassed in run https://github.com/manaflow-ai/cmux/actions/runs/26997746067Test policy
f2a9088c3adds the failing regression test onlye17d0405badds the fix405e8257eupdates the Swift file length budget metadataLocalization
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Changes are limited to Dock/right-sidebar lifecycle wiring and UI tests; no auth, data, or user-facing copy changes.
Overview
Fixes Dock terminals going blank after toggling the right sidebar off and on by driving activation from sidebar state instead of
DockPanelViewlifecycle hooks.DockControlsStoregainssynchronizeSidebarLifecycle, which deactivates when the sidebar is hidden or not in Dock mode and otherwise reactivates with the current root directory and workspace.RightSidebarPanelViewcalls this on appear/disappear and when visibility, mode,dockRootDirectory, orworkspaceIdchange;DockPanelViewno longer runs its ownonAppear/onDisappear/onChangeactivate logic. A UI regression test hides/shows the sidebar (⌥⌘B), keeps the Dock render PID alive, and checks the terminal redraws via a bright-pixel screenshot crop.Reviewed by Cursor Bugbot for commit 405e825. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes the Dock terminal staying blank after hiding and reopening the right sidebar. The Dock lifecycle now reactivates on reopen, keeps the render process alive, and redraws the terminal. Fixes #5435.
Bug Fixes
RightSidebarPanelViewviasynchronizeDockLifecycle(...), which callsDockControlsStore.synchronizeSidebarLifecycle(...)on appear/disappear and when sidebar visibility, mode,rootDirectory, orworkspaceIdchange; deactivates when hidden or not in Dock mode, and re-syncs when mode availability refreshes.DockPanelView; lifecycle is driven by sidebar state so reopen reactivates and redraws.Refactors
.github/swift-file-length-budget.tsvforSources/DockPanelView.swift.Written for commit 405e825. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Refactor
Tests