fix: address review findings (#5862)

This commit is contained in:
Jinjing
2026-06-19 18:11:12 -07:00
committed by GitHub
parent 6c941d8e75
commit d063e8d5cb
7 changed files with 119 additions and 7 deletions
@@ -525,6 +525,27 @@ describe('WorktreeCardAgents', () => {
expect(markup).toContain('>+2<')
})
it('rotates the compact summary chevron when collapsed', async () => {
const { CompactAgentSummaryButton } = await import('./worktree-card-compact-agents')
const agents = [
mockAgent({ paneKey: 'tab-1:1', agentType: 'codex', startedAt: 1000, prompt: 'One' })
] as DashboardAgentRowData[]
const markup = renderToStaticMarkup(
<CompactAgentSummaryButton
agents={agents}
subjectLabel="1 agent"
expanded={false}
onToggle={vi.fn()}
/>
)
expect(markup).toContain('aria-expanded="false"')
expect(markup).toContain(
'lucide-chevron-down size-3 shrink-0 transition-transform duration-150 -rotate-90'
)
})
it('uses a neutral compact summary label while expanded', async () => {
const { CompactAgentSummaryButton } = await import('./worktree-card-compact-agents')
const agents = [
@@ -599,7 +599,10 @@ function setLineageFixtureState(
}
}
function setProjectGroupWithoutWorktreeRowsState(filterRepoIds: string[] = []): void {
function setProjectGroupWithoutWorktreeRowsState(
filterRepoIds: string[] = [],
collapsedGroups = new Set<string>()
): void {
const group: ProjectGroup = {
id: 'group-1',
name: 'Imported Services',
@@ -626,7 +629,7 @@ function setProjectGroupWithoutWorktreeRowsState(filterRepoIds: string[] = []):
agentStatusByPaneKey: {},
browserTabsByWorktree: {},
clearPendingRevealWorktreeId: vi.fn(),
collapsedGroups: new Set<string>(),
collapsedGroups,
deleteStateByWorktreeId: {},
filterRepoIds,
...makeFolderWorkspacePathStatusState(),
@@ -773,6 +776,46 @@ describe('WorktreeList lineage child card renderer', () => {
expect(markup).not.toContain('No workspaces found')
})
it('renders a collapse chevron on project group headers with children', async () => {
setProjectGroupWithoutWorktreeRowsState()
const markup = await renderWorktreeListMarkup()
expect(markup).toContain('data-repo-header-collapse-affordance=""')
expect(markup).toContain('aria-expanded="true"')
})
it('renders collapsed project group header affordance state', async () => {
setProjectGroupWithoutWorktreeRowsState([], new Set(['project-group:group-1']))
const markup = await renderWorktreeListMarkup()
expect(markup).toContain('data-repo-header-collapse-affordance=""')
expect(markup).toContain('aria-expanded="false"')
expect(markup).toContain('-rotate-90')
})
it('does not render the project collapse affordance on flat section headers', async () => {
setLineageFixtureState('none')
const markup = await renderWorktreeListMarkup()
expect(markup).not.toContain('data-repo-header-collapse-affordance=""')
})
it('renders a collapse chevron on grouped repo headers with worktrees', async () => {
setLineageFixtureState('repo')
const markup = await renderWorktreeListMarkup()
expect(markup).toContain('data-repo-header-collapse-affordance=""')
expect(markup).toContain('data-repo-header-id="repo-1"')
expect(markup).toContain('aria-expanded="true"')
})
it('does not render the collapse affordance on empty ungrouped projects', async () => {
setEmptyUngroupedProjectState()
const markup = await renderWorktreeListMarkup()
expect(markup).not.toContain('data-repo-header-collapse-affordance=""')
})
it('shows Clear Filters when filters exclude pre-worktree project groups', async () => {
setProjectGroupWithoutWorktreeRowsState(['another-repo'])
const markup = await renderWorktreeListMarkup()
@@ -348,6 +348,14 @@ function handleRepoHeaderActionPointerDown(event: React.PointerEvent<HTMLElement
event.stopPropagation()
}
function handleRepoHeaderCollapseAffordancePointerDown(
event: React.PointerEvent<HTMLElement>
): void {
// Why: repo-header drag arms from the row press surface; keep collapse
// clicks on the hover chevron from promoting into a drag session.
event.stopPropagation()
}
function stopRepoHeaderMenuEvent(event: React.SyntheticEvent<HTMLElement>): void {
event.stopPropagation()
}
@@ -3615,6 +3623,11 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
(isConfirmedStaleFolderPathStatus(projectGroupPathStatus) ||
projectGroupPathStatus.reason === 'ambiguous-connection')
const projectGroupDepth = row.projectGroupDepth ?? 0
const isHeaderCollapsed = collapsedGroups.has(row.key)
// Why: repo/project headers already reveal actions on hover; tuck
// the collapse chevron into that cluster instead of a new surface.
const showHeaderCollapseAffordance =
row.count > 0 && (isRepoHeader || isProjectGroupHeader)
// Why: non-project section headers like "All" are labels for the
// flat list, so they should not reserve project hierarchy indent.
const headerPaddingLeft =
@@ -3652,6 +3665,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
<div
role="button"
tabIndex={0}
aria-expanded={showHeaderCollapseAffordance ? !isHeaderCollapsed : undefined}
data-repo-header-id={projectIdForHeader}
data-workspace-status-drop-target={headerWorkspaceStatus ? '' : undefined}
data-workspace-status={headerWorkspaceStatus ?? undefined}
@@ -3746,6 +3760,27 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
</div>
<ProjectHeaderActions>
{showHeaderCollapseAffordance ? (
<div
className="flex size-5 shrink-0 items-center justify-center rounded-md text-muted-foreground transition-colors hover:bg-accent/70 hover:text-foreground"
data-repo-header-collapse-affordance=""
aria-hidden
onPointerDown={handleRepoHeaderCollapseAffordancePointerDown}
onClick={(event) => {
event.preventDefault()
event.stopPropagation()
toggleGroupWithScrollAnchor(row.key)
}}
>
<ChevronDown
className={cn(
'size-3.5 transition-transform',
isHeaderCollapsed && '-rotate-90'
)}
/>
</div>
) : null}
{isProjectGroupHeader && !row.repo && row.projectGroup?.id ? (
<DropdownMenu modal={false}>
<DropdownMenuTrigger asChild>
@@ -48,7 +48,7 @@ export const PROJECT_HEADER_DRAG_THRESHOLD_PX = 4
const REPO_HEADER_DRAG_HANDLE_SELECTOR = '[data-repo-header-drag-handle]'
const REPO_HEADER_ACTION_SELECTOR =
'[data-repo-header-action], button, a, input, textarea, select, [contenteditable=""], [contenteditable="true"]'
'[data-repo-header-action], [data-repo-header-collapse-affordance], button, a, input, textarea, select, [contenteditable=""], [contenteditable="true"]'
export function isProjectHeaderDragHandleTarget(
target: EventTarget | null,
@@ -34,4 +34,14 @@ describe('repo header action targets', () => {
expect(isRepoHeaderActionTarget(header.querySelector('#label'), header)).toBe(false)
expect(isRepoHeaderActionTarget(header, header)).toBe(false)
})
it('ignores the hover collapse affordance', () => {
const header = createHeader(`
<div data-repo-header-collapse-affordance="">
<span id="chevron"></span>
</div>
`)
expect(isRepoHeaderActionTarget(header.querySelector('#chevron'), header)).toBe(true)
})
})
@@ -53,7 +53,7 @@ export type RepoHeaderDragController = {
// real drag. Below this we treat the press as a normal click (toggle group).
const DRAG_THRESHOLD_PX = 4
const REPO_HEADER_ACTION_SELECTOR =
'[data-repo-header-action], button, a, input, textarea, select, [contenteditable=""], [contenteditable="true"]'
'[data-repo-header-action], [data-repo-header-collapse-affordance], button, a, input, textarea, select, [contenteditable=""], [contenteditable="true"]'
export function isRepoHeaderActionTarget(
target: EventTarget | null,
@@ -1,5 +1,5 @@
import React, { useCallback, useRef } from 'react'
import { ChevronRight } from 'lucide-react'
import { ChevronDown } from 'lucide-react'
import { AgentStateDot } from '@/components/AgentStateDot'
import type { DashboardAgentRow as DashboardAgentRowData } from '@/components/dashboard/useDashboardData'
import { AgentIcon } from '@/lib/agent-catalog'
@@ -179,8 +179,11 @@ export function CompactAgentSummaryButton({
)}
</>
)}
<ChevronRight
className={cn('size-3 shrink-0 transition-transform duration-150', expanded && 'rotate-90')}
<ChevronDown
className={cn(
'size-3 shrink-0 transition-transform duration-150',
!expanded && '-rotate-90'
)}
aria-hidden
/>
</button>