feat(desktop): add CLOSE_WORKSPACE hotkey and context menu delete option#2907
Merged
Kitenite merged 5 commits intoMar 28, 2026
Merged
Conversation
…ion (superset-sh#2742, superset-sh#2741) Add ⌘+Backspace hotkey to close/delete the active workspace via DeleteWorkspaceDialog, and expose a "Close Worktree"/"Close Workspace" option in the sidebar context menu for all workspace types using the existing deleteDialogCoordinator pattern. Closes superset-sh#2742 Closes superset-sh#2741 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a global CLOSE_WORKSPACE hotkey and wires it into the existing workspace close/delete flow; replaces context-menu Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HotkeySystem as Hotkey System
participant Layout as Dashboard Layout
participant Dialog as DeleteWorkspaceDialog
User->>HotkeySystem: press meta+backspace
HotkeySystem->>Layout: invoke CLOSE_WORKSPACE handler
Layout->>Layout: set deleteTarget (workspaceId/name/type)
Layout->>Dialog: mount with workspace info
Dialog->>User: show confirmation
alt User confirms
User->>Dialog: confirm
Dialog->>Layout: onOpenChange(false) (confirm)
Layout->>Dialog: unmount (clear deleteTarget)
else User cancels
User->>Dialog: cancel
Dialog->>Layout: onOpenChange(false) (cancel)
Layout->>Dialog: unmount (clear deleteTarget)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/shared/hotkeys.test.ts (1)
136-147: Expand conflict coverage beyond Darwin defaults.Line 137 checks only
defaults.darwin. Since Line 463 inapps/desktop/src/shared/hotkeys.tsderives non-mac defaults automatically, add Linux (and optionally win32) conflict assertions to catch future cross-platform collisions.🧪 Suggested test extension
it("does not conflict with existing workspace hotkeys", () => { - const closeDefault = HOTKEYS.CLOSE_WORKSPACE.defaults.darwin; - const workspaceHotkeys = Object.entries(HOTKEYS) + const closeDefaults = HOTKEYS.CLOSE_WORKSPACE.defaults; + const workspaceHotkeys = Object.entries(HOTKEYS) .filter( ([key, def]) => def.category === "Workspace" && key !== "CLOSE_WORKSPACE", ) - .map(([key, def]) => ({ key, darwin: def.defaults.darwin })); + .map(([key, def]) => ({ key, defaults: def.defaults })); for (const hotkey of workspaceHotkeys) { - expect(hotkey.darwin).not.toBe(closeDefault); + expect(hotkey.defaults.darwin).not.toBe(closeDefaults.darwin); + expect(hotkey.defaults.linux).not.toBe(closeDefaults.linux); + // Optional if win32 remains relevant: + // expect(hotkey.defaults.win32).not.toBe(closeDefaults.win32); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/shared/hotkeys.test.ts` around lines 136 - 147, Test only checked Darwin defaults for CLOSE_WORKSPACE; extend it to assert workspace hotkeys don't equal CLOSE_WORKSPACE on other platforms. Update the test in hotkeys.test.ts that iterates workspaceHotkeys (using HOTKEYS and CLOSE_WORKSPACE) to compare each def.defaults.linux (and def.defaults.win32 if present) against HOTKEYS.CLOSE_WORKSPACE.defaults.linux (and .win32) as well as the existing .darwin check so cross-platform collisions are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx`:
- Around line 98-109: The hotkey handler and other parts currently only toggle
showDeleteDialog and read currentWorkspaceId live, which allows the active
workspace to change before confirmation and creates duplicate modal sources (see
useAppHotkey, showDeleteDialog/setShowDeleteDialog, currentWorkspaceId,
DeleteWorkspaceDialog, WorkspaceListItem). Fix by introducing a frozen delete
target state (e.g., deleteTargetId + setDeleteTargetId) and open the single
shared DeleteWorkspaceDialog with that frozen id; update the hotkey handler to
setDeleteTargetId(currentWorkspaceId) then setShowDeleteDialog(true), and
refactor other delete entry points (including WorkspaceListItem) to call a
single openDeleteDialog(deleteId) handler instead of mounting their own
DeleteWorkspaceDialog so all deletes route through one modal owner.
---
Nitpick comments:
In `@apps/desktop/src/shared/hotkeys.test.ts`:
- Around line 136-147: Test only checked Darwin defaults for CLOSE_WORKSPACE;
extend it to assert workspace hotkeys don't equal CLOSE_WORKSPACE on other
platforms. Update the test in hotkeys.test.ts that iterates workspaceHotkeys
(using HOTKEYS and CLOSE_WORKSPACE) to compare each def.defaults.linux (and
def.defaults.win32 if present) against HOTKEYS.CLOSE_WORKSPACE.defaults.linux
(and .win32) as well as the existing .darwin check so cross-platform collisions
are detected.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc0761a6-efcb-4dae-9fb5-bbbd14e2e451
📒 Files selected for processing (6)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceContextMenu.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/shared/hotkeys.test.tsapps/desktop/src/shared/hotkeys.ts
Address CodeRabbit review feedback: - Freeze workspace data at hotkey press time to prevent stale target if the active workspace changes before dialog confirmation - Extend conflict test to also check linux platform defaults
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx:98">
P1: Capture the workspace id/name/type when the hotkey is triggered instead of only toggling a boolean. Using live `currentWorkspace*` values for the dialog can retarget this destructive action if the active workspace changes before confirmation.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/shared/hotkeys.test.ts (2)
99-104: Consider asserting the Linux default binding.The test verifies the Darwin default (
meta+backspace) but doesn't explicitly assert the Linux binding. Per the PR summary, the non-Mac default should bectrl+shift+backspace. Adding this assertion would improve coverage and catch regressions in the cross-platform binding derivation.🔧 Suggested addition
expect(HOTKEYS.CLOSE_WORKSPACE.category).toBe("Workspace"); expect(HOTKEYS.CLOSE_WORKSPACE.defaults.darwin).toBe("meta+backspace"); + expect(HOTKEYS.CLOSE_WORKSPACE.defaults.linux).toBe("ctrl+shift+backspace"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/shared/hotkeys.test.ts` around lines 99 - 104, The test for HOTKEYS.CLOSE_WORKSPACE only checks the Darwin binding—add an assertion to verify the Linux/non-Mac default binding too; specifically, in the same test that references HOTKEYS.CLOSE_WORKSPACE, add an expect(HOTKEYS.CLOSE_WORKSPACE.defaults.linux).toBe("ctrl+shift+backspace") (or the appropriate non-Mac key string) so the cross-platform default binding is covered.
136-149: Consider improving failure messages for easier debugging.The
keyvariable is captured at line 143 but not used. If this test fails, the error won't indicate which hotkey conflicts. Using a more descriptive assertion or including the key in the check would help.🔧 Suggested improvement
for (const hotkey of workspaceHotkeys) { - expect(hotkey.defaults.darwin).not.toBe(closeDefaults.darwin); - expect(hotkey.defaults.linux).not.toBe(closeDefaults.linux); + expect( + hotkey.defaults.darwin, + `${hotkey.key} darwin conflicts with CLOSE_WORKSPACE`, + ).not.toBe(closeDefaults.darwin); + expect( + hotkey.defaults.linux, + `${hotkey.key} linux conflicts with CLOSE_WORKSPACE`, + ).not.toBe(closeDefaults.linux); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/shared/hotkeys.test.ts` around lines 136 - 149, The assertions don't report which hotkey conflicts when they fail; update the loop in the test so failures include the hotkey key (use HOTKEYS.CLOSE_WORKSPACE.defaults, workspaceHotkeys, and hotkey.key). Replace the plain expect(...).not.toBe(...) checks with explicit checks that throw descriptive Errors (or use conditional expect wrappers) that include hotkey.key and the platform (darwin/linux) when a conflict is detected, so failing messages identify the conflicting hotkey.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/shared/hotkeys.test.ts`:
- Around line 99-104: The test for HOTKEYS.CLOSE_WORKSPACE only checks the
Darwin binding—add an assertion to verify the Linux/non-Mac default binding too;
specifically, in the same test that references HOTKEYS.CLOSE_WORKSPACE, add an
expect(HOTKEYS.CLOSE_WORKSPACE.defaults.linux).toBe("ctrl+shift+backspace") (or
the appropriate non-Mac key string) so the cross-platform default binding is
covered.
- Around line 136-149: The assertions don't report which hotkey conflicts when
they fail; update the loop in the test so failures include the hotkey key (use
HOTKEYS.CLOSE_WORKSPACE.defaults, workspaceHotkeys, and hotkey.key). Replace the
plain expect(...).not.toBe(...) checks with explicit checks that throw
descriptive Errors (or use conditional expect wrappers) that include hotkey.key
and the platform (darwin/linux) when a conflict is detected, so failing messages
identify the conflicting hotkey.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8602b31-42ce-4678-b507-c4c1a12b46d2
📒 Files selected for processing (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsxapps/desktop/src/shared/hotkeys.test.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CLOSE_WORKSPACEhotkey (⌘+Backspace/Ctrl+Shift+Backspace) to close or delete the active workspace via the existingDeleteWorkspaceDialogcreateContextMenuDeleteDialogCoordinatorpattern for proper focus coordinationonCloseprop withonDeleteinWorkspaceContextMenuto align with the coordinator-based flowCloses #2742
Closes #2741
Changes
shared/hotkeys.tsCLOSE_WORKSPACEhotkey definition (meta+backspace)layout.tsxDeleteWorkspaceDialogfor active workspaceWorkspaceContextMenu.tsxdeleteDialogCoordinator+onCloseAutoFocuson bothContextMenuContentWorkspaceListItem.tsxonDelete={handleDeleteClick}to context menuhotkeys.test.tsWorkspaceContextMenu.test.tsImplementation notes
NEW_WORKSPACEhotkey registration inDashboardLayoutcreateContextMenuDeleteDialogCoordinatorfromuseWorkspaceDeleteHandler(same pattern asCollapsedWorkspaceItem)meta+backspacewas chosen to match macOS conventions (⌘+Delete = move to trash) and has no conflicts with existing hotkeysenabled: !!currentWorkspaceId)Test plan
CLOSE_WORKSPACEhotkey is defined with correct label, category, and key bindingmeta+backspacematches the correct keyboard eventonDeleteafterrequestOpenDeleteDialog+handleCloseAutoFocus⌘+Backspacewith active workspace → delete dialog opensSummary by cubic
Adds a
CLOSE_WORKSPACEhotkey (⌘+Backspace on macOS; Ctrl+Shift+Backspace on Windows/Linux) and a "Close Worktree"/"Close Workspace" context-menu action that opens the existing delete dialog. The hotkey freezes the current workspace as the delete target to avoid closing the wrong one. Addresses #2742 and #2741.New Features
CLOSE_WORKSPACEhotkey to openDeleteWorkspaceDialogfor the active workspace (enabled only when a workspace is active; target frozen at keypress).createContextMenuDeleteDialogCoordinator; close buttons show the hotkey in tooltips when active.Refactors
WorkspaceContextMenuproponClosetoonDeleteto align with the coordinator flow.Written for commit 2c1bcc1. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests