-
Notifications
You must be signed in to change notification settings - Fork 964
feat(desktop): V2 preset execution mode parity with V1 #3354
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.
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
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| # V2 Preset Execution Mode Parity | ||
|
|
||
| ## Problem | ||
|
|
||
| `V2PresetsBar.openPresetInNewTab()` ignores `executionMode` -- always creates 1 tab with 1 pane and joins all commands with `" && "`. V1 supports three execution modes that produce different layouts: | ||
|
|
||
| - **`split-pane`**: N commands -> N panes in the active tab | ||
| - **`new-tab`**: N commands -> N separate tabs | ||
| - **`new-tab-split-pane`**: N commands -> 1 new tab with N split panes | ||
|
|
||
| V2's pane store already has multi-pane `addTab()` (with balanced tree) and `addPane()`. The infrastructure exists; it's just not wired up in the preset path. | ||
|
|
||
| Preset hotkeys (OPEN_PRESET_1-9) also show labels in the V2 UI but have no handlers. | ||
|
|
||
| ## Design Decisions | ||
|
|
||
| ### 1. Extract execution logic into a `useV2PresetExecution` hook | ||
|
|
||
| Both bar clicks and hotkeys need the same execution function. Currently the logic is inline in `V2PresetsBar` as a `useCallback`. Extracting it: | ||
|
|
||
| - Lets `useWorkspaceHotkeys` call the same function without duplicating the query or logic | ||
| - Keeps `V2PresetsBar` focused on rendering | ||
| - Makes the execution logic testable | ||
|
|
||
| ### 2. Reuse V1's `getPresetLaunchPlan()` | ||
|
|
||
| 37 lines of pure logic with no V1 store dependency. Takes `{ mode, target, commandCount, hasActiveTab }` and returns one of 5 launch plans. No reason to duplicate it. | ||
|
|
||
| ### 3. Both clicks and hotkeys follow the preset's `executionMode` | ||
|
|
||
| No separate `target` override. The preset's `executionMode` determines the behavior: | ||
|
|
||
| - `split-pane` -> adds panes to the active tab (falls back to new tab with splits if no active tab) | ||
| - `new-tab` -> creates separate tabs, one per command | ||
| - `new-tab-split-pane` -> creates one new tab with N split panes | ||
|
|
||
| This means we always pass `target: "active-tab"` to `getPresetLaunchPlan` for `split-pane` mode presets, and the function handles the fallback internally via `hasActiveTab`. | ||
|
|
||
| ### 4. Rename `onOpenInNewTab` -> `onExecutePreset` | ||
|
|
||
| Descriptive: the callback executes the preset according to its configured mode. | ||
|
|
||
| ## Implementation | ||
|
|
||
| ### New: `useV2PresetExecution` hook | ||
|
|
||
| `v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts` | ||
|
|
||
| - Accepts `store`, `workspaceId`, `projectId` | ||
| - Queries presets via `useLiveQuery` + `filterMatchingPresetsForProject` | ||
| - Exposes `executePreset(preset)` and `matchedPresets` | ||
| - Derives the `target` from the preset's `executionMode`: | ||
| - `split-pane` -> `target: "active-tab"` | ||
| - `new-tab` / `new-tab-split-pane` -> `target: "new-tab"` | ||
| - Maps `getPresetLaunchPlan()` result to V2 store calls: | ||
|
|
||
| | Plan | Store Call | | ||
| |---|---| | ||
| | `new-tab-single` | `addTab({ panes: [1 pane] })` | | ||
| | `new-tab-multi-pane` | `addTab({ panes: [N panes] })` (auto balanced tree) | | ||
| | `new-tab-per-command` | `addTab()` x N, 1 pane each | | ||
| | `active-tab-single` | `addPane({ tabId, pane })`, fallback to new-tab | | ||
| | `active-tab-multi-pane` | `addPane()` x N, fallback to new-tab | | ||
|
|
||
| Each command gets its own pane with `initialCommand` (no `&&` joining). | ||
|
|
||
| ### Modify: `V2PresetsBar.tsx` | ||
|
|
||
| - Remove inline `openPresetInNewTab`, preset querying, `matchedPresets` derivation | ||
| - Receive `executePreset` and `matchedPresets` via props from `WorkspaceContent` (which calls `useV2PresetExecution`) | ||
| - Pass `executePreset` to `V2PresetBarItem` as `onExecutePreset` | ||
|
|
||
| ### Modify: `V2PresetBarItem.tsx` | ||
|
|
||
| - Rename `onOpenInNewTab` prop to `onExecutePreset` | ||
| - Context menu label: "Open in new tab" -> "Run preset" | ||
|
|
||
| ### Modify: `useWorkspaceHotkeys.ts` | ||
|
|
||
| - Accept `matchedPresets` and `executePreset` as params | ||
| - Add `useHotkey("OPEN_PRESET_N", () => executePreset(matchedPresets[N-1]))` x 9 | ||
|
|
||
| ### Modify: `page.tsx` | ||
|
|
||
| - Call `useV2PresetExecution({ store, workspaceId, projectId })` in `WorkspaceContent` | ||
| - Pass results to `V2PresetsBar` and `useWorkspaceHotkeys` | ||
|
|
||
| ## File Summary | ||
|
|
||
| | File | Action | | ||
| |---|---| | ||
| | `.../hooks/useV2PresetExecution/useV2PresetExecution.ts` | Create | | ||
| | `.../hooks/useV2PresetExecution/index.ts` | Create | | ||
| | `.../V2PresetsBar/V2PresetsBar.tsx` | Simplify, use hook | | ||
| | `.../V2PresetBarItem/V2PresetBarItem.tsx` | Rename prop + label | | ||
| | `.../useWorkspaceHotkeys/useWorkspaceHotkeys.ts` | Add preset hotkeys | | ||
| | `.../v2-workspace/$workspaceId/page.tsx` | Wire hook | | ||
| | `renderer/stores/tabs/preset-launch.ts` | Reuse as-is | | ||
|
|
||
| ## Verification | ||
|
|
||
| 1. `bun run typecheck && bun run lint:fix` | ||
| 2. Manual: multi-command presets with each mode, hotkeys Ctrl+1-9, single/empty edge cases | ||
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
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
1 change: 1 addition & 0 deletions
1
...s/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/index.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { useV2PresetExecution } from "./useV2PresetExecution"; |
140 changes: 140 additions & 0 deletions
140
...d/_dashboard/v2-workspace/$workspaceId/hooks/useV2PresetExecution/useV2PresetExecution.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| import type { CreatePaneInput, WorkspaceStore } from "@superset/panes"; | ||
| import { useLiveQuery } from "@tanstack/react-db"; | ||
| import { useCallback, useMemo } from "react"; | ||
| import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider"; | ||
| import type { V2TerminalPresetRow } from "renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal"; | ||
| import { getPresetLaunchPlan } from "renderer/stores/tabs/preset-launch"; | ||
| import { filterMatchingPresetsForProject } from "shared/preset-project-targeting"; | ||
| import type { StoreApi } from "zustand/vanilla"; | ||
| import type { PaneViewerData, TerminalPaneData } from "../../types"; | ||
|
|
||
| function makeTerminalPane(command?: string): CreatePaneInput<PaneViewerData> { | ||
| return { | ||
| kind: "terminal", | ||
| data: { | ||
| terminalId: crypto.randomUUID(), | ||
| initialCommand: command, | ||
| } as TerminalPaneData, | ||
| }; | ||
| } | ||
|
|
||
| function resolveTarget(executionMode: V2TerminalPresetRow["executionMode"]) { | ||
| return executionMode === "split-pane" ? "active-tab" : "new-tab"; | ||
| } | ||
|
|
||
| interface UseV2PresetExecutionArgs { | ||
| store: StoreApi<WorkspaceStore<PaneViewerData>>; | ||
| projectId: string; | ||
| } | ||
|
|
||
| export function useV2PresetExecution({ | ||
| store, | ||
| projectId, | ||
| }: UseV2PresetExecutionArgs) { | ||
|
Kitenite marked this conversation as resolved.
|
||
| const collections = useCollections(); | ||
|
|
||
| const { data: allPresets = [] } = useLiveQuery( | ||
| (query) => | ||
| query | ||
| .from({ v2TerminalPresets: collections.v2TerminalPresets }) | ||
| .orderBy(({ v2TerminalPresets }) => v2TerminalPresets.tabOrder), | ||
| [collections], | ||
| ); | ||
|
|
||
| const matchedPresets = useMemo( | ||
| () => filterMatchingPresetsForProject(allPresets, projectId), | ||
| [allPresets, projectId], | ||
| ); | ||
|
|
||
| const executePreset = useCallback( | ||
| (preset: V2TerminalPresetRow) => { | ||
| const state = store.getState(); | ||
| const activeTabId = state.activeTabId; | ||
| const target = resolveTarget(preset.executionMode); | ||
|
|
||
| const plan = getPresetLaunchPlan({ | ||
| mode: preset.executionMode, | ||
| target, | ||
| commandCount: preset.commands.length, | ||
| hasActiveTab: !!activeTabId, | ||
| }); | ||
|
|
||
| switch (plan) { | ||
| case "new-tab-single": { | ||
| state.addTab({ | ||
| titleOverride: preset.name || "Terminal", | ||
| panes: [makeTerminalPane(preset.commands[0])], | ||
| }); | ||
| break; | ||
| } | ||
|
|
||
| case "new-tab-multi-pane": { | ||
| const panes = preset.commands.map((cmd) => makeTerminalPane(cmd)); | ||
| state.addTab({ | ||
| titleOverride: preset.name || "Terminal", | ||
| panes: | ||
| panes.length > 0 | ||
| ? (panes as [ | ||
| CreatePaneInput<PaneViewerData>, | ||
| ...CreatePaneInput<PaneViewerData>[], | ||
| ]) | ||
| : [makeTerminalPane()], | ||
| }); | ||
| break; | ||
| } | ||
|
|
||
| case "new-tab-per-command": { | ||
| for (const command of preset.commands) { | ||
| state.addTab({ | ||
| titleOverride: preset.name || "Terminal", | ||
| panes: [makeTerminalPane(command)], | ||
| }); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case "active-tab-single": { | ||
| if (!activeTabId) { | ||
| state.addTab({ | ||
| titleOverride: preset.name || "Terminal", | ||
| panes: [makeTerminalPane(preset.commands[0])], | ||
| }); | ||
| break; | ||
| } | ||
| state.addPane({ | ||
| tabId: activeTabId, | ||
| pane: makeTerminalPane(preset.commands[0]), | ||
| }); | ||
| break; | ||
| } | ||
|
|
||
| case "active-tab-multi-pane": { | ||
| if (!activeTabId) { | ||
| const panes = preset.commands.map((cmd) => makeTerminalPane(cmd)); | ||
| state.addTab({ | ||
| titleOverride: preset.name || "Terminal", | ||
| panes: | ||
| panes.length > 0 | ||
| ? (panes as [ | ||
| CreatePaneInput<PaneViewerData>, | ||
| ...CreatePaneInput<PaneViewerData>[], | ||
| ]) | ||
| : [makeTerminalPane()], | ||
| }); | ||
| break; | ||
| } | ||
| for (const command of preset.commands) { | ||
| state.addPane({ | ||
| tabId: activeTabId, | ||
| pane: makeTerminalPane(command), | ||
| }); | ||
| } | ||
| break; | ||
| } | ||
|
Kitenite marked this conversation as resolved.
|
||
| } | ||
| }, | ||
| [store], | ||
| ); | ||
|
|
||
| return { matchedPresets, executePreset }; | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.