Skip to content

feat(desktop): pane context menus + binary tree layout #3196

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
saddlepaddle merged 2 commits into from
Apr 6, 2026
Merged

feat(desktop): pane context menus + binary tree layout #3196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0610b51
feat(desktop): add pane context menus + rewrite layout to binary tree
saddlepaddle Apr 6, 2026
6a5c725
fix: resolve lint warnings (non-null assertions, unused suppression)
saddlepaddle Apr 6, 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
24 changes: 24 additions & 0 deletions apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,30 @@ class TerminalRuntimeRegistryImpl {
this.entries.delete(terminalId);
}

getSelection(terminalId: string): string {
const entry = this.entries.get(terminalId);
return entry?.runtime?.terminal.getSelection() ?? "";
}

clear(terminalId: string): void {
const entry = this.entries.get(terminalId);
entry?.runtime?.terminal.clear();
}

scrollToBottom(terminalId: string): void {
const entry = this.entries.get(terminalId);
entry?.runtime?.terminal.scrollToBottom();
}

paste(terminalId: string, text: string): void {
const entry = this.entries.get(terminalId);
entry?.runtime?.terminal.paste(text);
}

getTerminal(terminalId: string) {
return this.entries.get(terminalId)?.runtime?.terminal ?? null;
}

getAllTerminalIds(): Set<string> {
return new Set(this.entries.keys());
}
Expand Down
1 change: 1 addition & 0 deletions ...nticated/_dashboard/v2-workspace/$workspaceId/hooks/useDefaultContextMenuActions/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { useDefaultContextMenuActions } from "./useDefaultContextMenuActions";
160 changes: 160 additions & 0 deletions ...orkspace/$workspaceId/hooks/useDefaultContextMenuActions/useDefaultContextMenuActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import type { ContextMenuActionConfig, RendererContext } from "@superset/panes";
import { useMemo } from "react";
import {
LuColumns2,
LuEqual,
LuGlobe,
LuMessageSquare,
LuMoveRight,
LuPlus,
LuRows2,
LuX,
} from "react-icons/lu";
import { useHotkeyDisplay } from "renderer/hotkeys";
import type {
BrowserPaneData,
ChatPaneData,
PaneViewerData,
TerminalPaneData,
} from "../../types";

export function useDefaultContextMenuActions(): ContextMenuActionConfig<PaneViewerData>[] {
const splitDownShortcut = useHotkeyDisplay("SPLIT_DOWN").text;
const splitRightShortcut = useHotkeyDisplay("SPLIT_RIGHT").text;
const splitWithChatShortcut = useHotkeyDisplay("SPLIT_WITH_CHAT").text;
const splitWithBrowserShortcut = useHotkeyDisplay("SPLIT_WITH_BROWSER").text;
const equalizePaneSplitsShortcut = useHotkeyDisplay(
"EQUALIZE_PANE_SPLITS",
).text;
const closePaneShortcut = useHotkeyDisplay("CLOSE_TERMINAL").text;

@coderabbitai coderabbitai Bot Apr 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's a separate CLOSE_PANE hotkey defined
rg -n "CLOSE_PANE|CLOSE_TERMINAL" --type=ts -C2 apps/desktop/src/renderer/hotkeys/

Repository: superset-sh/superset

Length of output: 874

🏁 Script executed:

sed -n '25,35p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/hooks/useDefaultContextMenuActions/useDefaultContextMenuActions.tsx

Repository: superset-sh/superset

Length of output: 437

Use CLOSE_PANE hotkey instead of CLOSE_TERMINAL for the generic close pane action.

Line 29 uses useHotkeyDisplay("CLOSE_TERMINAL") for closePaneShortcut, but a dedicated CLOSE_PANE hotkey is defined in the registry (with label "Close Pane"). The "Close Pane" action is generic and applies to all pane types, not just terminals, so it should reference CLOSE_PANE to match the semantic intent.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useDefaultContextMenuActions/useDefaultContextMenuActions.tsx
at line 29, Replace the terminal-specific hotkey with the generic pane hotkey:
change the useHotkeyDisplay call that sets closePaneShortcut from
useHotkeyDisplay("CLOSE_TERMINAL") to useHotkeyDisplay("CLOSE_PANE") so the
closePaneShortcut variable uses the registry's "Close Pane" binding; update any
references to closePaneShortcut in the same hook/component to ensure they now
reflect the generic CLOSE_PANE hotkey.


return useMemo<ContextMenuActionConfig<PaneViewerData>[]>(
() => [
{
key: "split-horizontal",
label: "Split Horizontally",
icon: <LuRows2 />,
shortcut:
splitDownShortcut !== "Unassigned" ? splitDownShortcut : undefined,
onSelect: (ctx) => {
ctx.actions.split("down", {
kind: "terminal",
data: {
terminalId: crypto.randomUUID(),
} as TerminalPaneData,
});
},
},
{
key: "split-vertical",
label: "Split Vertically",
icon: <LuColumns2 />,
shortcut:
splitRightShortcut !== "Unassigned" ? splitRightShortcut : undefined,
onSelect: (ctx) => {
ctx.actions.split("right", {
kind: "terminal",
data: {
terminalId: crypto.randomUUID(),
} as TerminalPaneData,
});
},
},
{
key: "split-with-chat",
label: "Split with New Chat",
icon: <LuMessageSquare />,
shortcut:
splitWithChatShortcut !== "Unassigned"
? splitWithChatShortcut
: undefined,
onSelect: (ctx) => {
ctx.actions.split("right", {
kind: "chat",
data: { sessionId: null } as ChatPaneData,
});
},

@greptile-apps greptile-apps Bot Apr 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 CLOSE_TERMINAL shortcut displayed for all pane types

closePaneShortcut is derived from useHotkeyDisplay("CLOSE_TERMINAL") and used for the generic “Close Pane” action shown in every pane’s context menu. If CLOSE_TERMINAL is terminal-specific, right-clicking a browser or chat pane will show a misleading shortcut hint. Consider a generic CLOSE_PANE hotkey ID or omit the shortcut from the default and let each pane-kind override add the correct binding.

Suggested change
},
const closePaneShortcut = useHotkeyDisplay("CLOSE_PANE").text;

},
{
key: "split-with-browser",
label: "Split with New Browser",
icon: <LuGlobe />,
shortcut:
splitWithBrowserShortcut !== "Unassigned"
? splitWithBrowserShortcut
: undefined,
onSelect: (ctx) => {
ctx.actions.split("right", {
kind: "browser",
data: {
url: "http://localhost:3000",
mode: "preview",
} as BrowserPaneData,
});
},
},
{
key: "equalize-splits",
label: "Equalize Pane Splits",
icon: <LuEqual />,
shortcut:
equalizePaneSplitsShortcut !== "Unassigned"
? equalizePaneSplitsShortcut
: undefined,
onSelect: (ctx) => {
ctx.store.getState().equalizeTab({ tabId: ctx.tab.id });
},
},
{ key: "sep-move", type: "separator" },
{
key: "move-to-tab",
label: "Move to Tab",
icon: <LuMoveRight />,
children: (ctx: RendererContext<PaneViewerData>) => {
const tabs = ctx.store.getState().tabs;
const otherTabs = tabs.filter((t) => t.id !== ctx.tab.id);
const items: ContextMenuActionConfig<PaneViewerData>[] =
otherTabs.map((tab) => ({
key: `move-to-${tab.id}`,
label: tab.titleOverride ?? tab.id,
onSelect: () => {
ctx.store
.getState()
.movePaneToTab({ paneId: ctx.pane.id, targetTabId: tab.id });
},
}));
if (otherTabs.length > 0) {
items.push({ key: "sep-new-tab", type: "separator" });
}
items.push({
key: "move-to-new-tab",
label: "New Tab",
icon: <LuPlus />,
onSelect: () => {
ctx.store.getState().movePaneToNewTab({ paneId: ctx.pane.id });
},
});
return items;
},
},
{ key: "sep-close", type: "separator" },
{
key: "close-pane",
label: "Close Pane",
icon: <LuX />,
variant: "destructive",
shortcut:
closePaneShortcut !== "Unassigned" ? closePaneShortcut : undefined,
onSelect: (ctx) => ctx.actions.close(),
},
],
[
splitDownShortcut,
splitRightShortcut,
splitWithChatShortcut,
splitWithBrowserShortcut,
equalizePaneSplitsShortcut,
closePaneShortcut,
],
);
}
32 changes: 26 additions & 6 deletions .../v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { RendererContext } from "@superset/panes";
import "@xterm/xterm/css/xterm.css";
import { useEffect, useRef, useSyncExternalStore } from "react";
import { useEffect, useMemo, useRef, useSyncExternalStore } from "react";
import { useHotkey } from "renderer/hotkeys";
import {
type ConnectionState,
terminalRuntimeRegistry,
Expand All @@ -10,6 +11,7 @@ import type {
TerminalPaneData,
} from "renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types";
import { useWorkspaceWsUrl } from "renderer/routes/_authenticated/_dashboard/v2-workspace/providers/WorkspaceTrpcProvider/WorkspaceTrpcProvider";
import { ScrollToBottomButton } from "renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton";
import { useTheme } from "renderer/stores/theme";
import { resolveTerminalThemeType } from "renderer/stores/theme/utils";
import { useTerminalAppearance } from "./hooks/useTerminalAppearance";
Expand Down Expand Up @@ -76,13 +78,31 @@ export function TerminalPane({ ctx, workspaceId }: TerminalPaneProps) {
terminalRuntimeRegistry.updateAppearance(terminalId, appearance);
}, [terminalId, appearance]);

useHotkey("CLEAR_TERMINAL", () => {
terminalRuntimeRegistry.clear(terminalId);
});

useHotkey("SCROLL_TO_BOTTOM", () => {
terminalRuntimeRegistry.scrollToBottom(terminalId);
});

// connectionState in deps ensures terminal ref re-derives after connect/disconnect
// biome-ignore lint/correctness/useExhaustiveDependencies: connectionState is intentionally included to trigger re-derive
const terminal = useMemo(
() => terminalRuntimeRegistry.getTerminal(terminalId),
[terminalId, connectionState],
);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Resolved
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

return (
<div className="flex h-full w-full flex-col p-2">
<div
ref={containerRef}
className="min-h-0 flex-1 overflow-hidden"
style={{ backgroundColor: appearance.background }}
/>
<div className="relative min-h-0 flex-1 overflow-hidden">
<div
ref={containerRef}
Comment on lines 96 to +100

@greptile-apps greptile-apps Bot Apr 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 terminal is permanently null — ScrollToBottomButton never works

The useMemo only lists [terminalId] as dependencies. On the first render the terminal isn’t yet in the registry (attachment happens inside a useEffect that runs after render), so getTerminal(terminalId) returns null and that result is cached. When connectionState later changes and triggers a re-render, useMemo finds terminalId unchanged and returns the cached null — it does not re-run the factory.

The biome-ignore comment claims “connectionState triggers re-derive after attach”, but this is incorrect: useMemo only re-runs when a listed dependency changes, not on every re-render.

Suggested change
return (
<div className="flex h-full w-full flex-col p-2">
<div
ref={containerRef}
className="min-h-0 flex-1 overflow-hidden"
style={{ backgroundColor: appearance.background }}
/>
<div className="relative min-h-0 flex-1 overflow-hidden">
<div
ref={containerRef}
const terminal = useMemo(
() => terminalRuntimeRegistry.getTerminal(terminalId),
[terminalId, connectionState],
);

className="h-full w-full"
style={{ backgroundColor: appearance.background }}
/>
<ScrollToBottomButton terminal={terminal} />
</div>
{connectionState === "closed" && (
<div className="flex items-center gap-2 border-t border-border px-3 py-1.5 text-xs text-muted-foreground">
<span>Disconnected</span>
Expand Down
Loading
Loading