feat(desktop): add preset keyboard shortcuts and drag-and-drop reordering#939
feat(desktop): add preset keyboard shortcuts and drag-and-drop reordering#939
Conversation
Press NEW_GROUP hotkey (Cmd+T by default) followed by a number key (1-9) within 300ms to open a new tab with the preset at that position. If no number is pressed, falls back to default preset behavior. - Add preset-chord-store for chord state management - Add usePresetChordShortcut hook with chord logic - Add PresetChordIndicator for visual feedback during chord wait - Respects user's customized hotkey bindings
Add unit tests for the chord shortcut store and hook logic including state machine behavior, hotkey detection, and preset index calculation.
- Fix test comment: update chord timeout from 300ms to DEFAULT_CHORD_TIMEOUT_MS (500ms) - Remove redundant clearTimeout in timeout test (was no-op after timeout fired) - Wrap handleChordTimeoutCommit in useCallback for consistency
- Remove unused 'open_default' action type from ChordAction union - Extract MAX_VISIBLE_PRESETS to shared constants file - Update imports in PresetChordIndicator and tests
Add the ability to reorder terminal presets via drag-and-drop in the Settings page. Uses the existing react-dnd library with a drag handle pattern for clear interaction.
- Add .int().min(0) validation to reorderTerminalPresets indices - Fix server mutation firing on every hover by deferring to drop event - Fix index-based server lookups to use ID-based lookup after reorder
Resolve conflicts in TerminalSettings.tsx: - Keep useCallback wrappers from PR branch - Add handleExecutionModeChange from main - Add HiOutlineQuestionMarkCircle import from main
- Remove unused isOver state from PresetRow drag-and-drop (visual feedback already provided by optimistic reorder) - Use ID-based reorder mutation instead of indices to prevent race conditions when multiple reorders occur quickly - Extract preset hotkeys into usePresetHotkeys hook to reduce repetitive code in WorkspacePage
Group keyboard shortcuts into separate tables by category (Workspace, Terminal, Layout, Window, Help) for better organization and readability. Search still filters across all categories.
Rename hotkey constants and labels to match actual UI terminology:
- PREV_TERMINAL → PREV_TAB ("Previous Tab")
- NEXT_TERMINAL → NEXT_TAB ("Next Tab")
Remove comments that restate what code obviously does: - Remove "helper to open a tab" comment (function name is clear) - Remove "switch between tabs/panes" comments (hotkey IDs are clear) - Remove "update local state optimistically" comment (function name is clear) - Remove "persist to server" comment (function name is clear) - Remove "if already default" comment (ternary is self-explanatory)
📝 WalkthroughWalkthroughThis PR adds terminal preset reordering functionality via drag-and-drop backed by a new TRPC mutation, introduces nine preset hotkeys (meta+shift+1-9), renames terminal navigation hotkeys from PREV/NEXT_TERMINAL to PREV/NEXT_TAB, refactors keyboard shortcuts UI to filter hotkeys per category, and integrates preset opening into the workspace dashboard with hotkey support. Changes
Sequence DiagramssequenceDiagram
actor User
participant UI as PresetRow UI
participant TerminalSettings
participant ReactQuery as React Query Hook
participant TRPC as TRPC Router
participant DB as Database
User->>UI: Drag preset row
UI->>TerminalSettings: onLocalReorder(fromIdx, toIdx)
TerminalSettings->>TerminalSettings: Reorder localPresets via splice
TerminalSettings->>UI: Update visual state
User->>UI: Drop preset row
UI->>TerminalSettings: onPersistReorder(presetId, targetIdx)
TerminalSettings->>ReactQuery: reorderPresets.mutate({presetId, targetIndex})
ReactQuery->>TRPC: electronTrpc.settings.reorderTerminalPresets(input)
TRPC->>DB: Persist reordered presets
DB-->>TRPC: Confirm update
TRPC-->>ReactQuery: Return success
ReactQuery->>ReactQuery: Invalidate getTerminalPresets cache
ReactQuery-->>UI: UI re-renders with persisted state
sequenceDiagram
actor User
participant Hotkey as Hotkey System
participant WorkspacePage as Workspace Page
participant PresetHook as usePresetHotkeys
participant PresetAPI as usePresets Hook
participant Tab as Tab Manager
User->>Hotkey: Press meta+shift+<n>
Hotkey->>PresetHook: Trigger OPEN_PRESET_N action
PresetHook->>WorkspacePage: Call openTabWithPreset(presetIndex)
WorkspacePage->>PresetAPI: Fetch preset at presetIndex
PresetAPI-->>WorkspacePage: Return preset (commands, cwd, name)
WorkspacePage->>Tab: Create new tab with preset.cwd
WorkspacePage->>Tab: Execute preset.initialCommands
alt preset.name exists
WorkspacePage->>Tab: Rename tab to preset.name
end
Tab-->>User: New tab with preset configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
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
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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx`:
- Around line 102-125: The drag logic needs an end handler to revert local
mutations when a drag is cancelled: update the useDrag call (the hook that
returns [{ isDragging }, drag, preview]) to add an end: (item, monitor) => { if
(!monitor.didDrop()) { /* revert */ onLocalReorder(item.index,
item.originalIndex) and set item.index = item.originalIndex } } so any
hover-mutated item.index is restored when monitor.didDrop() is false; also
include onLocalReorder in the useDrag dependency array and keep references to
PRESET_TYPE, preset.id, rowIndex, onPersistReorder unchanged.
🧹 Nitpick comments (4)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/hooks/usePresetHotkeys.ts (1)
16-46: Consider reducing repetition with a loop.The nine nearly-identical
useAppHotkeycalls can be consolidated. Since the hook count is fixed, a loop is safe here (no conditional hook calls).♻️ Suggested refactor
export function usePresetHotkeys( openTabWithPreset: (presetIndex: number) => void, ) { - useAppHotkey(PRESET_HOTKEY_IDS[0], () => openTabWithPreset(0), undefined, [ - openTabWithPreset, - ]); - useAppHotkey(PRESET_HOTKEY_IDS[1], () => openTabWithPreset(1), undefined, [ - openTabWithPreset, - ]); - useAppHotkey(PRESET_HOTKEY_IDS[2], () => openTabWithPreset(2), undefined, [ - openTabWithPreset, - ]); - useAppHotkey(PRESET_HOTKEY_IDS[3], () => openTabWithPreset(3), undefined, [ - openTabWithPreset, - ]); - useAppHotkey(PRESET_HOTKEY_IDS[4], () => openTabWithPreset(4), undefined, [ - openTabWithPreset, - ]); - useAppHotkey(PRESET_HOTKEY_IDS[5], () => openTabWithPreset(5), undefined, [ - openTabWithPreset, - ]); - useAppHotkey(PRESET_HOTKEY_IDS[6], () => openTabWithPreset(6), undefined, [ - openTabWithPreset, - ]); - useAppHotkey(PRESET_HOTKEY_IDS[7], () => openTabWithPreset(7), undefined, [ - openTabWithPreset, - ]); - useAppHotkey(PRESET_HOTKEY_IDS[8], () => openTabWithPreset(8), undefined, [ - openTabWithPreset, - ]); + for (let i = 0; i < PRESET_HOTKEY_IDS.length; i++) { + // eslint-disable-next-line react-hooks/rules-of-hooks + useAppHotkey(PRESET_HOTKEY_IDS[i], () => openTabWithPreset(i), undefined, [ + openTabWithPreset, + ]); + } }Note: The lint disable is needed because the linter can't statically verify the loop count is constant, but since
PRESET_HOTKEY_IDSis a fixed-length constant array, this is safe.apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx (1)
70-82: Use object parameters for the new reorder callbacks.
The newonLocalReorder/onPersistReordersignatures use positional params; guidelines prefer a single params object for 2+ arguments. This will also make call sites clearer.As per coding guidelines, prefer object parameters for functions with 2+ arguments.
Also applies to: 115-124
apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/TerminalSettings.tsx (2)
154-158: AvoiduseEffectfor ref mirroring if a render-time assignment suffices.
Since you only need the latestserverPresetsin callbacks, you can assignserverPresetsRef.current = serverPresetsduring render and drop the effect.Based on learnings, avoid
useEffectunless absolutely necessary.
316-333: Use object parameters for reorder handlers and props.
handleLocalReorder(fromIndex, toIndex)/handlePersistReorder(presetId, targetIndex)and their prop usage should be refactored to{ fromIndex, toIndex }/{ presetId, targetIndex }for clarity and extensibility.As per coding guidelines, prefer object parameters for functions with 2+ arguments.
Also applies to: 648-649
| const [{ isDragging }, drag, preview] = useDrag( | ||
| () => ({ | ||
| type: PRESET_TYPE, | ||
| item: { id: preset.id, index: rowIndex, originalIndex: rowIndex }, | ||
| collect: (monitor) => ({ | ||
| isDragging: monitor.isDragging(), | ||
| }), | ||
| }), | ||
| [preset.id, rowIndex], | ||
| ); | ||
|
|
||
| const [, drop] = useDrop({ | ||
| accept: PRESET_TYPE, | ||
| hover: (item: { id: string; index: number; originalIndex: number }) => { | ||
| if (item.index !== rowIndex) { | ||
| onLocalReorder(item.index, rowIndex); | ||
| item.index = rowIndex; | ||
| } | ||
| }, | ||
| drop: (item: { id: string; index: number; originalIndex: number }) => { | ||
| if (item.originalIndex !== item.index) { | ||
| onPersistReorder(item.id, item.index); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the PresetRow.tsx file
fd -t f "PresetRow.tsx" --path "*settings/terminal*"Repository: superset-sh/superset
Length of output: 297
🏁 Script executed:
# Read the full file to understand the context and callback signatures
cat -n "apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx"Repository: superset-sh/superset
Length of output: 7617
🏁 Script executed:
# Search for usages of onLocalReorder and onPersistReorder to understand how they're called
rg -A 3 "onLocalReorder|onPersistReorder" apps/desktop/src/renderer/routes/_authenticated/settings/terminal/Repository: superset-sh/superset
Length of output: 3569
🏁 Script executed:
# Let's look at the parent component TerminalSettings.tsx to understand how the callbacks are implemented
cat -n "apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/TerminalSettings.tsx" | head -200Repository: superset-sh/superset
Length of output: 6584
🏁 Script executed:
# Search for handleLocalReorder and handlePersistReorder implementations
rg -B 5 -A 10 "handleLocalReorder|handlePersistReorder" apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/TerminalSettings.tsxRepository: superset-sh/superset
Length of output: 1362
🏁 Script executed:
# Verify react-dnd behavior: check if onLocalReorder is in dependency array of useDrag end handler
rg "useDrag" apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx -A 10Repository: superset-sh/superset
Length of output: 713
Handle drag cancellation to prevent stale local state.
When a drag is cancelled (dropped outside a valid drop target), the hover callback may have already mutated item.index, but drop won't fire to persist the change. This leaves the UI out of sync with the server. Add an end handler to useDrag that reverts the index when monitor.didDrop() is false, and update the dependency array to include onLocalReorder.
🔧 Suggested fix
const [{ isDragging }, drag, preview] = useDrag(
() => ({
type: PRESET_TYPE,
item: { id: preset.id, index: rowIndex, originalIndex: rowIndex },
+ end: (item, monitor) => {
+ if (!monitor.didDrop()) {
+ onLocalReorder(item.index, item.originalIndex);
+ item.index = item.originalIndex;
+ }
+ },
collect: (monitor) => ({
isDragging: monitor.isDragging(),
}),
}),
- [preset.id, rowIndex],
+ [preset.id, rowIndex, onLocalReorder],
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [{ isDragging }, drag, preview] = useDrag( | |
| () => ({ | |
| type: PRESET_TYPE, | |
| item: { id: preset.id, index: rowIndex, originalIndex: rowIndex }, | |
| collect: (monitor) => ({ | |
| isDragging: monitor.isDragging(), | |
| }), | |
| }), | |
| [preset.id, rowIndex], | |
| ); | |
| const [, drop] = useDrop({ | |
| accept: PRESET_TYPE, | |
| hover: (item: { id: string; index: number; originalIndex: number }) => { | |
| if (item.index !== rowIndex) { | |
| onLocalReorder(item.index, rowIndex); | |
| item.index = rowIndex; | |
| } | |
| }, | |
| drop: (item: { id: string; index: number; originalIndex: number }) => { | |
| if (item.originalIndex !== item.index) { | |
| onPersistReorder(item.id, item.index); | |
| } | |
| }, | |
| const [{ isDragging }, drag, preview] = useDrag( | |
| () => ({ | |
| type: PRESET_TYPE, | |
| item: { id: preset.id, index: rowIndex, originalIndex: rowIndex }, | |
| end: (item, monitor) => { | |
| if (!monitor.didDrop()) { | |
| onLocalReorder(item.index, item.originalIndex); | |
| item.index = item.originalIndex; | |
| } | |
| }, | |
| collect: (monitor) => ({ | |
| isDragging: monitor.isDragging(), | |
| }), | |
| }), | |
| [preset.id, rowIndex, onLocalReorder], | |
| ); | |
| const [, drop] = useDrop({ | |
| accept: PRESET_TYPE, | |
| hover: (item: { id: string; index: number; originalIndex: number }) => { | |
| if (item.index !== rowIndex) { | |
| onLocalReorder(item.index, rowIndex); | |
| item.index = rowIndex; | |
| } | |
| }, | |
| drop: (item: { id: string; index: number; originalIndex: number }) => { | |
| if (item.originalIndex !== item.index) { | |
| onPersistReorder(item.id, item.index); | |
| } | |
| }, |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/terminal/components/TerminalSettings/components/PresetRow/PresetRow.tsx`
around lines 102 - 125, The drag logic needs an end handler to revert local
mutations when a drag is cancelled: update the useDrag call (the hook that
returns [{ isDragging }, drag, preview]) to add an end: (item, monitor) => { if
(!monitor.didDrop()) { /* revert */ onLocalReorder(item.index,
item.originalIndex) and set item.index = item.originalIndex } } so any
hover-mutated item.index is restored when monitor.didDrop() is false; also
include onLocalReorder in the useDrag dependency array and keep references to
PRESET_TYPE, preset.id, rowIndex, onPersistReorder unchanged.
Summary
Supersedes #876
Changes from original PR
isOverstate from drag-and-dropusePresetHotkeyshook to reduce repetitive codeTest plan
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.