mirror of
https://github.com/stablyai/orca.git
synced 2026-10-09 00:02:39 +00:00
fix: address review findings (#422)
This commit is contained in:
@@ -34,6 +34,10 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() {
|
||||
const deleteError = deleteState?.error ?? null
|
||||
const canForceDelete = deleteState?.canForceDelete ?? false
|
||||
const worktreeName = worktree?.displayName ?? 'unknown'
|
||||
// Why: the main worktree is the repo's original clone directory — `git worktree remove`
|
||||
// always rejects it. We block the delete button upfront so the user doesn't have to
|
||||
// discover this limitation via a confusing force-delete dead-end.
|
||||
const isMainWorktree = worktree?.isMainWorktree ?? false
|
||||
|
||||
useEffect(() => {
|
||||
if (isOpen && worktreeId && !worktree && !isDeleting) {
|
||||
@@ -127,7 +131,19 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{deleteError && (
|
||||
{isMainWorktree && (
|
||||
<div className="rounded-md border border-blue-500/40 bg-blue-500/8 px-3 py-2 text-xs text-blue-700 dark:text-blue-300">
|
||||
<div className="flex items-start gap-2">
|
||||
<AlertTriangle className="mt-0.5 size-3.5 shrink-0" />
|
||||
<div className="min-w-0 flex-1">
|
||||
This is the <span className="font-semibold">main worktree</span> (the original clone
|
||||
directory). Git does not allow removing the main worktree.
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{deleteError && !isMainWorktree && (
|
||||
<div className="rounded-md border border-destructive/40 bg-destructive/8 px-3 py-2 text-xs text-destructive">
|
||||
<div className="flex items-start gap-2">
|
||||
<AlertTriangle className="mt-0.5 size-3.5 shrink-0" />
|
||||
@@ -138,19 +154,28 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() {
|
||||
|
||||
<DialogFooter>
|
||||
<Button variant="outline" onClick={() => handleOpenChange(false)} disabled={isDeleting}>
|
||||
Cancel
|
||||
{isMainWorktree ? 'Close' : 'Cancel'}
|
||||
</Button>
|
||||
{canForceDelete ? (
|
||||
<Button variant="destructive" onClick={() => handleDelete(true)} disabled={isDeleting}>
|
||||
{isDeleting ? <LoaderCircle className="size-4 animate-spin" /> : <Trash2 />}
|
||||
{isDeleting ? 'Force Deleting…' : 'Force Delete'}
|
||||
</Button>
|
||||
) : (
|
||||
<Button variant="destructive" onClick={() => handleDelete(false)} disabled={isDeleting}>
|
||||
{isDeleting ? <LoaderCircle className="size-4 animate-spin" /> : <Trash2 />}
|
||||
{isDeleting ? 'Deleting…' : 'Delete'}
|
||||
</Button>
|
||||
)}
|
||||
{!isMainWorktree &&
|
||||
(canForceDelete ? (
|
||||
<Button
|
||||
variant="destructive"
|
||||
onClick={() => handleDelete(true)}
|
||||
disabled={isDeleting}
|
||||
>
|
||||
{isDeleting ? <LoaderCircle className="size-4 animate-spin" /> : <Trash2 />}
|
||||
{isDeleting ? 'Force Deleting…' : 'Force Delete'}
|
||||
</Button>
|
||||
) : (
|
||||
<Button
|
||||
variant="destructive"
|
||||
onClick={() => handleDelete(false)}
|
||||
disabled={isDeleting}
|
||||
>
|
||||
{isDeleting ? <LoaderCircle className="size-4 animate-spin" /> : <Trash2 />}
|
||||
{isDeleting ? 'Deleting…' : 'Delete'}
|
||||
</Button>
|
||||
))}
|
||||
</DialogFooter>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
|
||||
@@ -351,6 +351,9 @@ const WorktreeCard = React.memo(function WorktreeCard({
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Branch / folder badge — unchanged from the original logic so we
|
||||
never lose the branch name, even when the main worktree is checked
|
||||
out on a non-primary branch like "feature-x". */}
|
||||
{isFolder ? (
|
||||
<Badge
|
||||
variant="secondary"
|
||||
@@ -359,18 +362,55 @@ const WorktreeCard = React.memo(function WorktreeCard({
|
||||
{repo ? getRepoKindLabel(repo) : 'Folder'}
|
||||
</Badge>
|
||||
) : isPrimaryBranch(worktree.branch) ? (
|
||||
<Badge
|
||||
variant="secondary"
|
||||
className="h-[16px] px-1.5 text-[10px] font-medium rounded shrink-0 text-muted-foreground bg-accent border border-border dark:bg-accent/80 dark:border-border/50 leading-none"
|
||||
>
|
||||
main
|
||||
</Badge>
|
||||
worktree.isMainWorktree ? (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Badge
|
||||
variant="outline"
|
||||
className="h-[16px] px-1.5 text-[10px] font-medium rounded shrink-0 leading-none text-blue-600 border-blue-500/30 bg-blue-500/5 dark:text-blue-400 dark:border-blue-400/30 dark:bg-blue-400/5"
|
||||
>
|
||||
main
|
||||
</Badge>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="right" sideOffset={8}>
|
||||
Main worktree
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
) : (
|
||||
<Badge
|
||||
variant="secondary"
|
||||
className="h-[16px] px-1.5 text-[10px] font-medium rounded shrink-0 leading-none text-muted-foreground bg-accent border border-border dark:bg-accent/80 dark:border-border/50"
|
||||
>
|
||||
main
|
||||
</Badge>
|
||||
)
|
||||
) : (
|
||||
<span className="text-[11px] text-muted-foreground truncate leading-none">
|
||||
{branch}
|
||||
</span>
|
||||
)}
|
||||
|
||||
{/* Why: the main worktree (the original clone directory) cannot be
|
||||
deleted via `git worktree remove`. Surfacing this in the card lets
|
||||
users identify it at a glance. When the branch is already primary,
|
||||
the blue "main" badge above does double duty; otherwise we add a
|
||||
separate blue badge so both the branch and worktree type are visible. */}
|
||||
{worktree.isMainWorktree && !isFolder && !isPrimaryBranch(worktree.branch) && (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Badge
|
||||
variant="outline"
|
||||
className="h-[16px] px-1.5 text-[10px] font-medium rounded shrink-0 text-blue-600 border-blue-500/30 bg-blue-500/5 dark:text-blue-400 dark:border-blue-400/30 dark:bg-blue-400/5 leading-none"
|
||||
>
|
||||
main
|
||||
</Badge>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="right" sideOffset={8}>
|
||||
Main worktree
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
)}
|
||||
|
||||
{/* Why: the conflict operation (merge/rebase/cherry-pick) is the
|
||||
only signal that the worktree is in an incomplete operation state.
|
||||
Showing it on the card lets the user spot worktrees that need
|
||||
|
||||
@@ -176,7 +176,20 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({ worktree,
|
||||
<XCircle className="size-3.5" />
|
||||
Shutdown
|
||||
</DropdownMenuItem>
|
||||
<DropdownMenuItem variant="destructive" onSelect={handleDelete} disabled={isDeleting}>
|
||||
{/* Why: `git worktree remove` always rejects the main worktree, so we
|
||||
disable the item upfront. Radix forwards unknown props to the DOM
|
||||
element, so `title` works directly without a wrapper span — this
|
||||
preserves Radix's flat roving-tabindex keyboard navigation. */}
|
||||
<DropdownMenuItem
|
||||
variant="destructive"
|
||||
onSelect={handleDelete}
|
||||
disabled={isDeleting || (!isFolder && worktree.isMainWorktree)}
|
||||
title={
|
||||
!isFolder && worktree.isMainWorktree
|
||||
? 'The main worktree cannot be deleted'
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
<Trash2 className="size-3.5" />
|
||||
{isDeleting ? 'Deleting…' : isFolder ? 'Remove Folder from Orca' : 'Delete'}
|
||||
</DropdownMenuItem>
|
||||
|
||||
Reference in New Issue
Block a user