feat(desktop): compact add button + presets dropdown#1762
Conversation
📝 WalkthroughWalkthroughThis change introduces a compact terminal add button feature for the desktop application. It adds new UI components (AddTabButton and PresetsSubmenu), corresponding database schema and migration, a new TRPC settings procedure, and updates the build configuration to treat new external dependencies as external modules. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AddTabButton
participant PresetsSubmenu
participant SettingsMutation
participant Server
User->>AddTabButton: Click compact plus button
activate AddTabButton
AddTabButton->>AddTabButton: Render dropdown menu
alt User selects preset
AddTabButton->>PresetsSubmenu: Show presets submenu
activate PresetsSubmenu
User->>PresetsSubmenu: Select preset
PresetsSubmenu-->>AddTabButton: onOpenPreset(preset)
deactivate PresetsSubmenu
AddTabButton->>Server: Navigate to open preset
else User toggles "Use Compact Button"
User->>AddTabButton: Click toggle
AddTabButton->>SettingsMutation: setUseCompactTerminalAddButton
activate SettingsMutation
SettingsMutation->>SettingsMutation: Optimistic update (local state)
SettingsMutation->>Server: Send mutation request
activate Server
Server-->>SettingsMutation: Success/Error response
deactivate Server
alt Error received
SettingsMutation->>SettingsMutation: Rollback to previous state
end
deactivate SettingsMutation
else User selects Terminal/Chat/Browser
User->>AddTabButton: Click action button
AddTabButton-->>AddTabButton: onAddTerminal/Chat/Browser
end
deactivate AddTabButton
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/components/PresetsSubmenu/PresetsSubmenu.tsx (1)
15-15: Decouple this component from route-layer hooks.Line 15 pulls
PRESET_HOTKEY_IDSfrom a route hook module. Please move that constant to a co-located/shared constants module and import it from there to keep this component independent of route-layer code.As per coding guidelines, "Co-locate hooks, utils, constants, config, stories, and dependencies next to the files using them".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/components/PresetsSubmenu/PresetsSubmenu.tsx` at line 15, PresetsSubmenu.tsx currently imports PRESET_HOTKEY_IDS from a route-hook module; extract PRESET_HOTKEY_IDS into a co-located/shared constants module (e.g., create a new constants file next to the PresetsSubmenu component such as presetsConstants or constants.ts), move the constant definition there, update PresetsSubmenu.tsx to import PRESET_HOTKEY_IDS from that new module, and update any other files that reference PRESET_HOTKEY_IDS to import from the new shared constants module so the component is decoupled from route-layer hooks.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx (1)
258-278: Consider memoizing inline mutation callbacks and action handlers.
onToggleShowPresetsBar,onToggleCompactAddButton,handleAddGroup,handleAddChat, andhandleAddBrowserare all recreated on every render.AddTabButtonis currently not memoized so this causes no cascading re-renders, but memoizing them now would make addingReact.memotoAddTabButtonstraightforward in the future.♻️ Example: memoize mutation callbacks
- onToggleShowPresetsBar={(enabled) => - setShowPresetsBar.mutate({ enabled }) - } - onToggleCompactAddButton={(enabled) => - setUseCompactTerminalAddButton.mutate({ enabled }) - }Add before
plusControl:+ const handleToggleShowPresetsBar = useCallback( + (enabled: boolean) => setShowPresetsBar.mutate({ enabled }), + [setShowPresetsBar], + ); + const handleToggleCompactAddButton = useCallback( + (enabled: boolean) => setUseCompactTerminalAddButton.mutate({ enabled }), + [setUseCompactTerminalAddButton], + );Apply the same pattern to
handleAddGroup,handleAddChat, andhandleAddBrowser.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx` around lines 258 - 278, The inline mutation callbacks and action handlers passed into AddTabButton (onToggleShowPresetsBar, onToggleCompactAddButton, handleAddGroup, handleAddChat, handleAddBrowser) are recreated on every render; memoize them with React.useCallback (e.g., create stable callbacks that call setShowPresetsBar.mutate({ enabled }), setUseCompactTerminalAddButton.mutate({ enabled }), and the existing handler logic) and reference those memoized handlers when building plusControl so AddTabButton can be safely wrapped with React.memo later; keep names the same (onToggleShowPresetsBar, onToggleCompactAddButton, handleAddGroup, handleAddChat, handleAddBrowser, plusControl, AddTabButton) so callers remain unchanged.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/AddTabButton.tsx (1)
19-33: ExportAddTabButtonPropsfor type-safe external consumption.The interface is only accessible within this file. Exporting it allows
GroupStrip.tsx(and future callers) to reference the type explicitly rather than relying on inference.♻️ Proposed fix
-interface AddTabButtonProps { +export interface AddTabButtonProps {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/AddTabButton.tsx` around lines 19 - 33, The AddTabButtonProps interface is currently file-local; export it so external components (e.g. GroupStrip.tsx) can import it for type-safe usage. Update the declaration of AddTabButtonProps to be exported (export interface AddTabButtonProps { ... }) and ensure any imports or references in GroupStrip or other callers use this exported type name; no other API changes required.
🤖 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/electron.vite.config.ts`:
- Around line 105-112: The packaging misses two external modules — mastracode
and `@ast-grep/napi` — that must be explicitly included: add "mastracode" and
"@ast-grep/napi" to the NATIVE_MODULE_DEPS (or NATIVE_MODULES) array in
copy-native-modules.ts so they are de-symlinked/copied; add matching glob
patterns for these packages to the asarUnpack array in electron-builder.ts so
native artifacts are unpacked at runtime; and add corresponding entries to the
electron-builder.ts files section using the same copy rules/patterns you used
for "better-sqlite3" so the built app includes the modules. Ensure you reference
the exact symbol names NATIVE_MODULE_DEPS/NATIVE_MODULES, asarUnpack, and files
when making the changes.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/AddTabButton.tsx`:
- Around line 19-33: The AddTabButtonProps interface is currently file-local;
export it so external components (e.g. GroupStrip.tsx) can import it for
type-safe usage. Update the declaration of AddTabButtonProps to be exported
(export interface AddTabButtonProps { ... }) and ensure any imports or
references in GroupStrip or other callers use this exported type name; no other
API changes required.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/components/PresetsSubmenu/PresetsSubmenu.tsx`:
- Line 15: PresetsSubmenu.tsx currently imports PRESET_HOTKEY_IDS from a
route-hook module; extract PRESET_HOTKEY_IDS into a co-located/shared constants
module (e.g., create a new constants file next to the PresetsSubmenu component
such as presetsConstants or constants.ts), move the constant definition there,
update PresetsSubmenu.tsx to import PRESET_HOTKEY_IDS from that new module, and
update any other files that reference PRESET_HOTKEY_IDS to import from the new
shared constants module so the component is decoupled from route-layer hooks.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx`:
- Around line 258-278: The inline mutation callbacks and action handlers passed
into AddTabButton (onToggleShowPresetsBar, onToggleCompactAddButton,
handleAddGroup, handleAddChat, handleAddBrowser) are recreated on every render;
memoize them with React.useCallback (e.g., create stable callbacks that call
setShowPresetsBar.mutate({ enabled }), setUseCompactTerminalAddButton.mutate({
enabled }), and the existing handler logic) and reference those memoized
handlers when building plusControl so AddTabButton can be safely wrapped with
React.memo later; keep names the same (onToggleShowPresetsBar,
onToggleCompactAddButton, handleAddGroup, handleAddChat, handleAddBrowser,
plusControl, AddTabButton) so callers remain unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
apps/desktop/electron.vite.config.tsapps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/AddTabButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/components/PresetsSubmenu/PresetsSubmenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/components/PresetsSubmenu/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/components/AddTabButton/index.tsapps/desktop/src/shared/constants.tspackages/local-db/drizzle/0034_add_use_compact_terminal_add_button_setting.sqlpackages/local-db/drizzle/meta/0034_snapshot.jsonpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.ts
| external: [ | ||
| "electron", | ||
| "better-sqlite3", | ||
| "node-pty", | ||
| "pg-native", | ||
| "mastracode", | ||
| "@ast-grep/napi", | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate and inspect native-module copy script(s) =="
fd 'copy-native-modules\.ts$' apps/desktop -t f -x sh -c 'echo "--- {} ---"; sed -n "1,260p" "{}"'
echo
echo "== Check where mastracode / `@ast-grep/napi` are imported =="
rg -n -C2 --type=ts --type=tsx \
'from\s+["'"'"'](mastracode|@ast-grep/napi)["'"'"']|require\(["'"'"'](mastracode|@ast-grep/napi)["'"'"']\)' \
apps/desktop/src/main apps/desktop/src/preload apps/desktop/src/renderer
echo
echo "== Inspect electron-builder config for files/asarUnpack handling =="
fd '^electron-builder\.(ts|json|ya?ml)$' apps/desktop -t f -x sh -c 'echo "--- {} ---"; sed -n "1,320p" "{}"'Repository: superset-sh/superset
Length of output: 2812
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check where mastracode / `@ast-grep/napi` are imported =="
rg -n -C2 'from\s+["'"'"'](mastracode|@ast-grep/napi)["'"'"']|require\(["'"'"'](mastracode|@ast-grep/napi)["'"'"']\)' \
apps/desktop/src/ || echo "No imports found for these modules"
echo
echo "== Inspect electron-builder config for files/asarUnpack handling =="
fd '(electron-builder|builder)' apps/desktop -t f | head -20
fd 'electron-builder' . -t f | head -20
echo
echo "== Check for any references to mastracode or `@ast-grep` in config/build files =="
rg 'mastracode|@ast-grep' apps/desktop -t ts -t json 2>/dev/null || echo "Not found in configs"Repository: superset-sh/superset
Length of output: 682
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check where mastracode / `@ast-grep/napi` are imported =="
rg -n -C2 'mastracode|@ast-grep/napi' apps/desktop/src/ || echo "No imports found for these modules"
echo
echo "== List all .ts and .json files in apps/desktop root and check for electron-builder config =="
fd -t f '\.json$|\.ts$' apps/desktop | grep -E '(builder|vite|package)' | head -20
echo
echo "== Inspect package.json in apps/desktop for dependencies =="
fd 'package\.json$' apps/desktop -x sh -c 'echo "--- {} ---"; cat "{}"'
echo
echo "== Check for any references to mastracode or `@ast-grep` in vite or build config =="
rg 'mastracode|@ast-grep' apps/desktop --type-list | grep -E 'ts|json' || true
rg -t json -t ts 'mastracode|@ast-grep' apps/desktop || echo "Not found"Repository: superset-sh/superset
Length of output: 8908
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Inspect electron-builder.ts =="
cat apps/desktop/electron-builder.ts
echo
echo "== Inspect electron-builder.canary.ts =="
cat apps/desktop/electron-builder.canary.tsRepository: superset-sh/superset
Length of output: 6859
Add mastracode and @ast-grep/napi to packaging configuration.
Both modules are externalized but missing from the packaging pipeline:
- Not in
copy-native-modules.ts(won't de-symlink if Bun created symlinks) - Not in
electron-builder.tsasarUnpackarray - Not in
electron-builder.tsfilessection
Since these are dependencies (likely pulled in by workspace packages like @superset/chat-mastra), they must be explicitly handled during build to avoid runtime module resolution failures, especially if they contain native artifacts or encounter the Bun 1.3+ symlink issue that affects better-sqlite3 and node-pty.
Add them to:
copy-native-modules.tsNATIVE_MODULE_DEPSarray (orNATIVE_MODULESif required)electron-builder.tsasarUnpackpatternselectron-builder.tsfilessection with explicit copy rules matching the pattern used forbetter-sqlite3
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/electron.vite.config.ts` around lines 105 - 112, The packaging
misses two external modules — mastracode and `@ast-grep/napi` — that must be
explicitly included: add "mastracode" and "@ast-grep/napi" to the
NATIVE_MODULE_DEPS (or NATIVE_MODULES) array in copy-native-modules.ts so they
are de-symlinked/copied; add matching glob patterns for these packages to the
asarUnpack array in electron-builder.ts so native artifacts are unpacked at
runtime; and add corresponding entries to the electron-builder.ts files section
using the same copy rules/patterns you used for "better-sqlite3" so the built
app includes the modules. Ensure you reference the exact symbol names
NATIVE_MODULE_DEPS/NATIVE_MODULES, asarUnpack, and files when making the
changes.
Summary
Summary by CodeRabbit
New Features
Chores