Skip to content

[codex] Add v2 workspace delete hotkey support #4673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kitenite merged 5 commits into from
May 17, 2026
Merged

[codex] Add v2 workspace delete hotkey support #4673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
969d46e
Add v2 workspace delete hotkey support
Kitenite May 17, 2026
2f3f2f9
Show workspace delete hotkey in context menus
Kitenite May 17, 2026
07181cd
Address delete dialog review feedback
Kitenite May 17, 2026
892fe81
Keep Enter as delete dialog confirm
Kitenite May 17, 2026
5786e9b
Simplify v2 delete dialog state
Kitenite May 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion ...ponents/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
import { Button } from "@superset/ui/button";
import { Checkbox } from "@superset/ui/checkbox";
import { Label } from "@superset/ui/label";
import { useId } from "react";
import { useEffect, useId } from "react";
import { shouldConfirmDeleteDialogKey } from "../../utils/shouldConfirmDeleteDialogKey";

interface DestroyConfirmPaneProps {
open: boolean;
Expand Down Expand Up @@ -40,6 +41,20 @@ export function DestroyConfirmPane({
}: DestroyConfirmPaneProps) {
const checkboxId = useId();
const hasWarnings = hasChanges || hasUnpushedCommits;

useEffect(() => {
if (!open || !canConfirm) return;

const handleKeyDown = (event: KeyboardEvent) => {
if (!shouldConfirmDeleteDialogKey(event)) return;
event.preventDefault();
onConfirm();
};

window.addEventListener("keydown", handleKeyDown);
return () => window.removeEventListener("keydown", handleKeyDown);
}, [canConfirm, onConfirm, open]);
Comment on lines +45 to +56
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Enter on Cancel button triggers accidental deletion

The window keydown listener fires during the bubble phase for every keydown in the document, including when focus is on the Cancel button. Pressing Enter with the Cancel button focused calls event.preventDefault() (suppressing the button's native Enter→click activation) and then calls onConfirm(), which deletes the workspace. The user intended to cancel but instead confirms a destructive action.

The same pattern applies to TeardownFailedPane — pressing Enter on its Cancel button calls onForceDelete().

The fix is to exit early when the event target is any button element, letting normal button click-from-Enter behavior proceed unimpeded: add if (event.target instanceof HTMLButtonElement) return; before the shouldConfirmDeleteDialogKey call in both components.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog/components/DestroyConfirmPane/DestroyConfirmPane.tsx
Line: 45-56

Comment:
**Enter on Cancel button triggers accidental deletion**

The `window` keydown listener fires during the bubble phase for every keydown in the document, including when focus is on the Cancel button. Pressing Enter with the Cancel button focused calls `event.preventDefault()` (suppressing the button's native Enter→click activation) and then calls `onConfirm()`, which deletes the workspace. The user intended to cancel but instead confirms a destructive action.

The same pattern applies to `TeardownFailedPane` — pressing Enter on its Cancel button calls `onForceDelete()`.

The fix is to exit early when the event target is any button element, letting normal button click-from-Enter behavior proceed unimpeded: add `if (event.target instanceof HTMLButtonElement) return;` before the `shouldConfirmDeleteDialogKey` call in both components.

How can I resolve this? If you propose a fix, please make it concise.


return (
<AlertDialog open={open} onOpenChange={onOpenChange}>
<AlertDialogContent className="max-w-[340px] gap-0 p-0">
Expand Down
15 changes: 15 additions & 0 deletions ...ponents/DashboardSidebarDeleteDialog/components/TeardownFailedPane/TeardownFailedPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import {
AlertDialogTitle,
} from "@superset/ui/alert-dialog";
import { Button } from "@superset/ui/button";
import { useEffect } from "react";
import stripAnsi from "strip-ansi";
import { shouldConfirmDeleteDialogKey } from "../../utils/shouldConfirmDeleteDialogKey";
import { formatTeardownReason } from "./formatTeardownReason";

interface TeardownFailedPaneProps {
Expand All @@ -30,6 +32,19 @@ export function TeardownFailedPane({
// Strip ANSI so raw PTY bytes render readably in the <pre>.
const cleanTail = stripAnsi(cause.outputTail ?? "");

useEffect(() => {
if (!open) return;

const handleKeyDown = (event: KeyboardEvent) => {
if (!shouldConfirmDeleteDialogKey(event)) return;
event.preventDefault();
onForceDelete();
};

window.addEventListener("keydown", handleKeyDown);
return () => window.removeEventListener("keydown", handleKeyDown);
}, [onForceDelete, open]);

return (
<AlertDialog open={open} onOpenChange={onOpenChange}>
<AlertDialogContent className="max-w-[500px] gap-0 p-0">
Expand Down
43 changes: 43 additions & 0 deletions ...idebar/components/DashboardSidebarDeleteDialog/utils/shouldConfirmDeleteDialogKey.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { describe, expect, test } from "bun:test";
import { shouldConfirmDeleteDialogKey } from "./shouldConfirmDeleteDialogKey";

const plainEnter = {
key: "Enter",
shiftKey: false,
metaKey: false,
ctrlKey: false,
altKey: false,
};

describe("shouldConfirmDeleteDialogKey", () => {
test("accepts unmodified Enter", () => {
expect(shouldConfirmDeleteDialogKey(plainEnter)).toBe(true);
});

test("rejects modified Enter", () => {
expect(shouldConfirmDeleteDialogKey({ ...plainEnter, metaKey: true })).toBe(
false,
);
expect(
shouldConfirmDeleteDialogKey({ ...plainEnter, shiftKey: true }),
).toBe(false);
expect(shouldConfirmDeleteDialogKey({ ...plainEnter, ctrlKey: true })).toBe(
false,
);
expect(shouldConfirmDeleteDialogKey({ ...plainEnter, altKey: true })).toBe(
false,
);
});

test("rejects composition and non-Enter keys", () => {
expect(
shouldConfirmDeleteDialogKey({ ...plainEnter, isComposing: true }),
).toBe(false);
expect(shouldConfirmDeleteDialogKey({ ...plainEnter, keyCode: 229 })).toBe(
false,
);
expect(shouldConfirmDeleteDialogKey({ ...plainEnter, key: " " })).toBe(
false,
);
});
});
22 changes: 22 additions & 0 deletions ...oardSidebar/components/DashboardSidebarDeleteDialog/utils/shouldConfirmDeleteDialogKey.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
interface ConfirmDeleteDialogKeyEvent {
key: string;
shiftKey: boolean;
metaKey: boolean;
ctrlKey: boolean;
altKey: boolean;
isComposing?: boolean;
keyCode?: number;
}

export function shouldConfirmDeleteDialogKey(
event: ConfirmDeleteDialogKeyEvent,
): boolean {
if (event.isComposing || event.keyCode === 229) return false;
return (
event.key === "Enter" &&
!event.shiftKey &&
!event.metaKey &&
!event.ctrlKey &&
!event.altKey
);
}
2 changes: 2 additions & 0 deletions ...shboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export function DashboardSidebarWorkspaceItem({
isLocalWorkspace={hostType === "local-device"}
isPinned={isMainWorkspace && hostType === "local-device"}
onCreateSection={handleCreateSection}
showDeleteHotkey={isActive}
onMoveToSection={(targetSectionId) =>
moveWorkspaceToSection(id, projectId, targetSectionId)
}
Expand Down Expand Up @@ -254,6 +255,7 @@ export function DashboardSidebarWorkspaceItem({
isLocalWorkspace={hostType === "local-device"}
isPinned={isMainWorkspace && hostType === "local-device"}
onOpenInFinder={handleOpenInFinder}
showDeleteHotkey={isActive}
onCopyPath={handleCopyPath}
onCopyBranchName={handleCopyBranchName}
onRemoveFromSidebar={handleRemoveFromSidebar}
Expand Down
10 changes: 10 additions & 0 deletions .../components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
ContextMenuContent,
ContextMenuItem,
ContextMenuSeparator,
ContextMenuShortcut,
ContextMenuSub,
ContextMenuSubContent,
ContextMenuSubTrigger,
Expand All @@ -23,6 +24,7 @@ import {
LuTrash2,
LuX,
} from "react-icons/lu";
import { useHotkeyDisplay } from "renderer/hotkeys";
import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider";
import { useDashboardSidebarHover } from "../../../../providers/DashboardSidebarHoverProvider";

Expand All @@ -32,6 +34,7 @@ interface DashboardSidebarWorkspaceContextMenuProps {
isLocalWorkspace: boolean;
isPinned?: boolean;
isUnread: boolean;
showDeleteHotkey?: boolean;
onCreateSection: () => void;
onMoveToSection: (sectionId: string | null) => void;
onOpenInFinder: () => void;
Expand All @@ -50,6 +53,7 @@ export function DashboardSidebarWorkspaceContextMenu({
isLocalWorkspace,
isPinned = false,
isUnread,
showDeleteHotkey = false,
onCreateSection,
onMoveToSection,
onOpenInFinder,
Expand All @@ -63,6 +67,9 @@ export function DashboardSidebarWorkspaceContextMenu({
}: DashboardSidebarWorkspaceContextMenuProps) {
const collections = useCollections();
const { setContextMenuOpen } = useDashboardSidebarHover();
const deleteHotkeyText = useHotkeyDisplay("CLOSE_WORKSPACE").text;
const showDeleteShortcut =
showDeleteHotkey && deleteHotkeyText !== "Unassigned";
const { data: sections = [] } = useLiveQuery(
(q) =>
q
Expand Down Expand Up @@ -174,6 +181,9 @@ export function DashboardSidebarWorkspaceContextMenu({
>
<LuTrash2 className="size-4 mr-2 text-destructive" />
Delete
{showDeleteShortcut && (
<ContextMenuShortcut>{deleteHotkeyText}</ContextMenuShortcut>
)}
</ContextMenuItem>
) : null}
</ContextMenuContent>
Expand Down
82 changes: 75 additions & 7 deletions apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { eq } from "@tanstack/db";
import { useLiveQuery } from "@tanstack/react-db";
import {
createFileRoute,
Outlet,
Expand All @@ -10,7 +12,10 @@ import { useIsV2CloudEnabled } from "renderer/hooks/useIsV2CloudEnabled";
import { useHotkey } from "renderer/hotkeys";
import { electronTrpc } from "renderer/lib/electron-trpc";
import { DashboardSidebar } from "renderer/routes/_authenticated/_dashboard/components/DashboardSidebar";
import { DashboardSidebarDeleteDialog } from "renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarDeleteDialog";
import { useDashboardSidebarState } from "renderer/routes/_authenticated/hooks/useDashboardSidebarState";
import { useDevSeedV2Sidebar } from "renderer/routes/_authenticated/hooks/useDevSeedV2Sidebar";
import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider";
import { ResizablePanel } from "renderer/screens/main/components/ResizablePanel";
import { WorkspaceSidebar } from "renderer/screens/main/components/WorkspaceSidebar";
import { DeleteWorkspaceDialog } from "renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components";
Expand All @@ -30,10 +35,26 @@ export const Route = createFileRoute("/_authenticated/_dashboard")({
component: DashboardLayout,
});

type DeleteTarget =
| {
version: "v1";
workspaceId: string;
workspaceName: string;
workspaceType: "worktree" | "branch";
}
| {
version: "v2";
workspaceId: string;
workspaceName: string;
open: boolean;
};

function DashboardLayout() {
const navigate = useNavigate();
const openNewWorkspaceModal = useOpenNewWorkspaceModal();
const isV2CloudEnabled = useIsV2CloudEnabled();
const collections = useCollections();
const { removeWorkspaceFromSidebar } = useDashboardSidebarState();
useDevSeedV2Sidebar();
// Get current workspace from route to pre-select project in new workspace modal
const matchRoute = useMatchRoute();
Expand All @@ -47,6 +68,8 @@ function DashboardLayout() {
to: "/v2-workspace/$workspaceId",
fuzzy: true,
});
const currentV2WorkspaceId =
v2WorkspaceMatch !== false ? v2WorkspaceMatch.workspaceId : null;
const onV1WorkspaceRoute = currentWorkspaceMatch !== false;
const onV2WorkspaceRoute = v2WorkspaceMatch !== false;
const versionMismatch =
Expand All @@ -58,6 +81,18 @@ function DashboardLayout() {
{ enabled: !!currentWorkspaceId },
);

const { data: currentV2Workspaces = [] } = useLiveQuery(
(q) =>
q
.from({ workspaces: collections.v2Workspaces })
.where(({ workspaces }) =>
eq(workspaces.id, currentV2WorkspaceId ?? ""),
),
[collections, currentV2WorkspaceId],
);
const currentV2Workspace =
currentV2WorkspaceId != null ? (currentV2Workspaces[0] ?? null) : null;

const {
isOpen: isWorkspaceSidebarOpen,
toggleCollapsed: toggleWorkspaceSidebarCollapsed,
Expand All @@ -83,11 +118,7 @@ function DashboardLayout() {
openNewWorkspaceModal(currentWorkspace?.projectId),
);

const [deleteTarget, setDeleteTarget] = useState<{
workspaceId: string;
workspaceName: string;
workspaceType: "worktree" | "branch";
} | null>(null);
const [deleteTarget, setDeleteTarget] = useState<DeleteTarget | null>(null);

useHotkey(
"CLOSE_WORKSPACE",
Expand All @@ -97,10 +128,31 @@ function DashboardLayout() {
workspaceId: currentWorkspaceId,
workspaceName: currentWorkspace.name,
workspaceType: currentWorkspace.type,
version: "v1",
});
return;
}

if (
currentV2WorkspaceId &&
currentV2Workspace &&
currentV2Workspace.type !== "main"
) {
setDeleteTarget({
workspaceId: currentV2WorkspaceId,
workspaceName: currentV2Workspace.name || currentV2Workspace.branch,
version: "v2",
open: true,
});
}
},
{ enabled: !!currentWorkspaceId },
{
enabled:
(!!currentWorkspaceId && !!currentWorkspace) ||
(!!currentV2WorkspaceId &&
!!currentV2Workspace &&
currentV2Workspace.type !== "main"),
},
);

const sidebarPanel = isWorkspaceSidebarOpen && (
Expand Down Expand Up @@ -152,7 +204,7 @@ function DashboardLayout() {
</div>
<div id="workspace-right-sidebar-slot" className="flex h-full shrink-0" />
<AddRepositoryModals />
{deleteTarget && (
{deleteTarget?.version === "v1" && (
<DeleteWorkspaceDialog
workspaceId={deleteTarget.workspaceId}
workspaceName={deleteTarget.workspaceName}
Expand All @@ -163,6 +215,22 @@ function DashboardLayout() {
}}
/>
)}
{deleteTarget?.version === "v2" && (
<DashboardSidebarDeleteDialog
workspaceId={deleteTarget.workspaceId}
workspaceName={deleteTarget.workspaceName}
open={deleteTarget.open}
onOpenChange={(open) => {
setDeleteTarget((target) =>
target?.version === "v2" ? { ...target, open } : target,
);
}}
Comment on lines +222 to +227
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 V2 dismiss leaks a mounted-but-hidden dialog

When the user dismisses the v2 delete dialog (Escape or clicking outside), onOpenChange(false) sets deleteTarget.open = false but leaves DashboardSidebarDeleteDialog mounted indefinitely. The v1 path sets deleteTarget to null on close, which unmounts the component. Because useDestroyDialogState already resets both error and inspectState to their initial values when open goes false, there is no state worth preserving across a dismiss — setting deleteTarget to null here, as v1 does, would match that behavior and avoid keeping the component alive unnecessarily.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx
Line: 222-227

Comment:
**V2 dismiss leaks a mounted-but-hidden dialog**

When the user dismisses the v2 delete dialog (Escape or clicking outside), `onOpenChange(false)` sets `deleteTarget.open = false` but leaves `DashboardSidebarDeleteDialog` mounted indefinitely. The v1 path sets `deleteTarget` to `null` on close, which unmounts the component. Because `useDestroyDialogState` already resets both `error` and `inspectState` to their initial values when `open` goes `false`, there is no state worth preserving across a dismiss — setting `deleteTarget` to `null` here, as v1 does, would match that behavior and avoid keeping the component alive unnecessarily.

How can I resolve this? If you propose a fix, please make it concise.

onDeleted={() => {
removeWorkspaceFromSidebar(deleteTarget.workspaceId);
setDeleteTarget(null);
}}
/>
)}
</div>
);
}
7 changes: 7 additions & 0 deletions ...rer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/CollapsedWorkspaceItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
ContextMenuContent,
ContextMenuItem,
ContextMenuSeparator,
ContextMenuShortcut,
ContextMenuTrigger,
} from "@superset/ui/context-menu";
import {
Expand All @@ -14,6 +15,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { cn } from "@superset/ui/utils";
import { type RefObject, useMemo, useState } from "react";
import { LuCopy, LuGitBranch, LuX } from "react-icons/lu";
import { useHotkeyDisplay } from "renderer/hotkeys";
import { createContextMenuDeleteDialogCoordinator } from "renderer/react-query/workspaces/useWorkspaceDeleteHandler";
import type { ActivePaneStatus } from "shared/tabs-types";
import { STROKE_WIDTH } from "../constants";
Expand Down Expand Up @@ -69,6 +71,8 @@ export function CollapsedWorkspaceItem({
const [renameBranchTarget, setRenameBranchTarget] = useState<string | null>(
null,
);
const deleteHotkeyText = useHotkeyDisplay("CLOSE_WORKSPACE").text;
const showDeleteShortcut = isActive && deleteHotkeyText !== "Unassigned";

const collapsedButton = (
<button
Expand Down Expand Up @@ -163,6 +167,9 @@ export function CollapsedWorkspaceItem({
>
<LuX className="size-4 mr-2" strokeWidth={STROKE_WIDTH} />
Close Workspace
{showDeleteShortcut && (
<ContextMenuShortcut>{deleteHotkeyText}</ContextMenuShortcut>
)}
</ContextMenuItem>
</ContextMenuContent>
</ContextMenu>
Expand Down
Loading
Loading