Add right panel close controls#4148
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
github-actions
Bot
added
vouch:unvouched
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces a new feature (right panel close controls) with significant complexity including cross-platform focus tracking and Electron menu integration. Multiple unresolved review comments identify bugs, including a high-severity deadlock issue with Cmd+W in preview webviews. You can customize Macroscope's approvability policy. Learn more. |
|
what is the interaction demo doing? looks like you're just toggling visibility which you can already do with |
|
pressing cmd+w to close a single panel, since in the vid there was only one, the whole panel closed, sorry for the bad video, ill record another one |
|
Updated the PR description with a separate real-app middle-click demo. It starts with Files and Terminal open, then middle-click closes only Files while Terminal, the right panel, and the app window stay open. I also added focused regression coverage for button 1 and clarified what the existing Cmd+W clip is showing. |
github-actions
Bot
added
size:XL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
|
Follow-up review fix in 85e780f: hosted preview focus is now matched by tab ID against the active right-panel preview, so a standalone preview cannot trigger the right-panel close shortcut. The focused tests cover matching and mismatched preview IDs. |
85e780f to
4935f60
Compare
Keep native close focus scoped to the active hosted preview and preserve pointer ownership across webview blur transitions. Co-authored-by: codex <codex@users.noreply.github.com>
4935f60 to
137c501
Compare
| terminalFocus: terminalFocusOwner !== null, | ||
| terminalOpen: Boolean(terminalUiState.terminalOpen), | ||
| modelPickerOpen: composerRef.current?.isModelPickerOpen() ?? false, | ||
| rightPanelFocus: isRightPanelFocused({ |
There was a problem hiding this comment.
🟡 Medium components/ChatView.tsx:3959
The keydown effect computes rightPanelFocus from rightPanelOpen and activeRightPanelPreviewTabId but omits both from its dependency array, so the installed handler retains stale values. If the right panel is closed without another listed dependency changing, the handler still sees rightPanelOpen: true, and Cmd+W resolves rightPanel.closeActiveSurface even though the panel is already closed. Preview-tab changes are similarly affected — focus ownership can be stale. Add rightPanelOpen and activeRightPanelPreviewTabId to the effect's dependency array.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatView.tsx around line 3959:
The keydown effect computes `rightPanelFocus` from `rightPanelOpen` and `activeRightPanelPreviewTabId` but omits both from its dependency array, so the installed handler retains stale values. If the right panel is closed without another listed dependency changing, the handler still sees `rightPanelOpen: true`, and `Cmd+W` resolves `rightPanel.closeActiveSurface` even though the panel is already closed. Preview-tab changes are similarly affected — focus ownership can be stale. Add `rightPanelOpen` and `activeRightPanelPreviewTabId` to the effect's dependency array.
| send(); | ||
| yield* sendMenuAction(targetWindow, action, true); | ||
| }), | ||
| dispatchMenuActionToMainWindow: Effect.fn("desktop.window.dispatchMenuActionToMainWindow")( |
There was a problem hiding this comment.
🟡 Medium window/DesktopWindow.ts:799
dispatchMenuActionToMainWindow returns true whenever the window matches the main window, even when sendMenuAction only registers a once("did-finish-load") listener and the IPC message has not actually been sent yet. If a menu action like Close Window is invoked while the main frame is still loading and that load fails or is replaced without emitting did-finish-load, the action is silently never delivered — yet the caller treats true as handled and skips its native fallback (e.g. performClose:). The result is that the Close Window command does nothing in this state. Consider returning true only after the message has actually been sent, or adding a fallback so the action is not lost when did-finish-load never fires.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/desktop/src/window/DesktopWindow.ts around line 799:
`dispatchMenuActionToMainWindow` returns `true` whenever the window matches the main window, even when `sendMenuAction` only registers a `once("did-finish-load")` listener and the IPC message has not actually been sent yet. If a menu action like Close Window is invoked while the main frame is still loading and that load fails or is replaced without emitting `did-finish-load`, the action is silently never delivered — yet the caller treats `true` as handled and skips its native fallback (e.g. `performClose:`). The result is that the Close Window command does nothing in this state. Consider returning `true` only after the message has actually been sent, or adding a fallback so the action is not lost when `did-finish-load` never fires.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 137c501. Configure here.
| terminalFocused: terminalFocusOwner !== null, | ||
| }) | ||
| ) { | ||
| return; |
There was a problem hiding this comment.
Preview Cmd+W close deadlocks
High Severity
On macOS desktop, Cmd+W from a focused right-panel preview webview is deferred to the application menu, but preview webContents already preventDefaults that accelerator and forwards a synthetic keydown into the main renderer. The deferred handler then ignores that synthetic event, so neither the menu path nor the renderer close path runs and the surface stays open.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 137c501. Configure here.
| rightPanelFocus: isRightPanelFocused({ | ||
| rightPanelOpen, | ||
| activePreviewTabId: activeRightPanelPreviewTabId, | ||
| }), |
There was a problem hiding this comment.
Keyboard close ignores retained focus
Medium Severity
rightPanel.closeActiveSurface gates on live isRightPanelFocused only, while pointer/webview ownership lives in nativeCloseFocus and is used solely by the macOS menu path via shouldDeferCloseCommandToDesktopMenu. On Windows/Linux Electron, Ctrl+W after a pointer activation that leaves document.activeElement on body (or other non-panel nodes) never resolves the command, so the active surface does not close.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 137c501. Configure here.
2 participants
What Changed
rightPanel.closeActiveSurfacecommand, bound toCmd+W/Ctrl+Wwhile focus is inside the right panel.Why
The right panel has per-tab close actions but no obvious way to hide the whole panel. On macOS,
Cmd+Walso reaches Electron's native Close Window action instead of closing the active side-panel tab. Right-panel tabs also follow the familiar browser/editor convention: middle-click closes the selected tab without closing the panel or app window.Closes #4146.
Closes #4147.
UI Changes
Before
After
Interaction
Cmd+W / whole-panel close
Watch the last active tab close and the panel reopen
The Cmd+W clip starts with one Files tab, so closing the last tab hides the panel; the panel toggle then reopens it.
Middle-click tab close
Watch middle-click close Files while Terminal and the app window stay open
This clip uses an isolated
Close controls demofixture and contains no private workspace content.Validation
bun x vp test run apps/web/src/components/RightPanelTabs.test.tsx(5 tests passed)bun x vp test run apps/web/src/lib/rightPanelFocus.test.ts apps/web/src/lib/nativeCloseFocus.test.ts apps/web/src/lib/rightPanelCloseRequest.test.ts apps/web/src/components/RightPanelTabs.test.tsx(4 files, 33 tests passed)bun run --cwd apps/web typecheck(passes)bun x vp check(passes with 10 existing warnings in unrelated files)bun x vp run typecheck(passes across all 15 workspaces)Checklist
Note
Medium Risk
Touches macOS menu accelerators, window-close routing, and keyboard shortcut precedence across desktop and renderer; behavior is heavily tested but regressions could affect Cmd+W or accidental window closes.
Overview
Adds right-panel close affordances and wires macOS Close Window / Cmd+W through the renderer before falling back to native window close.
The web app gains a title-bar Close right panel control, a
rightPanel.closeActiveSurfacebinding (mod+wwhenrightPanelFocusand notterminalFocus), middle-click-to-close on panel tabs, and focus tracking (nativeCloseFocus,rightPanelFocus) so close targets the drawer terminal, a right-panel terminal pane, an active surface, or the whole panel. On macOS Electron, in-app Cmd+W for those close commands is deferred so the menu path can run;AppSidebarLayouthandlesclose-window-or-right-panelvia a cancelable event beforewindow.close().The desktop app replaces macOS File’s native
closerole with a custom Close Window item that sendsclose-window-or-right-panelto the main window through newdispatchMenuActionToMainWindow(no reveal), orperformClose:when focus is on system UI or a non-main window.ElectronMenu.sendActionToFirstRespondersupports the native fallback.Reviewed by Cursor Bugbot for commit 137c501. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add right-panel close controls with Cmd+W and title bar button support
RightPanelCloseControlbutton to the right-panel title bar and wiresCmd+W(asrightPanel.closeActiveSurface) to close the active right-panel pane, surface, or panel when the right panel owns focus.role: closewith a customClose Windowhandler in DesktopApplicationMenu.ts that routesclose-window-or-right-panelto the renderer's main window first, falling back toperformClose:viaElectronMenu.sendActionToFirstResponder.dispatchMenuActionToMainWindowtoDesktopWindowfor targeted dispatch that returns whether the action was handled.Cmd+Wnow closes the active right-panel surface when the right panel is focused, deferring to the native window close only when the panel is not focused or does not handle the action.Macroscope summarized 137c501.