diff --git a/src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx b/src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx index d1e95db8c74..a219841b949 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx @@ -24,7 +24,6 @@ let issueCache: Record = {} let prCache: Record = {} let workspacePortScan: WorkspacePortScanResult | null = null let settings: Partial | null = null -let activityStatus = 'inactive' vi.mock('@/store', () => ({ useAppStore: (selector: (state: unknown) => unknown) => @@ -61,7 +60,7 @@ vi.mock('@/components/ui/tooltip', () => ({ })) vi.mock('./use-worktree-activity-status', () => ({ - useWorktreeActivityStatus: () => activityStatus + useWorktreeActivityStatus: () => 'inactive' })) vi.mock('./CacheTimer', () => ({ @@ -163,7 +162,6 @@ describe('WorktreeCard linked PR display', () => { prCache = {} workspacePortScan = null settings = null - activityStatus = 'inactive' }) it('keeps linked GH PR status out of the left status slot by default', async () => { @@ -231,10 +229,11 @@ describe('WorktreeCard linked PR display', () => { ) expect(unreadMarkup).not.toContain('aria-label="Mark as read"') - expect(unreadMarkup).toContain('Inactive · Unread') - expect(unreadMarkup).toContain('PR checks: Failed') + expect(unreadMarkup).toContain('PR checks: Failed · Unread') expect(unreadMarkup).not.toContain('Mark read') - expect(unreadMarkup).toContain('size-[13px]') + expect(unreadMarkup).toContain('size-[13px] translate-x-px') + expect(unreadMarkup).not.toContain('lucide-bell') + expect(unreadMarkup).not.toContain('text-amber-500') expect(unreadMarkup).toContain('data-worktree-status-lane-unread=""') expect(unreadMarkup).toContain('data-worktree-unread-alert=""') expect(unreadMarkup).toContain('bg-amber-500') @@ -242,7 +241,7 @@ describe('WorktreeCard linked PR display', () => { expect(getInlineRenameTitleTag(readMarkup)).toContain('font-normal text-foreground/80') }, 20_000) - it('shows linked GH PR status in the title row before hosted review details are cached when new card style is on', async () => { + it('shows linked GH PR status in the left status slot before hosted review details are cached when new card style is on', async () => { settings = { experimentalNewWorktreeCardStyle: true } const { default: WorktreeCard } = await import('./WorktreeCard') @@ -630,10 +629,9 @@ describe('WorktreeCard linked PR display', () => { expect(markup).not.toContain('58941') }) - it('renders activity and linked PR status in independent sidebar lanes', async () => { + it('renders linked PR status in the left status slot instead of the right metadata list', async () => { settings = { experimentalNewWorktreeCardStyle: true } worktreeCardProperties = ['status'] - activityStatus = 'active' hostedReviewCache = { 'local::repo-1::feature/local-branch': { data: makeHostedReview({ status: 'failure' }), @@ -647,13 +645,12 @@ describe('WorktreeCard linked PR display', () => { ) expect(markup).toContain('PR checks: Failed') - expect(markup).toContain('Active') - expect(markup).toContain('bg-emerald-500') - expect(markup).toContain('data-worktree-card-review-status=""') + expect(markup).toContain('text-rose-500/85') expect(markup).not.toContain('Linked PR #456') + expect(markup).not.toContain('CI checks') }) - it('uses branch PR cache for the title-row status before hosted-review metadata warms', async () => { + it('uses branch PR cache for the status slot before hosted-review metadata warms', async () => { settings = { experimentalNewWorktreeCardStyle: true } worktreeCardProperties = ['status'] prCache = { diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index 26a2d020235..b64ffd91cc5 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -26,7 +26,6 @@ import { LinearAgentSkillSetupPrompt } from './LinearAgentSkillSetupPrompt' import WorktreeCardAgents from './WorktreeCardAgents' import { useWorktreeAgentRows } from './useWorktreeAgentRows' import { WorktreeCardStatusSlot } from './WorktreeCardStatusSlot' -import { WorktreeCardReviewStatus } from './WorktreeCardReviewStatus' import { cn } from '@/lib/utils' import { WorktreeCardSshHostControl } from './WorktreeCardSshHostControl' import { activateWorktreeFromSidebar } from '@/lib/sidebar-worktree-activation' @@ -1208,13 +1207,11 @@ const WorktreeCard = React.memo(function WorktreeCard({ !!conflictOperation && conflictOperation !== 'unknown' && conflictOperation !== 'rebase' const hasMetadataBadge = showConflictOperationBadge const showUnreadQuickAction = !affiliateListMode && showStatus && !newCardStyle - // Why: the slot owns activity and unread; review/check state has its own title-row indicator. + // Why: the slot owns the unread/status lane; legacy keeps the bell toggle, the new card keeps the glyph passive. const showCombinedStatusSlot = showStatus - const showReviewStatus = newCardStyle && showStatus && statusLaneReview !== null const showTitleRowPrimary = compactCards && worktree.isMainWorktree && !isFolder const showMetaRowDetails = !newCardStyle && !compactCards && (hasDetails || hasPorts) - const showTitleRowIndicators = - (newCardStyle || compactCards) && (hasDetails || hasPorts || showReviewStatus) + const showTitleRowIndicators = (newCardStyle || compactCards) && (hasDetails || hasPorts) // Why: grouped views can hide the repo badge; don't reserve a blank metadata lane unless there's real content. const hasDetailedMetaRowContent = Boolean( (showRepoBadgeInMetaRow && repo) || @@ -1373,12 +1370,7 @@ const WorktreeCard = React.memo(function WorktreeCard({ detailsAndPortsContent ) const titleRowIndicators = showTitleRowIndicators ? ( -
- {showReviewStatus && statusLaneReview ? ( - - ) : null} - {detailsAndPorts} -
+
{detailsAndPorts}
) : null const hasSecondaryCardContent = hasMetaRow || !!remoteBranchConflict || showInlineAgentList || showLineageChildChip @@ -1412,6 +1404,7 @@ const WorktreeCard = React.memo(function WorktreeCard({ unreadTooltip={unreadTooltip} onPointerDown={stopQuickActionPointerPropagation} onToggleUnread={handleToggleUnreadQuick} + prDisplay={statusLaneReview} newCardStyle={newCardStyle} hasBranchIdentity={hasPathIdentityEnabled && Boolean(branchIdentityDisplay)} /> diff --git a/src/renderer/src/components/sidebar/WorktreeCardReviewStatus.test.tsx b/src/renderer/src/components/sidebar/WorktreeCardReviewStatus.test.tsx deleted file mode 100644 index dab06044df9..00000000000 --- a/src/renderer/src/components/sidebar/WorktreeCardReviewStatus.test.tsx +++ /dev/null @@ -1,53 +0,0 @@ -import { renderToStaticMarkup } from 'react-dom/server' -import type { ReactNode } from 'react' -import { describe, expect, it, vi } from 'vitest' -import { WorktreeCardReviewStatus } from './WorktreeCardReviewStatus' -import type { WorktreeCardPrDisplay } from './worktree-card-pr-display' - -vi.mock('@/components/ui/tooltip', () => ({ - Tooltip: ({ children }: { children: ReactNode }) => <>{children}, - TooltipContent: ({ children }: { children: ReactNode }) => <>{children}, - TooltipTrigger: ({ children }: { children: ReactNode }) => <>{children} -})) - -type ReviewOverrides = { - provider?: Exclude - state?: WorktreeCardPrDisplay['state'] - status?: WorktreeCardPrDisplay['status'] -} - -function renderReview(overrides: ReviewOverrides = {}): string { - const review: WorktreeCardPrDisplay = { - provider: 'github', - number: 123, - title: 'Review me', - state: 'open', - ...overrides - } - return renderToStaticMarkup() -} - -describe('WorktreeCardReviewStatus', () => { - it.each([ - [{ state: 'merged' }, 'PR: Merged'], - [{ state: 'closed' }, 'PR: Closed'], - [{ state: 'draft' }, 'PR: Draft'], - [{ status: 'failure' }, 'PR checks: Failed'], - [{ status: 'pending' }, 'PR checks: Pending'], - [{ status: 'success' }, 'PR checks: Passing'], - [{}, 'PR: Open'] - ] as const)('describes review state %o as %s', (overrides, label) => { - expect(renderReview(overrides)).toContain(label) - }) - - it('uses the compact generic review glyph in its own lane', () => { - const markup = renderReview({ provider: 'gitlab', status: 'pending' }) - - expect(markup).toContain('data-worktree-card-review-status=""') - expect(markup).toContain('MR checks: Pending') - expect(markup).toContain('viewBox="0 0 16 16"') - expect(markup).toContain('size-[13px]') - expect(markup).toContain('text-amber-500/85') - expect(markup).not.toContain('lucide-git-merge') - }) -}) diff --git a/src/renderer/src/components/sidebar/WorktreeCardReviewStatus.tsx b/src/renderer/src/components/sidebar/WorktreeCardReviewStatus.tsx deleted file mode 100644 index cab5ba030c1..00000000000 --- a/src/renderer/src/components/sidebar/WorktreeCardReviewStatus.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' -import type { WorktreeCardPrDisplay } from './worktree-card-pr-display' -import { getReviewLabel, ReviewIcon } from './worktree-review-helpers' - -type WorktreeCardReviewStatusProps = { - review: WorktreeCardPrDisplay -} - -function getReviewStatusTooltip(review: WorktreeCardPrDisplay): string { - const label = getReviewLabel(review) - if (review.state === 'merged') { - return `${label}: Merged` - } - if (review.state === 'closed') { - return `${label}: Closed` - } - if (review.state === 'draft') { - return `${label}: Draft` - } - if (review.status === 'failure') { - return `${label} checks: Failed` - } - if (review.status === 'pending') { - return `${label} checks: Pending` - } - if (review.status === 'success') { - return `${label} checks: Passing` - } - return `${label}: Open` -} - -export function WorktreeCardReviewStatus({ - review -}: WorktreeCardReviewStatusProps): React.JSX.Element { - const tooltip = getReviewStatusTooltip(review) - return ( - - - - - {tooltip} - - - - {tooltip} - - - ) -} diff --git a/src/renderer/src/components/sidebar/WorktreeCardStatusSlot.test.tsx b/src/renderer/src/components/sidebar/WorktreeCardStatusSlot.test.tsx index bd1971bb614..8f204527c9a 100644 --- a/src/renderer/src/components/sidebar/WorktreeCardStatusSlot.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCardStatusSlot.test.tsx @@ -2,6 +2,7 @@ import { renderToStaticMarkup } from 'react-dom/server' import type { ReactNode } from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' import { WorktreeCardStatusSlot } from './WorktreeCardStatusSlot' +import type { WorktreeCardPrDisplay } from './worktree-card-pr-display' const mocks = vi.hoisted(() => ({ status: 'active' @@ -22,6 +23,21 @@ describe('WorktreeCardStatusSlot', () => { mocks.status = 'active' }) + const review: WorktreeCardPrDisplay = { + provider: 'github', + number: 123, + title: 'Review me', + state: 'open', + status: 'failure' + } + const gitlabReview: WorktreeCardPrDisplay = { + provider: 'gitlab', + number: 456, + title: 'Review me', + state: 'open', + status: 'pending' + } + it('lets the unread bell replace the visual status dot by default', () => { const markup = renderToStaticMarkup( { expect(markup).toContain('bg-emerald-500') }) + it('keeps the quiet active dot ahead of PR status by default', () => { + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('Active') + expect(markup).toContain('bg-emerald-500') + expect(markup).not.toContain('PR checks: Failed') + }) + + it('keeps the active dot ahead of PR status when new card style is on', () => { + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('Active') + expect(markup).toContain('inline-flex size-5 items-center justify-center') + expect(markup).toContain('bg-emerald-500') + expect(markup).not.toContain('PR checks: Failed') + }) + it('keeps the emerald activity dot ahead of branch identity when new card style is on', () => { const markup = renderToStaticMarkup( { expect(markup).not.toContain('lucide-git-branch') }) - it('shows done activity in new card style', () => { + it('uses the unified compact review glyph for GitLab MR status', () => { + mocks.status = 'inactive' + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('MR checks: Pending') + expect(markup).toContain('viewBox="0 0 16 16"') + expect(markup).toContain('size-[13px] translate-x-px') + expect(markup).toContain('text-amber-500/85') + expect(markup).not.toContain('lucide-git-merge') + }) + + it('keeps the done dot ahead of PR status when new card style is on', () => { mocks.status = 'done' const markup = renderToStaticMarkup( { unreadTooltip="Mark as unread" onPointerDown={vi.fn()} onToggleUnread={vi.fn()} + prDisplay={review} newCardStyle /> ) expect(markup).toContain('Done') expect(markup).toContain('bg-emerald-500') + expect(markup).not.toContain('PR checks: Failed') + }) + + it('uses PR status instead of the inactive dot when new card style is on', () => { + mocks.status = 'inactive' + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('PR checks: Failed') + expect(markup).toContain('text-rose-500/85') + expect(markup).not.toContain('bg-neutral-500/40') }) it('uses a branch icon with branch-only tooltip copy on quiet rows', () => { @@ -261,7 +363,7 @@ describe('WorktreeCardStatusSlot', () => { expect(markup).not.toContain('lucide-git-branch') }) - it('shows working activity in new card style', () => { + it('keeps working activity ahead of PR status in new card style', () => { mocks.status = 'working' const markup = renderToStaticMarkup( { unreadTooltip="Mark as unread" onPointerDown={vi.fn()} onToggleUnread={vi.fn()} + prDisplay={review} newCardStyle /> ) @@ -279,9 +382,10 @@ describe('WorktreeCardStatusSlot', () => { expect(markup).toContain('Working') expect(markup).toContain('inline-flex size-5 items-center justify-center') expect(markup).toContain('border-yellow-500') + expect(markup).not.toContain('PR checks: Failed') }) - it('shows permission activity in new card style', () => { + it('keeps permission activity ahead of PR status in new card style', () => { mocks.status = 'permission' const markup = renderToStaticMarkup( { unreadTooltip="Mark as unread" onPointerDown={vi.fn()} onToggleUnread={vi.fn()} + prDisplay={review} newCardStyle /> ) @@ -299,6 +404,59 @@ describe('WorktreeCardStatusSlot', () => { expect(markup).toContain('Needs permission') expect(markup).toContain('lucide-message-circle-question-mark') expect(markup).toContain('text-amber-500') + expect(markup).not.toContain('PR checks: Failed') + }) + + it('keeps unread ahead of PR status by default', () => { + const markup = renderToStaticMarkup( + + ) + + expect(markup).toContain('aria-label="Mark as read"') + expect(markup).toContain('Mark as read') + expect(markup).not.toContain('Active · Mark as read') + expect(markup).not.toContain('PR checks: Failed') + expect(markup).not.toContain('bg-emerald-500') + expect(markup).toContain('text-amber-500') + }) + + it('overlays an unread badge on PR status when new card style is on', () => { + mocks.status = 'inactive' + const markup = renderToStaticMarkup( + + ) + + expect(markup).not.toContain('aria-label="Mark as read"') + expect(markup).not.toContain('Mark as read') + expect(markup).toContain('PR checks: Failed · Unread') + expect(markup).toContain('data-worktree-status-lane-unread=""') + expect(markup).toContain('data-worktree-unread-alert=""') + expect(markup).not.toContain('group/unread') + expect(markup).not.toContain('cursor-pointer') + expect(markup).toContain('text-rose-500/85') + expect(markup).toContain('bg-amber-500') + expect(markup).not.toContain('lucide-bell') + expect(markup).not.toContain('text-amber-500') + expect(markup).not.toContain('bg-emerald-500') }) it('overlays an unread badge on the branch icon in new card style', () => { diff --git a/src/renderer/src/components/sidebar/WorktreeCardStatusSlot.tsx b/src/renderer/src/components/sidebar/WorktreeCardStatusSlot.tsx index d1a96bcdfea..527236c5549 100644 --- a/src/renderer/src/components/sidebar/WorktreeCardStatusSlot.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCardStatusSlot.tsx @@ -3,10 +3,12 @@ import { Bell, GitBranch } from 'lucide-react' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { translate } from '@/i18n/i18n' import { cn } from '@/lib/utils' -import { getWorktreeStatusLabel } from '@/lib/worktree-status' +import { getWorktreeStatusLabel, type WorktreeStatus } from '@/lib/worktree-status' import { FilledBellIcon } from './WorktreeCardHelpers' import StatusIndicator from './StatusIndicator' import { useWorktreeActivityStatus } from './use-worktree-activity-status' +import type { WorktreeCardPrDisplay } from './worktree-card-pr-display' +import { getReviewLabel, ReviewIcon } from './worktree-review-helpers' type WorktreeCardStatusSlotProps = { worktreeId: string @@ -16,20 +18,24 @@ type WorktreeCardStatusSlotProps = { unreadTooltip: string onToggleUnread: React.MouseEventHandler onPointerDown: React.PointerEventHandler + prDisplay?: WorktreeCardPrDisplay | null newCardStyle?: boolean hasBranchIdentity?: boolean branchIdentityLabel?: string className?: string } -// Why: the passive label names the identity cue without claiming review state. +// Passive identity only replaces status when the workspace is inactive. +const IDENTITY_REPLACEABLE_STATUSES = new Set(['inactive']) +// Why: a missing review display can also mean provider state is unavailable, +// so the passive label names the identity cue without claiming no review exists. function getDefaultBranchIdentityLabel(): string { return translate('auto.components.sidebar.WorktreeCardStatusSlot.branchIdentity', 'Branch') } // Why: branch-style SVGs are optically left-heavy; this keeps them aligned with // the centered activity dots in the shared status column. -const compactBranchStatusIconClassName = 'size-[13px] translate-x-px' -const branchStatusIconClassName = `${compactBranchStatusIconClassName} text-muted-foreground/70` +const compactReviewAndBranchStatusIconClassName = 'size-[13px] translate-x-px' +const branchStatusIconClassName = `${compactReviewAndBranchStatusIconClassName} text-muted-foreground/70` // Why: a left-edge badge overlays unread on the status glyph without widening // the lane or indenting the title; ring-sidebar cuts the dot out from busy icons. const newCardUnreadAlertClassName = @@ -58,6 +64,29 @@ function overlayNewCardUnreadStatus( ) } +function getReviewStatusTooltip(review: WorktreeCardPrDisplay): string { + const label = getReviewLabel(review) + if (review.state === 'merged') { + return `${label}: Merged` + } + if (review.state === 'closed') { + return `${label}: Closed` + } + if (review.state === 'draft') { + return `${label}: Draft` + } + if (review.status === 'failure') { + return `${label} checks: Failed` + } + if (review.status === 'pending') { + return `${label} checks: Pending` + } + if (review.status === 'success') { + return `${label} checks: Passing` + } + return `${label}: Open` +} + export function WorktreeCardStatusSlot({ worktreeId, showStatus, @@ -66,6 +95,7 @@ export function WorktreeCardStatusSlot({ unreadTooltip, onToggleUnread, onPointerDown, + prDisplay = null, newCardStyle = false, hasBranchIdentity = false, branchIdentityLabel, @@ -73,43 +103,70 @@ export function WorktreeCardStatusSlot({ }: WorktreeCardStatusSlotProps): React.JSX.Element | null { const status = useWorktreeActivityStatus(worktreeId) const statusLabel = getWorktreeStatusLabel(status) || status + const canShowReviewStatus = + newCardStyle && showStatus && prDisplay !== null && IDENTITY_REPLACEABLE_STATUSES.has(status) const canShowBranchStatus = - newCardStyle && showStatus && hasBranchIdentity && status === 'inactive' - const passiveStatusLabel = canShowBranchStatus - ? (branchIdentityLabel ?? getDefaultBranchIdentityLabel()) - : statusLabel + newCardStyle && + showStatus && + hasBranchIdentity && + prDisplay === null && + IDENTITY_REPLACEABLE_STATUSES.has(status) + const passiveStatusLabel = + canShowReviewStatus && prDisplay + ? getReviewStatusTooltip(prDisplay) + : canShowBranchStatus + ? (branchIdentityLabel ?? getDefaultBranchIdentityLabel()) + : statusLabel const passiveStatusTooltip = newCardStyle && isUnread ? `${passiveStatusLabel} · Unread` : passiveStatusLabel // Why: working and permission already own the new-card status lane, but // unread state should still surface in tooltip/sr-only copy and reappear afterward. const showNewCardUnreadAlert = newCardStyle && isUnread && showStatus && status !== 'working' && status !== 'permission' + const reviewStatusIconClassName = compactReviewAndBranchStatusIconClassName const branchStatusIcon =