feat(desktop): add right-click context menu to changes tab file items#658
feat(desktop): add right-click context menu to changes tab file items#658
Conversation
Add a context menu for file items in the Changes tab with the following options: - Open in Split Pane (Horizontal/Vertical) - Open in App (external editor) - Open in Tab (existing or new tab) - Discard Changes (unstaged files only) Uses a shared FileContextMenuProps type throughout the component hierarchy to reduce code duplication. Context menu works for all file categories: against-base, commits, staged, and unstaged.
📝 WalkthroughWalkthroughThis PR introduces context menu support for file items in the Sidebar's ChangesView component. It defines a shared context menu contract, threads props through multiple file list components, creates a new FileItemContextMenu component for rendering contextual actions, adds mutations for discarding/deleting files, and integrates these handlers into the Sidebar to wire tab and file actions. Changes
Sequence DiagramsequenceDiagram
actor User
participant Sidebar
participant ChangesView
participant FileList
participant FileItem as FileItem/ContextMenu
participant TabStore as TabsStore
participant API
User->>FileItem: Right-click file
FileItem->>FileItem: Render context menu
alt User selects "Open in Split"
User->>FileItem: Click action
FileItem->>Sidebar: onOpenInSplitHorizontal(file)
Sidebar->>TabStore: splitPaneHorizontal()
Sidebar->>TabStore: addFileViewerPane(file)
else User selects "Open in App"
User->>FileItem: Click action
FileItem->>Sidebar: onOpenInApp(file)
Sidebar->>API: openInApp mutation
else User selects "Move to Tab"
User->>FileItem: Click action
FileItem->>Sidebar: onMoveToTab(tabId, file)
Sidebar->>TabStore: addTab / switch tab
Sidebar->>TabStore: addFileViewerPane(file)
else User selects "Discard Changes"
User->>FileItem: Click action
FileItem->>ChangesView: onDiscardChanges(file)
ChangesView->>API: discardChangesMutation / deleteUntrackedMutation
API-->>ChangesView: Success, refetch
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx:
- Around line 212-227: handleDiscardChanges currently performs irreversible
actions without confirmation; before invoking deleteUntrackedMutation or
discardChangesMutation, show a confirmation dialog/modal (use the app's existing
confirmation UI or a simple window.confirm) that clearly states the file path
and whether it will be deleted (for untracked) or have changes discarded (for
modified), and only proceed to call deleteUntrackedMutation.mutate or
discardChangesMutation.mutate if the user confirms; for clarity, use distinct
messages for the untracked (delete) and tracked (discard) branches and abort on
cancel.
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx:
- Around line 109-119: The dependency array for the contextMenuProps recreates
on each render because it includes the unstable mutation object openInApp;
destructure the mutate function from openInApp (e.g., const { mutate:
openInAppMutate } = openInApp) and replace openInApp in the dependency list with
openInAppMutate (or, if your react-query version guarantees a stable function,
use openInApp.mutate directly) so that contextMenuProps only updates when the
actual mutate function reference changes; update any uses inside
contextMenuProps to call the destructured mutate symbol instead of openInApp.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx (1)
177-184: Consider simplifying the actions object creation.The explicit mapping is clear, but could be simplified using object rest destructuring if desired.
♻️ Optional simplification
if (contextMenuProps) { - const actions: FileItemContextMenuActions = { - onOpenInSplitHorizontal: contextMenuProps.onOpenInSplitHorizontal, - onOpenInSplitVertical: contextMenuProps.onOpenInSplitVertical, - onOpenInApp: contextMenuProps.onOpenInApp, - onOpenInNewTab: contextMenuProps.onOpenInNewTab, - onMoveToTab: contextMenuProps.onMoveToTab, - onDiscardChanges: contextMenuProps.onDiscardChanges, - }; + const { currentTabId, availableTabs, ...actions } = contextMenuProps; return ( <FileItemContextMenu actions={actions} - currentTabId={contextMenuProps.currentTabId} - availableTabs={contextMenuProps.availableTabs} + currentTabId={currentTabId} + availableTabs={availableTabs} > <ContextMenuTrigger asChild>{content}</ContextMenuTrigger> </FileItemContextMenu> ); }apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx (1)
84-87: Consider making the external app configurable.The app name
"cursor"is hardcoded. This should be extracted to a constant or made configurable to support different editors.💡 Suggested improvement
+const DEFAULT_EXTERNAL_APP = "cursor"; + // ... in contextMenuProps onOpenInApp: (file: ChangedFile) => { const fullPath = `${worktreePath}/${file.path}`; - openInApp.mutate({ path: fullPath, app: "cursor" }); + openInApp.mutate({ path: fullPath, app: DEFAULT_EXTERNAL_APP }); },apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsx (1)
110-128: Consider extracting the per-file context menu props transformation.This exact transformation logic is duplicated in
FileListTree.tsx(lines 160-178). Consider extracting a helper function to reduce duplication and ensure consistency.💡 Example helper function
// In FileList.tsx or a shared utils file export function createFileItemContextMenuProps( contextMenuProps: FileContextMenuProps, file: ChangedFile ): FileItemContextMenuProps { return { currentTabId: contextMenuProps.currentTabId, availableTabs: contextMenuProps.availableTabs, onOpenInSplitHorizontal: () => contextMenuProps.onOpenInSplitHorizontal(file), onOpenInSplitVertical: () => contextMenuProps.onOpenInSplitVertical(file), onOpenInApp: () => contextMenuProps.onOpenInApp(file), onOpenInNewTab: () => contextMenuProps.onOpenInNewTab(file), onMoveToTab: (tabId) => contextMenuProps.onMoveToTab(file, tabId), onDiscardChanges: contextMenuProps.onDiscardChanges ? () => contextMenuProps.onDiscardChanges?.(file) : undefined, }; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItemContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
🧠 Learnings (2)
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.ts
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
🧬 Code graph analysis (6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx (2)
apps/desktop/src/renderer/stores/tabs/types.ts (1)
Tab(18-20)apps/desktop/src/shared/changes-types.ts (1)
ChangedFile(22-28)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx (1)
FileContextMenuProps(11-20)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.ts (2)
FileContextMenuProps(2-2)ChangesView(1-1)apps/desktop/src/shared/changes-types.ts (1)
ChangedFile(22-28)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsx (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx (1)
FileContextMenuProps(11-20)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx (1)
FileContextMenuProps(11-20)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.ts (1)
FileContextMenuProps(1-1)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.ts (1)
FileContextMenuProps(2-2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItemContextMenu.tsx (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx (1)
FileItemContextMenuProps(14-23)packages/ui/src/components/ui/context-menu.tsx (7)
ContextMenu(237-237)ContextMenuContent(239-239)ContextMenuItem(240-240)ContextMenuSeparator(244-244)ContextMenuSub(248-248)ContextMenuSubTrigger(250-250)ContextMenuSubContent(249-249)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx (4)
apps/desktop/src/renderer/stores/tabs/store.ts (1)
useTabsStore(74-881)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.ts (2)
FileContextMenuProps(2-2)ChangesView(1-1)apps/desktop/src/shared/changes-types.ts (1)
ChangedFile(22-28)apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (1)
ChangesView(33-456)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (19)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItemContextMenu.tsx (1)
37-106: LGTM! Clean composition pattern for context menu.The component correctly separates the menu content from the trigger, allowing the parent to control the trigger element via children. The filtering of
targetTabsto exclude the current tab (line 43) prevents redundant "move to same tab" options.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/index.ts (1)
1-3: LGTM!Clean type re-export that extends the public API surface appropriately.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/index.ts (1)
1-2: LGTM!Type re-export enables consumers to access
FileContextMenuPropsat the appropriate module boundary.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/index.ts (1)
1-2: LGTM!Properly exposes the new context menu component and its props type through the barrel export.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/CommitItem/CommitItem.tsx (3)
10-10: LGTM!Import correctly brings in the type alongside the component.
23-24: LGTM!Good use of JSDoc to document the optional prop's purpose.
100-100: LGTM!Correctly forwards
contextMenuPropsto the innerFileListcomponent, enabling context menus on commit file items.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileList.tsx (3)
7-20: LGTM! Well-designed shared context menu contract.The interface correctly uses
(file: ChangedFile) => voidcallbacks, allowing a single props object to be threaded through the component tree. Per-file binding happens at the leaf level, avoiding closure creation overhead at the container level.
38-39: LGTM!Optional prop with clear JSDoc documentation maintains backward compatibility.
71-72: LGTM!Props correctly forwarded to both view mode components.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileItem/FileItem.tsx (2)
14-23: LGTM! Clear per-item props contract.This interface appropriately uses
() => voidcallbacks (pre-bound to the specific file), distinct fromFileContextMenuPropswhich uses(file: ChangedFile) => void. This separation allows clean prop threading from container to leaf.
176-197: LGTM! Clean conditional rendering pattern.The early return pattern for non-context-menu case keeps the code readable. The explicit
actionsobject construction provides clear documentation of what's passed to the menu component.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx (2)
98-106: Good use ofgetState()for synchronous state access.Using
useTabsStore.getState().setActiveTab()ensures the tab switch happens synchronously before adding the pane, avoiding the stale closure issue that would occur with the hook-derivedsetActiveTab.
88-96: The return value fromaddTabshould be captured to explicitly target the newly created tab.While Zustand's
set()is synchronous (so the state is updated immediately), the current code relies on implicit state lookup. It's clearer and more maintainable to capture the{ tabId, paneId }return value fromaddTaband either pass it toaddFileViewerPaneor use it explicitly. This pattern is already used elsewhere in the codebase (e.g.,GroupStrip.tsxline 101).Change from:
addTab(workspaceId); addFileViewerPane(workspaceId, {...});To something like:
const { tabId } = addTab(workspaceId); // Then either modify addFileViewerPane to accept an optional tabId, or explicitly use the tabIdapps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListGrouped.tsx (1)
1-166: Props threading looks correct.The
contextMenuPropsis properly threaded fromFileListGrouped→FolderGroupItem→FileItem, and the per-file transformation correctly binds the file to each callback.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/components/FileList/FileListTree.tsx (1)
125-139: Recursive context menu prop threading is correct.The
contextMenuPropsis properly passed to childTreeNodeComponentinstances, maintaining the context menu functionality throughout the tree hierarchy.apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx (3)
108-126: Error handling follows established patterns.The new mutations for
discardChangesanddeleteUntrackedcorrectly implementonSuccesswith refetch andonErrorwith console logging and toast notifications, consistent with the existing mutations in this file.
229-238: Clean helper for conditional discard handler injection.The
getContextMenuPropshelper elegantly handles the conditional inclusion ofonDiscardChangesbased on the category (only for unstaged files at line 449).
449-449: Correct: Only unstaged files get the discard option.Passing
truetogetContextMenuProps(true)only for the unstaged section ensures the "Discard Changes" context menu option is appropriately scoped.
| // Handle discard changes for unstaged files | ||
| const handleDiscardChanges = (file: ChangedFile) => { | ||
| if (!worktreePath) return; | ||
| // Untracked files need to be deleted, modified files use git checkout | ||
| if (file.status === "untracked") { | ||
| deleteUntrackedMutation.mutate({ | ||
| worktreePath, | ||
| filePath: file.path, | ||
| }); | ||
| } else { | ||
| discardChangesMutation.mutate({ | ||
| worktreePath, | ||
| filePath: file.path, | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Consider adding a confirmation dialog for destructive discard operations.
handleDiscardChanges can permanently delete untracked files or discard uncommitted changes. These are irreversible operations that users might trigger accidentally via the context menu. Consider adding a confirmation dialog, especially for the deleteUntrackedMutation path.
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/ChangesView/ChangesView.tsx
around lines 212 - 227, handleDiscardChanges currently performs irreversible
actions without confirmation; before invoking deleteUntrackedMutation or
discardChangesMutation, show a confirmation dialog/modal (use the app's existing
confirmation UI or a simple window.confirm) that clearly states the file path
and whether it will be deleted (for untracked) or have changes discarded (for
modified), and only proceed to call deleteUntrackedMutation.mutate or
discardChangesMutation.mutate if the user confirms; for clarity, use distinct
messages for the untracked (delete) and tracked (discard) branches and abort on
cancel.
| [ | ||
| workspaceId, | ||
| worktreePath, | ||
| currentTabId, | ||
| workspaceTabs, | ||
| addFileViewerPane, | ||
| addTab, | ||
| splitPaneHorizontal, | ||
| splitPaneVertical, | ||
| openInApp, | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx | head -150Repository: superset-sh/superset
Length of output: 5071
🌐 Web query:
@tanstack/react-query useMutation object reference stability dependency array
💡 Result:
Short answer: The object returned by useMutation is not referentially stable — don't put that whole object into a hook dependency array. Destructure the pieces you need (e.g. data, error, mutate, mutateAsync, status) and use those in deps instead. See TanStack's "no-unstable-deps" guidance. [1][2]
Practical notes
- The top-level object from useMutation (and useQuery/useQueries/etc.) will be a new reference each render — avoid using it directly in useEffect/useCallback/useMemo deps. [1][2]
- Destructure the values you need and put those in the dependency array. For example:
- const { mutate } = useMutation(...);
- const onClick = useCallback(() => mutate(x), [mutate]);
- The docs/ESLint plugin explicitly warn against putting the hook return object into deps; the community/maintainers added an ESLint rule to catch this. [1][3]
- Returned data properties are made as stable as possible; still prefer destructuring to be explicit. [2][4]
Sources
- TanStack Query — "No unstable deps" (eslint guidance). [1]
- TanStack Query — useMutation reference / returned properties. [2]
- Discussion / ESLint plugin / issue about preventing unstable return values in deps. [3]
- StackOverflow answer summarizing stability behavior of query hooks. [4]
Avoid including unstable mutation object in dependency array.
The openInApp mutation object is a new reference on each render. Including it in the dependency array will cause contextMenuProps to be recreated unnecessarily on every render.
Since only openInApp.mutate is used (line 86), destructure it directly:
const { mutate: openInAppMutate } = openInApp;
Then add openInAppMutate to the dependency array instead, or use openInApp.mutate directly if the mutate function is stable per your version of @tanstack/react-query.
🤖 Prompt for AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/Sidebar/index.tsx
around lines 109 - 119, The dependency array for the contextMenuProps recreates
on each render because it includes the unstable mutation object openInApp;
destructure the mutate function from openInApp (e.g., const { mutate:
openInAppMutate } = openInApp) and replace openInApp in the dependency list with
openInAppMutate (or, if your react-query version guarantees a stable function,
use openInApp.mutate directly) so that contextMenuProps only updates when the
actual mutate function reference changes; update any uses inside
contextMenuProps to call the destructured mutate symbol instead of openInApp.
Summary
FileContextMenuPropstype throughout the component hierarchy to reduce code duplication and ensure consistency across all file categories (against-base, commits, staged, unstaged)Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.