feat(desktop): add discard button to unstaged file rows#1698
Open
supungbab wants to merge 1 commit intosuperset-sh:mainfrom
Open
feat(desktop): add discard button to unstaged file rows#1698supungbab wants to merge 1 commit intosuperset-sh:mainfrom
supungbab wants to merge 1 commit intosuperset-sh:mainfrom
Conversation
Add a hover discard button next to the stage button for unstaged files, improving accessibility over the previous context-menu-only approach.
Contributor
📝 WalkthroughWalkthroughThe FileItem component now supports discarding file changes through a new optional callback prop. The component renders a discard action button and context menu option that trigger a confirmation dialog, with visual indicators (trash or undo icon) varying by file status. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/FileItem/FileItem.tsx (1)
285-288:⚠️ Potential issue | 🟠 MajorDiscard confirmation dialog unreachable when
worktreePathis absent.
fileContent— which contains the discard button — is returned by both code paths, but theAlertDialogis only rendered in the second return (lines 345–374), gated behind theworktreePathcheck. IfonDiscardis provided withoutworktreePath, clicking the discard button callssetShowDiscardDialog(true), but theAlertDialogis never mounted, so the dialog never appears andonDiscardis never invoked.The fix is to extract the dialog so it is always rendered alongside
fileContent:🐛 Proposed fix — decouple the AlertDialog from the `worktreePath` branch
+ const discardDialog = onDiscard ? ( + <AlertDialog open={showDiscardDialog} onOpenChange={setShowDiscardDialog}> + <AlertDialogContent className="max-w-[340px] gap-0 p-0"> + <AlertDialogHeader className="px-4 pt-4 pb-2"> + <AlertDialogTitle className="font-medium"> + {discardDialogTitle} + </AlertDialogTitle> + <AlertDialogDescription> + {discardDialogDescription} + </AlertDialogDescription> + </AlertDialogHeader> + <AlertDialogFooter className="px-4 pb-4 pt-2 flex-row justify-end gap-2"> + <Button + variant="ghost" + size="sm" + className="h-7 px-3 text-xs" + onClick={() => setShowDiscardDialog(false)} + > + Cancel + </Button> + <Button + variant="destructive" + size="sm" + className="h-7 px-3 text-xs" + onClick={handleConfirmDiscard} + > + {isDeleteAction ? "Delete" : "Discard"} + </Button> + </AlertDialogFooter> + </AlertDialogContent> + </AlertDialog> + ) : null; if (!worktreePath) { - return fileContent; + return ( + <> + {fileContent} + {discardDialog} + </> + ); } return ( <> <ContextMenu> <ContextMenuTrigger asChild>{fileContent}</ContextMenuTrigger> <ContextMenuContent className="w-48"> {/* … existing menu items … */} </ContextMenuContent> </ContextMenu> - <AlertDialog open={showDiscardDialog} onOpenChange={setShowDiscardDialog}> - {/* … dialog content … */} - </AlertDialog> + {discardDialog} </> );Also applies to: 345-374
🤖 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/RightSidebar/ChangesView/components/FileItem/FileItem.tsx` around lines 285 - 288, The discard confirmation AlertDialog is only rendered inside the worktreePath branch so when worktreePath is falsy the discard button (part of fileContent) can toggle setShowDiscardDialog(true) but the dialog never mounts; move/extract the AlertDialog out of the worktreePath conditional so it is always rendered alongside fileContent, keep the existing state (setShowDiscardDialog, showDiscardDialog) and confirmation handler (calling onDiscard) intact, and if the dialog logic needs worktreePath use a guarded check inside the dialog confirm handler to call onDiscard with worktreePath (or call onDiscard without it when absent) so confirming always invokes onDiscard regardless of the outer branch.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/FileItem/FileItem.tsx (1)
149-156:handleConfirmDiscard/handleDiscardClickmissinguseCallback.
handleClickandhandleDoubleClick(lines 114–139) are memoised withuseCallback, but these two new handlers are plain closures recreated on every render.handleConfirmDiscardcloses over theonDiscardprop, so it's worth stabilising for consistency and to avoid stale-closure edge cases.♻️ Memoize both handlers
- const handleDiscardClick = () => { + const handleDiscardClick = useCallback(() => { setShowDiscardDialog(true); - }; + }, []); - const handleConfirmDiscard = () => { + const handleConfirmDiscard = useCallback(() => { setShowDiscardDialog(false); onDiscard?.(); - }; + }, [onDiscard]);🤖 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/RightSidebar/ChangesView/components/FileItem/FileItem.tsx` around lines 149 - 156, The two handlers handleDiscardClick and handleConfirmDiscard should be wrapped with React.useCallback to stabilize their identities and avoid stale closures: memoize handleDiscardClick (which calls setShowDiscardDialog(true)) and memoize handleConfirmDiscard (which calls setShowDiscardDialog(false) and invokes onDiscard?.()) using useCallback with appropriate dependency arrays (e.g., handleDiscardClick depends on setShowDiscardDialog, handleConfirmDiscard depends on setShowDiscardDialog and onDiscard) so they match the memoization style used for handleClick/handleDoubleClick.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/FileItem/FileItem.tsx
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/FileItem/FileItem.tsx`:
- Around line 285-288: The discard confirmation AlertDialog is only rendered
inside the worktreePath branch so when worktreePath is falsy the discard button
(part of fileContent) can toggle setShowDiscardDialog(true) but the dialog never
mounts; move/extract the AlertDialog out of the worktreePath conditional so it
is always rendered alongside fileContent, keep the existing state
(setShowDiscardDialog, showDiscardDialog) and confirmation handler (calling
onDiscard) intact, and if the dialog logic needs worktreePath use a guarded
check inside the dialog confirm handler to call onDiscard with worktreePath (or
call onDiscard without it when absent) so confirming always invokes onDiscard
regardless of the outer branch.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/FileItem/FileItem.tsx`:
- Around line 149-156: The two handlers handleDiscardClick and
handleConfirmDiscard should be wrapped with React.useCallback to stabilize their
identities and avoid stale closures: memoize handleDiscardClick (which calls
setShowDiscardDialog(true)) and memoize handleConfirmDiscard (which calls
setShowDiscardDialog(false) and invokes onDiscard?.()) using useCallback with
appropriate dependency arrays (e.g., handleDiscardClick depends on
setShowDiscardDialog, handleConfirmDiscard depends on setShowDiscardDialog and
onDiscard) so they match the memoization style used for
handleClick/handleDoubleClick.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
feat(desktop): add discard button to unstaged file rows
Summary
View
Test plan
Summary by CodeRabbit