Fix folder PR checks row icons (#5744)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson
2026-06-18 17:54:42 -07:00
committed by GitHub
co-authored by Orca
parent cfc003452c
commit 445a389d77
4 changed files with 438 additions and 32 deletions
@@ -46,7 +46,12 @@ vi.mock('@/store', () => ({
vi.mock('@/i18n/i18n', () => ({
translate: (_key: string, fallback: string, values?: Record<string, unknown>) =>
values ? fallback.replace('{{value0}}', String(values.value0)) : fallback
values
? Object.entries(values).reduce(
(text, [key, value]) => text.replaceAll(`{{${key}}}`, String(value)),
fallback
)
: fallback
}))
vi.mock('@/lib/http-link-routing', () => ({
@@ -150,7 +155,7 @@ function makeWorktree(): Worktree {
}
}
function makeReview(): HostedReviewInfo {
function makeReview(overrides: Partial<HostedReviewInfo> = {}): HostedReviewInfo {
return {
provider: 'github',
number: 12,
@@ -160,16 +165,18 @@ function makeReview(): HostedReviewInfo {
status: 'success',
updatedAt: '2026-01-01T00:00:00.000Z',
mergeable: 'MERGEABLE',
headSha: 'abc'
headSha: 'abc',
...overrides
}
}
function makeCheck(): PRCheckDetail {
function makeCheck(overrides: Partial<PRCheckDetail> = {}): PRCheckDetail {
return {
name: 'verify',
status: 'completed',
conclusion: 'success',
url: 'https://example.test/check/verify'
url: 'https://example.test/check/verify',
...overrides
}
}
@@ -256,6 +263,114 @@ describe('FolderWorkspacePrChecksPanel', () => {
expect(mockState.store.setRightSidebarTab).not.toHaveBeenCalled()
})
it('shows a compact clean-state header summary without noisy aggregate counts', () => {
renderPanel()
expect(container.textContent).toContain('Review checks')
expect(container.textContent).toContain('1 worktree · all checks passing')
expect(container.textContent).not.toContain('1 attached')
expect(container.textContent).not.toContain('with PR/MR')
expect(container.textContent).not.toContain('unknown')
})
it('summarizes failing and pending rows before the worktree count', () => {
const repo = mockState.store.repos[0]
const passingWorktree = mockState.store.worktreesByRepo[repo.id][0]
const failingWorktree = {
...makeWorktree(),
id: 'repo-1::/failing-child',
path: '/failing-child',
head: 'def',
branch: 'refs/heads/failing-child',
displayName: 'Failing child',
linkedPR: 13,
lastActivityAt: 2
}
const pendingWorktree = {
...makeWorktree(),
id: 'repo-1::/pending-child',
path: '/pending-child',
head: 'fed',
branch: 'refs/heads/pending-child',
displayName: 'Pending child',
linkedPR: 14,
lastActivityAt: 1
}
const reviewsByBranch = {
feature: makeReview(),
'failing-child': makeReview({
number: 13,
title: 'Failing review',
status: 'failure',
headSha: 'def'
}),
'pending-child': makeReview({
number: 14,
title: 'Pending review',
status: 'pending',
headSha: 'fed'
})
}
const worktrees = [passingWorktree, failingWorktree, pendingWorktree]
mockState.store.worktreesByRepo = { [repo.id]: worktrees }
mockState.store.workspaceLineageByChildKey = Object.fromEntries(
worktrees.map((worktree, index) => [
worktreeWorkspaceKey(worktree.id),
{
childWorkspaceKey: worktreeWorkspaceKey(worktree.id),
childInstanceId: null,
parentWorkspaceKey: folderWorkspaceKey('folder-1'),
parentInstanceId: null,
origin: 'cli',
capture: { source: 'env-workspace', confidence: 'inferred' },
createdAt: index + 1
}
])
)
mockState.store.hostedReviewCache = {
[getHostedReviewCacheKey(repo.path, 'feature', null, repo.id)]: {
data: reviewsByBranch.feature,
fetchedAt: 1
},
[getHostedReviewCacheKey(repo.path, 'failing-child', null, repo.id)]: {
data: reviewsByBranch['failing-child'],
fetchedAt: 1
},
[getHostedReviewCacheKey(repo.path, 'pending-child', null, repo.id)]: {
data: reviewsByBranch['pending-child'],
fetchedAt: 1
}
}
mockState.store.checksCache = {
[getGitHubRepoCacheKey(repo.path, repo.id, prChecksCacheSuffix(12, null, 'abc'), null)]: {
data: [makeCheck()],
fetchedAt: 1,
headSha: 'abc'
},
[getGitHubRepoCacheKey(repo.path, repo.id, prChecksCacheSuffix(13, null, 'def'), null)]: {
data: [makeCheck({ name: 'unit-tests', conclusion: 'failure' })],
fetchedAt: 1,
headSha: 'def'
},
[getGitHubRepoCacheKey(repo.path, repo.id, prChecksCacheSuffix(14, null, 'fed'), null)]: {
data: [makeCheck({ name: 'integration', status: 'in_progress', conclusion: null })],
fetchedAt: 1,
headSha: 'fed'
}
}
mockState.store.fetchHostedReviewForBranch = vi.fn(
async (_repoPath: string, branch: keyof typeof reviewsByBranch) =>
reviewsByBranch[branch] ?? null
)
renderPanel()
expect(container.textContent).toContain('Review checks')
expect(container.textContent).toContain('1 failing · 1 pending · 3 worktrees')
expect(container.textContent).not.toContain('with PR/MR')
expect(container.textContent).not.toContain('unknown')
})
it('opens external review links without activating the row', () => {
renderPanel()
@@ -79,6 +79,10 @@ export default function FolderWorkspacePrChecksPanel({
[childWorktrees, repos, settings, hostedReviewCache, prCache, checksCache, refreshOutcomes]
)
const folderWorkspaceId = folderWorkspace?.id ?? null
const headerSummary = useMemo(
() => formatReviewChecksHeaderSummary(projection.summary),
[projection.summary]
)
const refreshCandidates = useMemo(
() => getParentPrChecksRefreshCandidates({ worktrees: childWorktrees, repos }),
[childWorktrees, repos]
@@ -210,11 +214,14 @@ export default function FolderWorkspacePrChecksPanel({
<div className="flex items-center gap-2">
<div className="min-w-0 flex-1">
<div className="truncate text-sm font-medium text-foreground">
{folderWorkspace.name}
</div>
<div className="mt-1 text-xs text-muted-foreground">
{formatSummary(projection.summary)}
{translate(
'auto.components.rightSidebar.FolderWorkspacePrChecksPanel.reviewChecks',
'Review checks'
)}
</div>
{headerSummary ? (
<div className="mt-1 truncate text-xs text-muted-foreground">{headerSummary}</div>
) : null}
</div>
<Tooltip>
<TooltipTrigger asChild>
@@ -276,26 +283,71 @@ export default function FolderWorkspacePrChecksPanel({
)
}
function formatSummary(summary: {
function formatReviewChecksHeaderSummary(summary: {
attached: number
knownReview: number
failing: number
pending: number
passing: number
noPr: number
unknown: number
}): string {
return translate(
'auto.components.rightSidebar.FolderWorkspacePrChecksPanel.summary',
'{{value0}} attached · {{value1}} with PR/MR · {{value2}} attention · {{value3}} pending · {{value4}} passing · {{value5}} no PR · {{value6}} unknown',
{
value0: summary.attached,
value1: summary.knownReview,
value2: summary.failing,
value3: summary.pending,
value4: summary.passing,
value5: summary.noPr,
value6: summary.unknown
}
)
}): string | null {
if (summary.attached === 0) {
return null
}
const worktreeCount = formatWorktreeCount(summary.attached)
const attentionParts = [
summary.failing > 0 ? formatFailingCount(summary.failing) : null,
summary.pending > 0 ? formatPendingCount(summary.pending) : null
].filter((part): part is string => part !== null)
if (attentionParts.length > 0) {
return [...attentionParts, worktreeCount].join(' · ')
}
if (summary.passing === summary.attached) {
return [
worktreeCount,
translate(
'auto.components.rightSidebar.FolderWorkspacePrChecksPanel.allChecksPassing',
'all checks passing'
)
].join(' · ')
}
return worktreeCount
}
function formatWorktreeCount(count: number): string {
return count === 1
? translate(
'auto.components.rightSidebar.FolderWorkspacePrChecksPanel.oneWorktree',
'1 worktree'
)
: translate(
'auto.components.rightSidebar.FolderWorkspacePrChecksPanel.worktreeCount',
'{{value0}} worktrees',
{ value0: count }
)
}
function formatFailingCount(count: number): string {
return count === 1
? translate(
'auto.components.rightSidebar.FolderWorkspacePrChecksPanel.oneFailing',
'1 failing'
)
: translate(
'auto.components.rightSidebar.FolderWorkspacePrChecksPanel.failingCount',
'{{value0}} failing',
{ value0: count }
)
}
function formatPendingCount(count: number): string {
return count === 1
? translate(
'auto.components.rightSidebar.FolderWorkspacePrChecksPanel.onePending',
'1 pending'
)
: translate(
'auto.components.rightSidebar.FolderWorkspacePrChecksPanel.pendingCount',
'{{value0}} pending',
{ value0: count }
)
}
@@ -0,0 +1,226 @@
// @vitest-environment happy-dom
import { act, type ReactNode } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { CheckStatus, Repo, Worktree } from '../../../../shared/types'
import type { ParentPrChecksRow, ParentPrChecksRowStatus } from './parent-pr-checks-rows'
vi.mock('@/i18n/i18n', () => ({
translate: (_key: string, fallback: string, values?: Record<string, unknown>) =>
values ? fallback.replace('{{value0}}', String(values.value0)) : fallback
}))
vi.mock('@/lib/http-link-routing', () => ({
openHttpLink: vi.fn()
}))
vi.mock('@/components/ui/tooltip', () => ({
Tooltip: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipContent: ({ children }: { children: ReactNode }) => <span>{children}</span>,
TooltipTrigger: ({ children }: { children: ReactNode }) => <>{children}</>
}))
vi.mock('./checks-panel-content', () => ({
CHECK_COLOR: {
success: 'success-color',
failure: 'failure-color',
pending: 'pending-color',
neutral: 'neutral-color'
},
CHECK_ICON: {
success: (props: { className?: string }) => <span data-icon="success" {...props} />,
failure: (props: { className?: string }) => <span data-icon="failure" {...props} />,
pending: (props: { className?: string }) => <span data-icon="pending" {...props} />,
neutral: (props: { className?: string }) => <span data-icon="neutral" {...props} />
},
ChecksList: () => <div data-testid="checks-list" />,
PullRequestIcon: (props: { className?: string }) => <span data-icon="review" {...props} />,
prStateColor: () => 'state-color'
}))
import { FolderWorkspacePrChecksRow } from './FolderWorkspacePrChecksRow'
let container: HTMLDivElement
let root: Root
function makeWorktree(): Worktree {
return {
id: 'repo-1::/child',
path: '/child',
head: 'abc',
branch: 'refs/heads/feature',
isBare: false,
isMainWorktree: false,
repoId: 'repo-1',
displayName: 'Enable child worktrees',
comment: '',
linkedIssue: null,
linkedPR: 12,
linkedLinearIssue: null,
linkedGitLabMR: null,
linkedGitLabIssue: null,
isArchived: false,
isUnread: false,
isPinned: false,
sortOrder: 0,
lastActivityAt: 0
}
}
function makeRepo(): Repo {
return {
id: 'repo-1',
path: '/repo',
displayName: 'Repo',
badgeColor: '#fff',
addedAt: 1,
kind: 'git'
}
}
function makeRow(
overrides: Partial<ParentPrChecksRow> & {
status?: ParentPrChecksRowStatus
checkTone?: CheckStatus
} = {}
): ParentPrChecksRow {
return {
id: 'row-1',
refreshIdentity: 'row-1::feature',
worktree: makeWorktree(),
repo: makeRepo(),
branch: 'feature',
status: 'success',
group: 'passing',
checkTone: 'success',
title: 'Review title',
reviewLabel: '#12',
reviewUrl: 'https://example.test/pr/12',
reviewState: 'open',
provider: 'github',
summary: 'Checks passing',
detailNames: [],
checks: [],
isRefreshing: false,
hasLinkedReview: true,
...overrides
}
}
function renderRow(row: ParentPrChecksRow): void {
act(() => {
root.render(
<FolderWorkspacePrChecksRow
row={row}
expanded={false}
onToggle={vi.fn()}
onLoadCheckDetails={vi.fn(async () => null)}
/>
)
})
}
function iconNames(): string[] {
return [...container.querySelectorAll<HTMLElement>('[data-icon]')].map(
(icon) => icon.dataset.icon ?? ''
)
}
describe('FolderWorkspacePrChecksRow', () => {
beforeEach(() => {
container = document.createElement('div')
document.body.appendChild(container)
root = createRoot(container)
})
afterEach(() => {
act(() => root.unmount())
container.remove()
})
it.each([
{
status: 'success' as const,
checkTone: 'success' as const,
icon: 'success',
color: 'success-color',
summary: 'Checks passing'
},
{
status: 'failing' as const,
checkTone: 'failure' as const,
icon: 'failure',
color: 'failure-color',
summary: 'Checks failing'
}
])('shows $icon status as summary metadata after the review identity', (state) => {
renderRow(
makeRow({
status: state.status,
checkTone: state.checkTone,
summary: state.summary
})
)
expect(iconNames()).toEqual(['review', state.icon])
expect(container.textContent).toContain('Enable child worktrees')
expect(container.textContent).toContain(state.summary)
expect(container.querySelector(`[data-icon="${state.icon}"]`)?.className).toContain(state.color)
expect(container.querySelector(`[data-icon="${state.icon}"]`)?.className).not.toContain(
'animate-spin'
)
})
it('spins pending summary status like regular pending check rows', () => {
renderRow(
makeRow({
status: 'pending',
group: 'pending',
checkTone: 'pending',
summary: 'Checks pending'
})
)
expect(iconNames()).toEqual(['review', 'pending'])
expect(container.querySelector('[data-icon="pending"]')?.className).toContain('pending-color')
expect(container.querySelector('[data-icon="pending"]')?.className).toContain('animate-spin')
})
it('spins loading summary status because it uses the pending check icon', () => {
renderRow(
makeRow({
status: 'loading',
group: 'unavailable',
checkTone: 'pending',
reviewLabel: null,
reviewUrl: null,
reviewState: null,
provider: null,
summary: 'Checking review status...'
})
)
expect(iconNames()).toEqual(['review', 'pending'])
expect(container.querySelector('[data-icon="pending"]')?.className).toContain('animate-spin')
})
it('does not render a dashed neutral status glyph for rows without a known check state', () => {
renderRow(
makeRow({
status: 'noReview',
group: 'noPr',
checkTone: 'neutral',
reviewLabel: null,
reviewUrl: null,
reviewState: null,
provider: null,
summary: 'No PR linked'
})
)
expect(iconNames()).toEqual(['review'])
expect(container.querySelector('[data-icon="neutral"]')).toBeNull()
expect(container.textContent).toContain('No PR linked')
})
})
@@ -1,4 +1,4 @@
import { ChevronRight, ExternalLink } from 'lucide-react'
import { ChevronRight, ExternalLink, GitMerge } from 'lucide-react'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { cn } from '@/lib/utils'
import { openHttpLink } from '@/lib/http-link-routing'
@@ -26,7 +26,12 @@ export function FolderWorkspacePrChecksRow({
onToggle,
onLoadCheckDetails
}: FolderWorkspacePrChecksRowProps): React.JSX.Element {
const Icon = CHECK_ICON[row.checkTone] ?? CHECK_ICON.neutral
const ReviewIcon = row.provider === 'gitlab' ? GitMerge : PullRequestIcon
const StatusIcon = CHECK_ICON[row.checkTone] ?? CHECK_ICON.neutral
// Why: match the regular PR checks header; the review identity leads,
// while aggregate check state stays with the summary metadata.
const showStatusIcon = row.checkTone !== 'neutral'
const animateStatusIcon = row.checkTone === 'pending'
const reviewProviderLabel = row.provider === 'gitlab' ? 'MR' : 'PR'
const toggleDetailsLabel = expanded
? translate(
@@ -72,11 +77,20 @@ export function FolderWorkspacePrChecksRow({
expanded && 'rotate-90'
)}
/>
<Icon className={cn('mt-0.5 size-3.5 shrink-0', CHECK_COLOR[row.checkTone])} />
<ReviewIcon className="mt-0.5 size-4 shrink-0 text-muted-foreground" />
<div className="min-w-0 flex-1">
<PrChecksRowHeader row={row} />
<div className="mt-1 truncate text-[12px] text-foreground/90">{row.title}</div>
<div className="mt-1 flex min-w-0 items-center gap-1.5 text-[11px] text-muted-foreground">
{showStatusIcon ? (
<StatusIcon
className={cn(
'size-3 shrink-0',
CHECK_COLOR[row.checkTone],
animateStatusIcon && 'animate-spin'
)}
/>
) : null}
<span className="truncate">{row.summary}</span>
{row.repo ? <span className="shrink-0">· {row.repo.displayName}</span> : null}
{row.branch ? <span className="truncate">· {row.branch}</span> : null}
@@ -130,8 +144,7 @@ function PrChecksRowHeader({ row }: { row: ParentPrChecksRow }): React.JSX.Eleme
{row.worktree.displayName}
</span>
{row.reviewLabel ? (
<span className="inline-flex shrink-0 items-center gap-1 rounded border border-border px-1.5 py-0.5 text-[10px] font-medium text-muted-foreground">
<PullRequestIcon className="size-3" />
<span className="inline-flex shrink-0 items-center rounded border border-border px-1.5 py-0.5 text-[10px] font-medium text-muted-foreground">
{row.reviewLabel}
</span>
) : null}