feat(source-control): full-row hover, tooltips, and staged discard-all (#1407)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson
GitHub
parent
a595c1f84b
commit
9b3b2a5486
|
|
@ -0,0 +1,151 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { Plus } from 'lucide-react'
|
||||
import { ActionButton } from './SourceControl'
|
||||
import { Button } from '@/components/ui/button'
|
||||
|
||||
type ReactElementLike = {
|
||||
type: unknown
|
||||
props: Record<string, unknown>
|
||||
}
|
||||
|
||||
function visit(node: unknown, cb: (node: ReactElementLike) => void): void {
|
||||
if (node == null || typeof node === 'string' || typeof node === 'number') {
|
||||
return
|
||||
}
|
||||
if (Array.isArray(node)) {
|
||||
node.forEach((entry) => visit(entry, cb))
|
||||
return
|
||||
}
|
||||
const element = node as ReactElementLike
|
||||
cb(element)
|
||||
if (element.props?.children) {
|
||||
visit(element.props.children, cb)
|
||||
}
|
||||
}
|
||||
|
||||
function findInnerButton(node: unknown): ReactElementLike {
|
||||
let found: ReactElementLike | null = null
|
||||
visit(node, (entry) => {
|
||||
if (entry.type === Button) {
|
||||
found = entry
|
||||
}
|
||||
})
|
||||
if (!found) {
|
||||
throw new Error('inner Button not found')
|
||||
}
|
||||
return found
|
||||
}
|
||||
|
||||
function findTooltipContentText(node: unknown): string {
|
||||
const texts: string[] = []
|
||||
visit(node, (entry) => {
|
||||
const typeName =
|
||||
typeof entry.type === 'function' || typeof entry.type === 'object'
|
||||
? ((entry.type as { displayName?: string; name?: string }).displayName ??
|
||||
(entry.type as { displayName?: string; name?: string }).name ??
|
||||
'')
|
||||
: ''
|
||||
if (typeName === 'TooltipContent') {
|
||||
const children = entry.props?.children
|
||||
if (typeof children === 'string') {
|
||||
texts.push(children)
|
||||
}
|
||||
}
|
||||
})
|
||||
return texts.join(' ')
|
||||
}
|
||||
|
||||
// Why: ActionButton's onClick handler only touches these three fields, so a
|
||||
// narrow event type is both correct and avoids a forbidden `as unknown as X`
|
||||
// double-cast on the event value.
|
||||
type MinimalMouseEvent = Pick<
|
||||
React.MouseEvent,
|
||||
'preventDefault' | 'stopPropagation' | 'defaultPrevented'
|
||||
>
|
||||
|
||||
function makeClickEvent(): {
|
||||
event: MinimalMouseEvent
|
||||
preventDefault: ReturnType<typeof vi.fn>
|
||||
} {
|
||||
const preventDefault = vi.fn()
|
||||
const event: MinimalMouseEvent = {
|
||||
preventDefault,
|
||||
stopPropagation: vi.fn(),
|
||||
defaultPrevented: false
|
||||
}
|
||||
return { event, preventDefault }
|
||||
}
|
||||
|
||||
const baseProps = {
|
||||
icon: Plus,
|
||||
title: 'Stage all',
|
||||
onClick: vi.fn()
|
||||
}
|
||||
|
||||
describe('ActionButton', () => {
|
||||
it('forwards the title as aria-label on the inner button', () => {
|
||||
const element = ActionButton({ ...baseProps, onClick: vi.fn() })
|
||||
const button = findInnerButton(element)
|
||||
expect(button.props['aria-label']).toBe('Stage all')
|
||||
})
|
||||
|
||||
it('renders the title as tooltip content (non-native, Radix)', () => {
|
||||
const element = ActionButton({ ...baseProps, onClick: vi.fn() })
|
||||
// Why: the Radix TooltipContent is what renders the visual tooltip.
|
||||
// Native `title` was removed because its styling diverges from the
|
||||
// rest of the sidebar chrome.
|
||||
expect(findTooltipContentText(element)).toContain('Stage all')
|
||||
})
|
||||
|
||||
it('calls the onClick handler when enabled', () => {
|
||||
const onClick = vi.fn()
|
||||
const element = ActionButton({ ...baseProps, onClick })
|
||||
const button = findInnerButton(element)
|
||||
const { event } = makeClickEvent()
|
||||
;(button.props.onClick as (e: MinimalMouseEvent) => void)(event)
|
||||
expect(onClick).toHaveBeenCalledWith(event)
|
||||
})
|
||||
|
||||
it('does NOT render the native disabled prop on the inner button', () => {
|
||||
// Why: Radix TooltipTrigger on a DOM-disabled <button> gets its pointer
|
||||
// events blocked in Chromium, suppressing the tooltip entirely — a
|
||||
// regression vs. the native `title` attribute it replaced. ActionButton
|
||||
// uses aria-disabled + a click guard instead.
|
||||
const element = ActionButton({ ...baseProps, onClick: vi.fn(), disabled: true })
|
||||
const button = findInnerButton(element)
|
||||
expect(button.props.disabled).toBeUndefined()
|
||||
})
|
||||
|
||||
it('marks the inner button aria-disabled when the disabled prop is true', () => {
|
||||
const element = ActionButton({ ...baseProps, onClick: vi.fn(), disabled: true })
|
||||
const button = findInnerButton(element)
|
||||
expect(button.props['aria-disabled']).toBe(true)
|
||||
})
|
||||
|
||||
it('applies the visual-disabled class when disabled', () => {
|
||||
const element = ActionButton({ ...baseProps, onClick: vi.fn(), disabled: true })
|
||||
const button = findInnerButton(element)
|
||||
expect(button.props.className).toContain('opacity-50')
|
||||
expect(button.props.className).toContain('cursor-not-allowed')
|
||||
})
|
||||
|
||||
it('omits the visual-disabled class when enabled', () => {
|
||||
const element = ActionButton({ ...baseProps, onClick: vi.fn(), disabled: false })
|
||||
const button = findInnerButton(element)
|
||||
expect(button.props.className).not.toContain('opacity-50')
|
||||
})
|
||||
|
||||
it('swallows clicks and calls preventDefault when disabled', () => {
|
||||
const onClick = vi.fn()
|
||||
const element = ActionButton({ ...baseProps, onClick, disabled: true })
|
||||
const button = findInnerButton(element)
|
||||
const { event, preventDefault } = makeClickEvent()
|
||||
;(button.props.onClick as (e: MinimalMouseEvent) => void)(event)
|
||||
// Why: keyboard Enter/Space also fires onClick on a non-DOM-disabled
|
||||
// button. The guard must block both pointer and keyboard activation
|
||||
// while `disabled` is true, even though the inner handler is trusted
|
||||
// to early-return on `isExecutingBulk`.
|
||||
expect(onClick).not.toHaveBeenCalled()
|
||||
expect(preventDefault).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
|
@ -34,6 +34,14 @@ import { Tooltip, TooltipTrigger, TooltipContent, TooltipProvider } from '@/comp
|
|||
import { Button } from '@/components/ui/button'
|
||||
import { BulkActionBar } from './BulkActionBar'
|
||||
import { useSourceControlSelection, type FlatEntry } from './useSourceControlSelection'
|
||||
import {
|
||||
getDiscardAllPaths,
|
||||
getStageAllPaths,
|
||||
getUnstageAllPaths,
|
||||
runDiscardAllForArea,
|
||||
type DiscardAllArea
|
||||
} from './discard-all-sequence'
|
||||
import { toast } from 'sonner'
|
||||
import {
|
||||
ContextMenu,
|
||||
ContextMenuContent,
|
||||
|
|
@ -617,6 +625,49 @@ function SourceControlInner(): React.JSX.Element {
|
|||
}
|
||||
}, [worktreePath, bulkUnstagePaths, clearSelection, activeWorktreeId])
|
||||
|
||||
// Why: "Stage all" on the Changes section intentionally skips unresolved
|
||||
// conflict rows. `git add` on a conflicted file silently clears the `u`
|
||||
// record — the only live signal we have — before the user has reviewed it,
|
||||
// which mirrors the per-row Stage suppression above.
|
||||
const handleStageAllInArea = useCallback(
|
||||
async (area: 'unstaged' | 'untracked') => {
|
||||
if (!worktreePath || isExecutingBulk) {
|
||||
return
|
||||
}
|
||||
const paths = getStageAllPaths(grouped[area], area)
|
||||
if (paths.length === 0) {
|
||||
return
|
||||
}
|
||||
setIsExecutingBulk(true)
|
||||
try {
|
||||
const connectionId = getConnectionId(activeWorktreeId ?? null) ?? undefined
|
||||
await window.api.git.bulkStage({ worktreePath, filePaths: paths, connectionId })
|
||||
clearSelection()
|
||||
} finally {
|
||||
setIsExecutingBulk(false)
|
||||
}
|
||||
},
|
||||
[worktreePath, grouped, activeWorktreeId, isExecutingBulk, clearSelection]
|
||||
)
|
||||
|
||||
const handleUnstageAll = useCallback(async () => {
|
||||
if (!worktreePath || isExecutingBulk) {
|
||||
return
|
||||
}
|
||||
const paths = getUnstageAllPaths(grouped.staged)
|
||||
if (paths.length === 0) {
|
||||
return
|
||||
}
|
||||
setIsExecutingBulk(true)
|
||||
try {
|
||||
const connectionId = getConnectionId(activeWorktreeId ?? null) ?? undefined
|
||||
await window.api.git.bulkUnstage({ worktreePath, filePaths: paths, connectionId })
|
||||
clearSelection()
|
||||
} finally {
|
||||
setIsExecutingBulk(false)
|
||||
}
|
||||
}, [worktreePath, grouped.staged, activeWorktreeId, isExecutingBulk, clearSelection])
|
||||
|
||||
const refreshBranchCompare = useCallback(async () => {
|
||||
if (!activeWorktreeId || !worktreePath || !effectiveBaseRef || isFolder) {
|
||||
return
|
||||
|
|
@ -758,34 +809,109 @@ function SourceControlInner(): React.JSX.Element {
|
|||
[worktreePath, activeWorktreeId]
|
||||
)
|
||||
|
||||
const handleDiscard = useCallback(
|
||||
// Why: split into two variants — `discardSingle` throws so bulk callers can
|
||||
// aggregate failures into a single toast via `runDiscardAllForArea`'s
|
||||
// onError, while `handleDiscard` swallows for the per-row fire-and-forget UI
|
||||
// contract (no individual failure toast).
|
||||
const discardSingle = useCallback(
|
||||
async (filePath: string) => {
|
||||
if (!worktreePath || !activeWorktreeId) {
|
||||
return
|
||||
}
|
||||
try {
|
||||
// Why: git discard replaces the working tree version of this file. Any
|
||||
// pending editor autosave must be quiesced first so it cannot recreate
|
||||
// the discarded edits after git restores the file.
|
||||
await requestEditorSaveQuiesce({
|
||||
worktreeId: activeWorktreeId,
|
||||
worktreePath,
|
||||
relativePath: filePath
|
||||
})
|
||||
const connectionId = getConnectionId(activeWorktreeId ?? null) ?? undefined
|
||||
await window.api.git.discard({ worktreePath, filePath, connectionId })
|
||||
notifyEditorExternalFileChange({
|
||||
worktreeId: activeWorktreeId,
|
||||
worktreePath,
|
||||
relativePath: filePath
|
||||
})
|
||||
} catch {
|
||||
// git operation failed silently
|
||||
}
|
||||
// Why: git discard replaces the working tree version of this file. Any
|
||||
// pending editor autosave must be quiesced first so it cannot recreate
|
||||
// the discarded edits after git restores the file.
|
||||
await requestEditorSaveQuiesce({
|
||||
worktreeId: activeWorktreeId,
|
||||
worktreePath,
|
||||
relativePath: filePath
|
||||
})
|
||||
const connectionId = getConnectionId(activeWorktreeId ?? null) ?? undefined
|
||||
await window.api.git.discard({ worktreePath, filePath, connectionId })
|
||||
notifyEditorExternalFileChange({
|
||||
worktreeId: activeWorktreeId,
|
||||
worktreePath,
|
||||
relativePath: filePath
|
||||
})
|
||||
},
|
||||
[activeWorktreeId, worktreePath]
|
||||
)
|
||||
|
||||
const handleDiscard = useCallback(
|
||||
async (filePath: string) => {
|
||||
try {
|
||||
await discardSingle(filePath)
|
||||
} catch {
|
||||
// Why: per-row discard is fire-and-forget for the UI; failures are not
|
||||
// surfaced individually. Bulk callers use `discardSingle` directly so
|
||||
// they can aggregate failures into a single toast.
|
||||
}
|
||||
},
|
||||
[discardSingle]
|
||||
)
|
||||
|
||||
// Why: "Discard all" mirrors the per-row discard rules — it skips unresolved
|
||||
// and resolved_locally rows because discarding those can silently re-create
|
||||
// the conflict or lose the resolution (no v1 UX to explain this clearly).
|
||||
// There is no bulk discard IPC, so we serialize per-file discard calls that
|
||||
// run the same editor-quiesce + external-change notification as the row action.
|
||||
// The sequencing + filter rules live in discard-all-sequence.ts so they can
|
||||
// be unit-tested independently of the full component (staged area needs a
|
||||
// bulk-unstage first, and a failed unstage must skip the discard loop).
|
||||
const handleRevertAllInArea = useCallback(
|
||||
async (area: DiscardAllArea) => {
|
||||
if (!worktreePath || !activeWorktreeId || isExecutingBulk) {
|
||||
return
|
||||
}
|
||||
const paths = getDiscardAllPaths(grouped[area], area)
|
||||
if (paths.length === 0) {
|
||||
return
|
||||
}
|
||||
setIsExecutingBulk(true)
|
||||
try {
|
||||
const connectionId = getConnectionId(activeWorktreeId) ?? undefined
|
||||
// Why: `onError` fires once per failure — both for the bulk-unstage
|
||||
// pre-step and for each per-file discard failure. Aggregate into one
|
||||
// toast after the sequence completes so a partial failure across N
|
||||
// files doesn't spam N error toasts.
|
||||
const errors: unknown[] = []
|
||||
const result = await runDiscardAllForArea(area, paths, {
|
||||
bulkUnstage: (filePaths) =>
|
||||
window.api.git.bulkUnstage({ worktreePath, filePaths, connectionId }),
|
||||
discardOne: discardSingle,
|
||||
onError: (error) => {
|
||||
errors.push(error)
|
||||
console.error('[SourceControl] discard-all failure', error)
|
||||
}
|
||||
})
|
||||
if (result.aborted) {
|
||||
toast.error('Discard all failed — unable to unstage files before discard', {
|
||||
description: errors[0] instanceof Error ? errors[0].message : undefined
|
||||
})
|
||||
} else if (result.failed.length > 0) {
|
||||
// Why: only include the first error message to avoid a huge toast
|
||||
// body on bulk failures; a short sample of failed paths gives users
|
||||
// enough context to retry or investigate.
|
||||
const firstMsg = errors[0] instanceof Error ? errors[0].message : undefined
|
||||
const sample = result.failed.slice(0, 3).join(', ')
|
||||
const more = result.failed.length > 3 ? `, +${result.failed.length - 3} more` : ''
|
||||
toast.error(
|
||||
`Failed to discard ${result.failed.length} file${result.failed.length === 1 ? '' : 's'}`,
|
||||
{
|
||||
description: firstMsg ? `${firstMsg} (e.g. ${sample}${more})` : `${sample}${more}`
|
||||
}
|
||||
)
|
||||
}
|
||||
if (!result.aborted) {
|
||||
clearSelection()
|
||||
}
|
||||
} finally {
|
||||
setIsExecutingBulk(false)
|
||||
}
|
||||
},
|
||||
[worktreePath, activeWorktreeId, grouped, isExecutingBulk, clearSelection, discardSingle]
|
||||
)
|
||||
|
||||
if (!activeWorktree || !activeRepo || !worktreePath) {
|
||||
return (
|
||||
<div className="flex items-center justify-center h-full text-xs text-muted-foreground px-4 text-center">
|
||||
|
|
@ -1038,6 +1164,26 @@ function SourceControlInner(): React.JSX.Element {
|
|||
return null
|
||||
}
|
||||
const isCollapsed = collapsedSections.has(area)
|
||||
// Why: "Stage all"/"Unstage all" operate on the *unfiltered*
|
||||
// group for the area — acting on just the filter-visible subset
|
||||
// would surprise users who don't realize a filter is active.
|
||||
// The +/- is hidden when the filter is active to avoid that
|
||||
// mismatch between what's shown and what would be staged.
|
||||
// Why: visibility and execution both resolve paths through the
|
||||
// same helpers (`getStageAllPaths`/`getUnstageAllPaths`/
|
||||
// `getDiscardAllPaths`) so the button can never show for a set
|
||||
// the handler would then filter to empty.
|
||||
const stageAllPaths =
|
||||
area === 'unstaged' || area === 'untracked'
|
||||
? getStageAllPaths(grouped[area], area)
|
||||
: []
|
||||
const canStageAll = !normalizedFilter && stageAllPaths.length > 0
|
||||
const canUnstageAll =
|
||||
!normalizedFilter &&
|
||||
area === 'staged' &&
|
||||
getUnstageAllPaths(grouped.staged).length > 0
|
||||
const canRevertAll =
|
||||
!normalizedFilter && getDiscardAllPaths(grouped[area], area).length > 0
|
||||
return (
|
||||
<div key={area}>
|
||||
<SectionHeader
|
||||
|
|
@ -1049,37 +1195,92 @@ function SourceControlInner(): React.JSX.Element {
|
|||
isCollapsed={isCollapsed}
|
||||
onToggle={() => toggleSection(area)}
|
||||
actions={
|
||||
items.some((entry) => entry.conflictStatus === 'unresolved') ? (
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="h-6 px-1.5 text-[10px] text-muted-foreground hover:text-foreground"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation()
|
||||
if (activeWorktreeId && worktreePath) {
|
||||
openAllDiffs(activeWorktreeId, worktreePath, undefined, area)
|
||||
}
|
||||
}}
|
||||
>
|
||||
View all
|
||||
</Button>
|
||||
) : (
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="h-auto px-1.5 py-0.5 text-xs text-muted-foreground hover:text-foreground"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation()
|
||||
if (activeWorktreeId && worktreePath) {
|
||||
openAllDiffs(activeWorktreeId, worktreePath, undefined, area)
|
||||
}
|
||||
}}
|
||||
>
|
||||
View all
|
||||
</Button>
|
||||
)
|
||||
<>
|
||||
{/* Why: bulk action buttons are hover-only on
|
||||
pointer devices to avoid cluttering the section
|
||||
header with persistent icons. On no-hover
|
||||
pointers (touch, and SSH sessions where hover
|
||||
state is unreliable — see AGENTS.md "SSH Use
|
||||
Case"), force them visible so they're reachable
|
||||
without tabbing. One outer wrapper so that
|
||||
focusing any action reveals all three siblings —
|
||||
otherwise keyboard users tab into an invisible
|
||||
next stop. */}
|
||||
<div className="flex items-center opacity-0 transition-opacity group-hover/section:opacity-100 focus-within:opacity-100 [@media(hover:none)]:opacity-100">
|
||||
{canRevertAll && (
|
||||
<ActionButton
|
||||
icon={Undo2}
|
||||
// Why: for untracked files, discard deletes the file
|
||||
// outright (rm -rf via git.discard's untracked branch).
|
||||
// A generic "Discard all" label hides that severity —
|
||||
// label explicitly for the destructive variant.
|
||||
title={
|
||||
area === 'untracked' ? 'Delete all untracked' : 'Discard all'
|
||||
}
|
||||
onClick={(event) => {
|
||||
event.stopPropagation()
|
||||
void handleRevertAllInArea(area)
|
||||
}}
|
||||
disabled={isExecutingBulk}
|
||||
/>
|
||||
)}
|
||||
{canStageAll && (
|
||||
<ActionButton
|
||||
icon={Plus}
|
||||
title="Stage all"
|
||||
onClick={(event) => {
|
||||
event.stopPropagation()
|
||||
if (area === 'unstaged' || area === 'untracked') {
|
||||
void handleStageAllInArea(area)
|
||||
}
|
||||
}}
|
||||
disabled={isExecutingBulk}
|
||||
/>
|
||||
)}
|
||||
{canUnstageAll && (
|
||||
<ActionButton
|
||||
icon={Minus}
|
||||
title="Unstage all"
|
||||
onClick={(event) => {
|
||||
event.stopPropagation()
|
||||
void handleUnstageAll()
|
||||
}}
|
||||
disabled={isExecutingBulk}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
{items.some((entry) => entry.conflictStatus === 'unresolved') ? (
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="h-6 px-1.5 text-[10px] text-muted-foreground hover:text-foreground"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation()
|
||||
if (activeWorktreeId && worktreePath) {
|
||||
openAllDiffs(activeWorktreeId, worktreePath, undefined, area)
|
||||
}
|
||||
}}
|
||||
>
|
||||
View all
|
||||
</Button>
|
||||
) : (
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="h-auto px-1.5 py-0.5 text-xs text-muted-foreground hover:text-foreground"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation()
|
||||
if (activeWorktreeId && worktreePath) {
|
||||
openAllDiffs(activeWorktreeId, worktreePath, undefined, area)
|
||||
}
|
||||
}}
|
||||
>
|
||||
View all
|
||||
</Button>
|
||||
)}
|
||||
</>
|
||||
}
|
||||
/>
|
||||
{!isCollapsed &&
|
||||
|
|
@ -1420,25 +1621,31 @@ function SectionHeader({
|
|||
onToggle: () => void
|
||||
actions?: React.ReactNode
|
||||
}): React.JSX.Element {
|
||||
// Why: wrap the toggle button and actions in a shared rounded container
|
||||
// so the hover background spans the entire row instead of clipping around
|
||||
// the label. The outer div keeps the vertical spacing that separates
|
||||
// sections; the inner wrapper owns the hover rectangle.
|
||||
return (
|
||||
<div className="group/section flex items-center pl-1 pr-3 pt-3 pb-1">
|
||||
<button
|
||||
type="button"
|
||||
className="flex flex-1 items-center gap-1 rounded-md px-0.5 py-0.5 text-left text-xs font-semibold uppercase tracking-wider text-foreground/70 hover:bg-accent hover:text-accent-foreground"
|
||||
onClick={onToggle}
|
||||
>
|
||||
<ChevronDown
|
||||
className={cn('size-3.5 shrink-0 transition-transform', isCollapsed && '-rotate-90')}
|
||||
/>
|
||||
<span>{label}</span>
|
||||
<span className="text-[11px] font-medium tabular-nums">{count}</span>
|
||||
{conflictCount > 0 && (
|
||||
<span className="text-[11px] font-medium text-destructive/80">
|
||||
· {conflictCount} conflict{conflictCount === 1 ? '' : 's'}
|
||||
</span>
|
||||
)}
|
||||
</button>
|
||||
<div className="shrink-0 flex items-center">{actions}</div>
|
||||
<div className="pl-1 pr-3 pt-3 pb-1">
|
||||
<div className="group/section flex items-center rounded-md pr-1 hover:bg-accent hover:text-accent-foreground">
|
||||
<button
|
||||
type="button"
|
||||
className="flex flex-1 items-center gap-1 px-0.5 py-0.5 text-left text-xs font-semibold uppercase tracking-wider text-foreground/70 group-hover/section:text-accent-foreground"
|
||||
onClick={onToggle}
|
||||
>
|
||||
<ChevronDown
|
||||
className={cn('size-3.5 shrink-0 transition-transform', isCollapsed && '-rotate-90')}
|
||||
/>
|
||||
<span>{label}</span>
|
||||
<span className="text-[11px] font-medium tabular-nums">{count}</span>
|
||||
{conflictCount > 0 && (
|
||||
<span className="text-[11px] font-medium text-destructive/80">
|
||||
· {conflictCount} conflict{conflictCount === 1 ? '' : 's'}
|
||||
</span>
|
||||
)}
|
||||
</button>
|
||||
<div className="shrink-0 flex items-center">{actions}</div>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
|
@ -1928,26 +2135,61 @@ function EmptyState({
|
|||
)
|
||||
}
|
||||
|
||||
function ActionButton({
|
||||
export function ActionButton({
|
||||
icon: Icon,
|
||||
title,
|
||||
onClick
|
||||
onClick,
|
||||
disabled
|
||||
}: {
|
||||
icon: React.ComponentType<{ className?: string }>
|
||||
title: string
|
||||
onClick: (event: React.MouseEvent) => void
|
||||
disabled?: boolean
|
||||
}): React.JSX.Element {
|
||||
// Why: use the Radix Tooltip instead of the native `title` attribute so the
|
||||
// label matches the rest of the sidebar chrome (consistent styling, no OS
|
||||
// delay quirks, dismissible on pointer leave).
|
||||
//
|
||||
// Why (no local TooltipProvider): the app root mounts a single
|
||||
// TooltipProvider (see App.tsx); nesting another one here gives this subtree
|
||||
// its own delay-timing state and breaks Radix's "skip the open delay when
|
||||
// moving between adjacent tooltip triggers" handoff between sibling action
|
||||
// buttons in the section header.
|
||||
//
|
||||
// Why (disabled handling): Radix's TooltipTrigger asChild on a disabled
|
||||
// <button> gets pointer-events blocked in Chromium, which suppresses the
|
||||
// tooltip entirely — a regression vs. the native `title` attribute it
|
||||
// replaced. We keep the button interactive and rely on the caller's
|
||||
// `isExecutingBulk` early-return to no-op the click during bulk ops;
|
||||
// `aria-disabled` + visual dimming preserves the disabled affordance.
|
||||
return (
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
className="h-auto w-auto p-0.5 text-muted-foreground hover:text-foreground"
|
||||
title={title}
|
||||
onClick={onClick}
|
||||
>
|
||||
<Icon className="size-3.5" />
|
||||
</Button>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-xs"
|
||||
className={cn(
|
||||
'h-auto w-auto p-0.5 text-muted-foreground hover:text-foreground',
|
||||
disabled && 'opacity-50 cursor-not-allowed'
|
||||
)}
|
||||
aria-label={title}
|
||||
aria-disabled={disabled}
|
||||
onClick={(event) => {
|
||||
if (disabled) {
|
||||
event.preventDefault()
|
||||
return
|
||||
}
|
||||
onClick(event)
|
||||
}}
|
||||
>
|
||||
<Icon className="size-3.5" />
|
||||
</Button>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" sideOffset={6}>
|
||||
{title}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,284 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
getDiscardAllPaths,
|
||||
getStageAllPaths,
|
||||
getUnstageAllPaths,
|
||||
runDiscardAllForArea,
|
||||
type DiscardAllArea
|
||||
} from './discard-all-sequence'
|
||||
import type { GitStatusEntry } from '../../../../shared/types'
|
||||
|
||||
function entry(partial: Partial<GitStatusEntry> & { path: string }): GitStatusEntry {
|
||||
return {
|
||||
status: 'modified',
|
||||
area: 'unstaged',
|
||||
...partial
|
||||
}
|
||||
}
|
||||
|
||||
describe('getDiscardAllPaths', () => {
|
||||
it('returns only paths in the requested area', () => {
|
||||
const entries: GitStatusEntry[] = [
|
||||
entry({ path: 'a.ts', area: 'staged' }),
|
||||
entry({ path: 'b.ts', area: 'unstaged' }),
|
||||
entry({ path: 'c.ts', area: 'untracked', status: 'untracked' })
|
||||
]
|
||||
expect(getDiscardAllPaths(entries, 'staged')).toEqual(['a.ts'])
|
||||
expect(getDiscardAllPaths(entries, 'unstaged')).toEqual(['b.ts'])
|
||||
expect(getDiscardAllPaths(entries, 'untracked')).toEqual(['c.ts'])
|
||||
})
|
||||
|
||||
it('skips entries with an unresolved conflict', () => {
|
||||
const entries: GitStatusEntry[] = [
|
||||
entry({ path: 'clean.ts', area: 'unstaged' }),
|
||||
entry({
|
||||
path: 'conflict.ts',
|
||||
area: 'unstaged',
|
||||
conflictKind: 'both_modified',
|
||||
conflictStatus: 'unresolved'
|
||||
})
|
||||
]
|
||||
// Why: `git restore --worktree --source=HEAD` on an unresolved conflict
|
||||
// clears the `u` record silently before the user has reviewed it, which
|
||||
// is why the per-row Stage/Discard buttons also suppress this case.
|
||||
expect(getDiscardAllPaths(entries, 'unstaged')).toEqual(['clean.ts'])
|
||||
})
|
||||
|
||||
it('skips entries resolved locally but not yet re-staged', () => {
|
||||
const entries: GitStatusEntry[] = [
|
||||
entry({ path: 'clean.ts', area: 'unstaged' }),
|
||||
entry({
|
||||
path: 'resolved.ts',
|
||||
area: 'unstaged',
|
||||
conflictKind: 'both_modified',
|
||||
conflictStatus: 'resolved_locally'
|
||||
})
|
||||
]
|
||||
// Why: discarding a locally-resolved file loses the resolution. The user
|
||||
// would have to re-resolve from scratch — treat it as too dangerous to
|
||||
// include in a bulk action.
|
||||
expect(getDiscardAllPaths(entries, 'unstaged')).toEqual(['clean.ts'])
|
||||
})
|
||||
|
||||
it('returns an empty array when nothing matches', () => {
|
||||
expect(getDiscardAllPaths([], 'staged')).toEqual([])
|
||||
expect(getDiscardAllPaths([entry({ path: 'a.ts', area: 'staged' })], 'unstaged')).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('getStageAllPaths', () => {
|
||||
it('returns only paths in the requested area', () => {
|
||||
const entries: GitStatusEntry[] = [
|
||||
entry({ path: 'a.ts', area: 'staged' }),
|
||||
entry({ path: 'b.ts', area: 'unstaged' }),
|
||||
entry({ path: 'c.ts', area: 'untracked', status: 'untracked' })
|
||||
]
|
||||
expect(getStageAllPaths(entries, 'unstaged')).toEqual(['b.ts'])
|
||||
expect(getStageAllPaths(entries, 'untracked')).toEqual(['c.ts'])
|
||||
})
|
||||
|
||||
it('skips entries with an unresolved conflict', () => {
|
||||
const entries: GitStatusEntry[] = [
|
||||
entry({ path: 'clean.ts', area: 'unstaged' }),
|
||||
entry({
|
||||
path: 'conflict.ts',
|
||||
area: 'unstaged',
|
||||
conflictKind: 'both_modified',
|
||||
conflictStatus: 'unresolved'
|
||||
})
|
||||
]
|
||||
// Why: `git add` on an unresolved conflict silently clears the `u`
|
||||
// record before the user has reviewed it — same hazard the per-row
|
||||
// Stage button guards against.
|
||||
expect(getStageAllPaths(entries, 'unstaged')).toEqual(['clean.ts'])
|
||||
})
|
||||
|
||||
it('includes entries that are resolved locally', () => {
|
||||
const entries: GitStatusEntry[] = [
|
||||
entry({ path: 'clean.ts', area: 'unstaged' }),
|
||||
entry({
|
||||
path: 'resolved.ts',
|
||||
area: 'unstaged',
|
||||
conflictKind: 'both_modified',
|
||||
conflictStatus: 'resolved_locally'
|
||||
})
|
||||
]
|
||||
// Why: staging a locally-resolved file is the normal resolution
|
||||
// workflow — it marks the conflict as finished. Unlike discard, this
|
||||
// must NOT be filtered out.
|
||||
expect(getStageAllPaths(entries, 'unstaged')).toEqual(['clean.ts', 'resolved.ts'])
|
||||
})
|
||||
|
||||
it('returns an empty array when nothing matches', () => {
|
||||
expect(getStageAllPaths([], 'unstaged')).toEqual([])
|
||||
expect(getStageAllPaths([entry({ path: 'a.ts', area: 'staged' })], 'unstaged')).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('getUnstageAllPaths', () => {
|
||||
it('returns only staged-area paths', () => {
|
||||
const entries: GitStatusEntry[] = [
|
||||
entry({ path: 'a.ts', area: 'staged' }),
|
||||
entry({ path: 'b.ts', area: 'unstaged' }),
|
||||
entry({ path: 'c.ts', area: 'untracked', status: 'untracked' })
|
||||
]
|
||||
expect(getUnstageAllPaths(entries)).toEqual(['a.ts'])
|
||||
})
|
||||
|
||||
it('includes staged conflict rows', () => {
|
||||
const entries: GitStatusEntry[] = [
|
||||
entry({ path: 'clean.ts', area: 'staged' }),
|
||||
entry({
|
||||
path: 'conflict.ts',
|
||||
area: 'staged',
|
||||
conflictKind: 'both_modified',
|
||||
conflictStatus: 'unresolved'
|
||||
}),
|
||||
entry({
|
||||
path: 'resolved.ts',
|
||||
area: 'staged',
|
||||
conflictKind: 'both_modified',
|
||||
conflictStatus: 'resolved_locally'
|
||||
})
|
||||
]
|
||||
// Why: `git reset HEAD` on a staged conflict row is safe and mirrors
|
||||
// the per-row Unstage action — no conflict filter here.
|
||||
expect(getUnstageAllPaths(entries)).toEqual(['clean.ts', 'conflict.ts', 'resolved.ts'])
|
||||
})
|
||||
|
||||
it('returns an empty array when nothing is staged', () => {
|
||||
expect(getUnstageAllPaths([])).toEqual([])
|
||||
expect(getUnstageAllPaths([entry({ path: 'a.ts', area: 'unstaged' })])).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('runDiscardAllForArea', () => {
|
||||
function makeDeps(
|
||||
overrides: {
|
||||
bulkUnstageError?: unknown
|
||||
discardOneError?: (path: string) => unknown
|
||||
} = {}
|
||||
) {
|
||||
const bulkUnstageCalls: string[][] = []
|
||||
const discardOneCalls: string[] = []
|
||||
const errors: unknown[] = []
|
||||
|
||||
const bulkUnstage = vi.fn(async (paths: string[]) => {
|
||||
bulkUnstageCalls.push([...paths])
|
||||
if (overrides.bulkUnstageError !== undefined) {
|
||||
throw overrides.bulkUnstageError
|
||||
}
|
||||
})
|
||||
const discardOne = vi.fn(async (path: string) => {
|
||||
discardOneCalls.push(path)
|
||||
if (overrides.discardOneError) {
|
||||
const err = overrides.discardOneError(path)
|
||||
if (err !== undefined) {
|
||||
throw err
|
||||
}
|
||||
}
|
||||
})
|
||||
const onError = vi.fn((error: unknown) => {
|
||||
errors.push(error)
|
||||
})
|
||||
|
||||
return {
|
||||
deps: { bulkUnstage, discardOne, onError },
|
||||
bulkUnstageCalls,
|
||||
discardOneCalls,
|
||||
errors,
|
||||
bulkUnstage,
|
||||
discardOne,
|
||||
onError
|
||||
}
|
||||
}
|
||||
|
||||
it('no-ops when the path list is empty', async () => {
|
||||
const ctx = makeDeps()
|
||||
const result = await runDiscardAllForArea('staged', [], ctx.deps)
|
||||
expect(result).toEqual({ discarded: [], failed: [], aborted: false })
|
||||
expect(ctx.bulkUnstage).not.toHaveBeenCalled()
|
||||
expect(ctx.discardOne).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('discards unstaged paths one-by-one without bulk-unstaging', async () => {
|
||||
const ctx = makeDeps()
|
||||
const result = await runDiscardAllForArea('unstaged', ['a.ts', 'b.ts'], ctx.deps)
|
||||
expect(result).toEqual({ discarded: ['a.ts', 'b.ts'], failed: [], aborted: false })
|
||||
expect(ctx.bulkUnstage).not.toHaveBeenCalled()
|
||||
expect(ctx.discardOneCalls).toEqual(['a.ts', 'b.ts'])
|
||||
})
|
||||
|
||||
it('discards untracked paths one-by-one without bulk-unstaging', async () => {
|
||||
const ctx = makeDeps()
|
||||
const result = await runDiscardAllForArea('untracked', ['new.ts'], ctx.deps)
|
||||
expect(result).toEqual({ discarded: ['new.ts'], failed: [], aborted: false })
|
||||
expect(ctx.bulkUnstage).not.toHaveBeenCalled()
|
||||
expect(ctx.discardOneCalls).toEqual(['new.ts'])
|
||||
})
|
||||
|
||||
it('bulk-unstages staged paths before the per-file discard loop', async () => {
|
||||
const ctx = makeDeps()
|
||||
const result = await runDiscardAllForArea('staged', ['a.ts', 'b.ts'], ctx.deps)
|
||||
expect(result).toEqual({ discarded: ['a.ts', 'b.ts'], failed: [], aborted: false })
|
||||
expect(ctx.bulkUnstageCalls).toEqual([['a.ts', 'b.ts']])
|
||||
expect(ctx.discardOneCalls).toEqual(['a.ts', 'b.ts'])
|
||||
// Why: bulk unstage MUST happen strictly before any discard, otherwise
|
||||
// the index would still hold the staged delta when the worktree was
|
||||
// reset and the files would reappear as inverse changes.
|
||||
expect(ctx.bulkUnstage.mock.invocationCallOrder[0]).toBeLessThan(
|
||||
ctx.discardOne.mock.invocationCallOrder[0]
|
||||
)
|
||||
})
|
||||
|
||||
it('aborts and skips the discard loop if bulk-unstage rejects', async () => {
|
||||
const error = new Error('index locked')
|
||||
const ctx = makeDeps({ bulkUnstageError: error })
|
||||
const result = await runDiscardAllForArea('staged', ['a.ts', 'b.ts'], ctx.deps)
|
||||
expect(result).toEqual({ discarded: [], failed: [], aborted: true })
|
||||
// Why: a failed unstage + successful discard would leave the index with
|
||||
// the staged delta and the worktree at HEAD — a worse state than we
|
||||
// started in. The discard loop must not run.
|
||||
expect(ctx.discardOne).not.toHaveBeenCalled()
|
||||
expect(ctx.errors).toEqual([error])
|
||||
})
|
||||
|
||||
it('continues past a per-file discard failure and records it in `failed`', async () => {
|
||||
const error = new Error('EPERM')
|
||||
const ctx = makeDeps({
|
||||
discardOneError: (path) => (path === 'b.ts' ? error : undefined)
|
||||
})
|
||||
const result = await runDiscardAllForArea('unstaged', ['a.ts', 'b.ts', 'c.ts'], ctx.deps)
|
||||
// Why: best-effort continuation — one stuck file shouldn't block the
|
||||
// rest of a bulk action the user explicitly triggered.
|
||||
expect(result).toEqual({
|
||||
discarded: ['a.ts', 'c.ts'],
|
||||
failed: ['b.ts'],
|
||||
aborted: false
|
||||
})
|
||||
expect(ctx.discardOneCalls).toEqual(['a.ts', 'b.ts', 'c.ts'])
|
||||
// Why: `aborted` is reserved for the pre-step (bulk unstage) failing —
|
||||
// per-file failures don't trip it, otherwise callers couldn't
|
||||
// distinguish "nothing ran" from "some ran, some didn't".
|
||||
expect(ctx.errors).toEqual([error])
|
||||
})
|
||||
|
||||
it('does not invoke the error callback on a happy-path staged run', async () => {
|
||||
const ctx = makeDeps()
|
||||
await runDiscardAllForArea('staged', ['a.ts'], ctx.deps)
|
||||
expect(ctx.onError).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not bulk-unstage for non-staged areas even if the dep is provided', async () => {
|
||||
const ctx = makeDeps()
|
||||
const areas: DiscardAllArea[] = ['unstaged', 'untracked']
|
||||
for (const area of areas) {
|
||||
await runDiscardAllForArea(area, ['x.ts'], ctx.deps)
|
||||
}
|
||||
// Why: the unstage step is specific to the staged area's two-step
|
||||
// reset. Accidentally invoking it for unstaged/untracked would be a
|
||||
// no-op for unstaged entries but could mask a regression where staged
|
||||
// entries leak into those paths.
|
||||
expect(ctx.bulkUnstage).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,122 @@
|
|||
import type { GitStatusEntry } from '../../../../shared/types'
|
||||
|
||||
export type DiscardAllArea = 'staged' | 'unstaged' | 'untracked'
|
||||
|
||||
/**
|
||||
* Collect the paths a "Discard all" bulk action should operate on for a given
|
||||
* area. Unresolved and locally-resolved conflicts are excluded — discarding
|
||||
* those can silently re-create the conflict or lose the resolution.
|
||||
*/
|
||||
export function getDiscardAllPaths(
|
||||
entries: readonly GitStatusEntry[],
|
||||
area: DiscardAllArea
|
||||
): string[] {
|
||||
return entries
|
||||
.filter(
|
||||
(entry) =>
|
||||
entry.area === area &&
|
||||
entry.conflictStatus !== 'unresolved' &&
|
||||
entry.conflictStatus !== 'resolved_locally'
|
||||
)
|
||||
.map((entry) => entry.path)
|
||||
}
|
||||
|
||||
export type StageAllArea = 'unstaged' | 'untracked'
|
||||
|
||||
/**
|
||||
* Collect the paths a "Stage all" action should operate on.
|
||||
* Unresolved conflict rows are excluded — `git add` on a conflicted file
|
||||
* silently clears the `u` record before the user has reviewed it.
|
||||
* `resolved_locally` rows are intentionally INCLUDED: staging them is how the
|
||||
* user finalises the resolution and mirrors the per-row Stage button.
|
||||
*/
|
||||
export function getStageAllPaths(entries: readonly GitStatusEntry[], area: StageAllArea): string[] {
|
||||
return entries
|
||||
.filter((entry) => entry.area === area && entry.conflictStatus !== 'unresolved')
|
||||
.map((entry) => entry.path)
|
||||
}
|
||||
|
||||
/**
|
||||
* Collect the paths an "Unstage all" action should operate on.
|
||||
* Every staged row is eligible — `git reset HEAD` on a staged conflict
|
||||
* row is safe and mirrors the per-row Unstage action.
|
||||
*/
|
||||
export function getUnstageAllPaths(entries: readonly GitStatusEntry[]): string[] {
|
||||
return entries.filter((entry) => entry.area === 'staged').map((entry) => entry.path)
|
||||
}
|
||||
|
||||
export type DiscardAllDeps = {
|
||||
/** Unstage the given paths in one IPC round-trip. Only called for 'staged'. */
|
||||
bulkUnstage: (paths: string[]) => Promise<void>
|
||||
/** Discard a single path (restore working-tree to HEAD, or rm if untracked). */
|
||||
discardOne: (path: string) => Promise<void>
|
||||
/**
|
||||
* Called when either the pre-step (bulkUnstage) rejects OR an individual
|
||||
* `discardOne` rejects. Invoked once per failure so callers can surface
|
||||
* each error (e.g. a toast per stuck file) rather than swallowing them.
|
||||
*/
|
||||
onError?: (error: unknown) => void
|
||||
}
|
||||
|
||||
export type DiscardAllResult = {
|
||||
/** Paths whose `discardOne` call resolved successfully. */
|
||||
discarded: string[]
|
||||
/** Paths whose `discardOne` call rejected. Best-effort: the loop continues past these. */
|
||||
failed: string[]
|
||||
/**
|
||||
* True only when the pre-step (bulk unstage for the 'staged' area) failed
|
||||
* and we never entered the per-file discard loop. Per-file failures do
|
||||
* NOT set this flag — they are reported via `failed`.
|
||||
*/
|
||||
aborted: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* Run the "Discard all" sequence for a given area.
|
||||
*
|
||||
* For 'staged', this first bulk-unstages the paths — without that step,
|
||||
* `discardOne` (which maps to `git restore --worktree --source=HEAD`) would
|
||||
* reset the working tree to HEAD but leave the index carrying the staged
|
||||
* delta, producing phantom inverse "Changes" rows the user thought they just
|
||||
* discarded. If the unstage fails we MUST skip the discard loop entirely for
|
||||
* the same reason: a stale index with a clean worktree is a worse state than
|
||||
* the one the user started in.
|
||||
*
|
||||
* Per-file `discardOne` failures are best-effort: we continue past a failed
|
||||
* file so a single stuck path does not block the rest of the bulk action.
|
||||
* Failed paths are reported via `failed`, `onError` is invoked once per
|
||||
* failure, and `aborted` remains `false` because the loop ran end-to-end.
|
||||
*/
|
||||
export async function runDiscardAllForArea(
|
||||
area: DiscardAllArea,
|
||||
paths: readonly string[],
|
||||
deps: DiscardAllDeps
|
||||
): Promise<DiscardAllResult> {
|
||||
if (paths.length === 0) {
|
||||
return { discarded: [], failed: [], aborted: false }
|
||||
}
|
||||
|
||||
if (area === 'staged') {
|
||||
try {
|
||||
await deps.bulkUnstage([...paths])
|
||||
} catch (error) {
|
||||
deps.onError?.(error)
|
||||
return { discarded: [], failed: [], aborted: true }
|
||||
}
|
||||
}
|
||||
|
||||
const discarded: string[] = []
|
||||
const failed: string[] = []
|
||||
for (const path of paths) {
|
||||
try {
|
||||
await deps.discardOne(path)
|
||||
discarded.push(path)
|
||||
} catch (error) {
|
||||
// Best-effort: record and continue so one stuck file doesn't block the
|
||||
// rest of the bulk action.
|
||||
failed.push(path)
|
||||
deps.onError?.(error)
|
||||
}
|
||||
}
|
||||
return { discarded, failed, aborted: false }
|
||||
}
|
||||
Loading…
Reference in New Issue