Refine experimental worktree card status visuals (#5848)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
bb0e98fcd1
commit
8bed35aa29
|
|
@ -124,6 +124,12 @@ function renderWorktreeCardMarkup(element: ReactNode): string {
|
|||
return renderToStaticMarkup(<>{element}</>)
|
||||
}
|
||||
|
||||
function getInlineRenameTitleTag(markup: string): string {
|
||||
const match = markup.match(/<span[^>]*data-worktree-title-inline-rename=""[^>]*>/)
|
||||
expect(match).not.toBeNull()
|
||||
return match?.[0] ?? ''
|
||||
}
|
||||
|
||||
describe('WorktreeCard linked PR display', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
|
@ -146,6 +152,66 @@ describe('WorktreeCard linked PR display', () => {
|
|||
expect(markup).not.toContain('Linked PR #456')
|
||||
}, 20_000)
|
||||
|
||||
it('keeps compact toggle-off unread and read-title visuals legacy', async () => {
|
||||
settings = { compactWorktreeCards: true, experimentalNewWorktreeCardStyle: false }
|
||||
hostedReviewCache = {
|
||||
'local::repo-1::feature/local-branch': {
|
||||
data: makeHostedReview({ status: 'failure' }),
|
||||
fetchedAt: Date.now()
|
||||
}
|
||||
}
|
||||
const { default: WorktreeCard } = await import('./WorktreeCard')
|
||||
|
||||
const unreadMarkup = renderWorktreeCardMarkup(
|
||||
<WorktreeCard
|
||||
worktree={makeWorktree({ linkedPR: 456, isUnread: true })}
|
||||
repo={makeRepo()}
|
||||
isActive={false}
|
||||
/>
|
||||
)
|
||||
const readMarkup = renderWorktreeCardMarkup(
|
||||
<WorktreeCard worktree={makeWorktree({ linkedPR: 456 })} repo={makeRepo()} isActive={false} />
|
||||
)
|
||||
const readTitleTag = getInlineRenameTitleTag(readMarkup)
|
||||
|
||||
expect(unreadMarkup).toContain('aria-label="Mark as read"')
|
||||
expect(unreadMarkup).toContain('text-amber-500')
|
||||
expect(unreadMarkup).not.toContain('PR checks: Failed · Mark read')
|
||||
expect(unreadMarkup).not.toContain('size-[13px] translate-x-px')
|
||||
expect(readTitleTag).toContain('font-normal text-foreground')
|
||||
expect(readTitleTag).not.toContain('text-foreground/80')
|
||||
}, 20_000)
|
||||
|
||||
it('applies experimental unread status and read-title visuals only when enabled', async () => {
|
||||
settings = { compactWorktreeCards: true, experimentalNewWorktreeCardStyle: true }
|
||||
hostedReviewCache = {
|
||||
'local::repo-1::feature/local-branch': {
|
||||
data: makeHostedReview({ status: 'failure' }),
|
||||
fetchedAt: Date.now()
|
||||
}
|
||||
}
|
||||
const { default: WorktreeCard } = await import('./WorktreeCard')
|
||||
|
||||
const unreadMarkup = renderWorktreeCardMarkup(
|
||||
<WorktreeCard
|
||||
worktree={makeWorktree({ linkedPR: 456, isUnread: true })}
|
||||
repo={makeRepo()}
|
||||
isActive={false}
|
||||
/>
|
||||
)
|
||||
const readMarkup = renderWorktreeCardMarkup(
|
||||
<WorktreeCard worktree={makeWorktree({ linkedPR: 456 })} repo={makeRepo()} isActive={false} />
|
||||
)
|
||||
|
||||
expect(unreadMarkup).toContain('aria-label="Mark as read"')
|
||||
expect(unreadMarkup).toContain('PR checks: Failed · Mark read')
|
||||
expect(unreadMarkup).toContain('size-[13px] translate-x-px')
|
||||
expect(unreadMarkup).not.toContain('lucide-bell')
|
||||
expect(unreadMarkup).not.toContain('text-amber-500')
|
||||
expect(getInlineRenameTitleTag(unreadMarkup)).toContain('font-semibold text-foreground')
|
||||
expect(getInlineRenameTitleTag(readMarkup)).toContain('font-normal text-foreground/80')
|
||||
}, 20_000)
|
||||
|
||||
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')
|
||||
|
|
|
|||
|
|
@ -1050,8 +1050,8 @@ const WorktreeCard = React.memo(function WorktreeCard({
|
|||
!!conflictOperation && conflictOperation !== 'unknown' && conflictOperation !== 'rebase'
|
||||
const hasMetadataBadge = showConflictOperationBadge
|
||||
const showUnreadQuickAction = !affiliateListMode && showStatus
|
||||
// Why: the activity dot and unread bell compete for the same tiny sidebar
|
||||
// lane. Keep one slot, and let an active unread bell visually win.
|
||||
// Why: the slot owns the tiny unread/status lane; legacy keeps the bell,
|
||||
// while the experimental card keeps the status glyph visible.
|
||||
const showCombinedStatusSlot = showStatus
|
||||
const showTitleRowPrimary = compactCards && worktree.isMainWorktree && !isFolder
|
||||
const showMetaRowDetails = !newCardStyle && !compactCards && (hasDetails || hasPorts)
|
||||
|
|
@ -1317,13 +1317,13 @@ const WorktreeCard = React.memo(function WorktreeCard({
|
|||
</RepoIdentityChip>
|
||||
)}
|
||||
|
||||
{/* Why: weight alone carries the unread signal; color stays
|
||||
at text-foreground in both states so the title keeps hierarchy
|
||||
against nearby status chips. */}
|
||||
{/* Why: in the experimental card, weight carries unread without a
|
||||
bell, so read titles step back slightly for scan contrast. */}
|
||||
<WorktreeTitleInlineRename
|
||||
displayName={visibleCardTitle}
|
||||
disabled={isDeleting || affiliateListMode}
|
||||
showUnreadEmphasis={showUnreadEmphasis}
|
||||
dimReadTitle={newCardStyle}
|
||||
className="text-[13px] leading-5"
|
||||
editingClassName="flex-1"
|
||||
titleWrapper={titleWrapper}
|
||||
|
|
|
|||
|
|
@ -30,8 +30,15 @@ describe('WorktreeCardStatusSlot', () => {
|
|||
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', () => {
|
||||
it('lets the unread bell replace the visual status dot by default', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<WorktreeCardStatusSlot
|
||||
worktreeId="wt-1"
|
||||
|
|
@ -48,9 +55,32 @@ describe('WorktreeCardStatusSlot', () => {
|
|||
expect(markup).toContain('Mark as read')
|
||||
expect(markup).not.toContain('Active · Mark as read')
|
||||
expect(markup).not.toContain('bg-emerald-500')
|
||||
expect(markup).toContain('text-amber-500')
|
||||
})
|
||||
|
||||
it('shows status until an unread bell is active', () => {
|
||||
it('keeps the status dot visible for unread rows when new card style is on', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<WorktreeCardStatusSlot
|
||||
worktreeId="wt-1"
|
||||
showStatus
|
||||
showUnreadAction
|
||||
isUnread
|
||||
unreadTooltip="Mark as read"
|
||||
onPointerDown={vi.fn()}
|
||||
onToggleUnread={vi.fn()}
|
||||
newCardStyle
|
||||
hasBranchIdentity={false}
|
||||
/>
|
||||
)
|
||||
|
||||
expect(markup).toContain('aria-label="Mark as read"')
|
||||
expect(markup).toContain('Active · Mark as read')
|
||||
expect(markup).toContain('bg-emerald-500')
|
||||
expect(markup).not.toContain('lucide-bell')
|
||||
expect(markup).not.toContain('text-amber-500')
|
||||
})
|
||||
|
||||
it('shows status in the unread toggle affordance', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<WorktreeCardStatusSlot
|
||||
worktreeId="wt-1"
|
||||
|
|
@ -103,10 +133,33 @@ describe('WorktreeCardStatusSlot', () => {
|
|||
|
||||
expect(markup).toContain('PR checks: Failed')
|
||||
expect(markup).toContain('inline-flex size-5 items-center justify-center')
|
||||
expect(markup).toContain('size-[13px] translate-x-px')
|
||||
expect(markup).toContain('text-rose-500/85')
|
||||
expect(markup).not.toContain('bg-emerald-500')
|
||||
})
|
||||
|
||||
it('uses the unified compact review glyph for GitLab MR status', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<WorktreeCardStatusSlot
|
||||
worktreeId="wt-1"
|
||||
showStatus
|
||||
showUnreadAction={false}
|
||||
isUnread={false}
|
||||
unreadTooltip="Mark as unread"
|
||||
onPointerDown={vi.fn()}
|
||||
onToggleUnread={vi.fn()}
|
||||
prDisplay={gitlabReview}
|
||||
newCardStyle
|
||||
/>
|
||||
)
|
||||
|
||||
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('uses PR status instead of the quiet done dot when new card style is on', () => {
|
||||
mocks.status = 'done'
|
||||
const markup = renderToStaticMarkup(
|
||||
|
|
@ -164,6 +217,7 @@ describe('WorktreeCardStatusSlot', () => {
|
|||
|
||||
expect(markup).toContain('Branch')
|
||||
expect(markup).toContain('lucide-git-branch')
|
||||
expect(markup).toContain('size-[13px] translate-x-px text-muted-foreground/70')
|
||||
expect(markup).toContain('text-muted-foreground/70')
|
||||
expect(markup).not.toContain('bg-emerald-500')
|
||||
})
|
||||
|
|
@ -247,11 +301,13 @@ describe('WorktreeCardStatusSlot', () => {
|
|||
|
||||
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 unread on PR status instead of replacing it when new card style is on', () => {
|
||||
it('keeps PR status visible for unread rows when new card style is on', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<WorktreeCardStatusSlot
|
||||
worktreeId="wt-1"
|
||||
|
|
@ -273,11 +329,12 @@ describe('WorktreeCardStatusSlot', () => {
|
|||
'group/unread relative flex cursor-pointer items-center justify-center rounded transition-all size-5'
|
||||
)
|
||||
expect(markup).toContain('text-rose-500/85')
|
||||
expect(markup).toContain('absolute -right-1 -top-1 size-[13px] text-amber-500')
|
||||
expect(markup).not.toContain('lucide-bell')
|
||||
expect(markup).not.toContain('text-amber-500')
|
||||
expect(markup).not.toContain('bg-emerald-500')
|
||||
})
|
||||
|
||||
it('overlays unread on the no-review branch icon in new card style', () => {
|
||||
it('keeps the branch icon visible for unread rows in new card style', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<WorktreeCardStatusSlot
|
||||
worktreeId="wt-1"
|
||||
|
|
@ -296,7 +353,8 @@ describe('WorktreeCardStatusSlot', () => {
|
|||
'group/unread relative flex cursor-pointer items-center justify-center rounded transition-all size-5'
|
||||
)
|
||||
expect(markup).toContain('lucide-git-branch')
|
||||
expect(markup).toContain('absolute -right-1 -top-1 size-[13px] text-amber-500')
|
||||
expect(markup).not.toContain('lucide-bell')
|
||||
expect(markup).not.toContain('text-amber-500')
|
||||
expect(markup).not.toContain('bg-emerald-500')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -27,7 +27,10 @@ const QUIET_REVIEW_REPLACEABLE_STATUSES = new Set<WorktreeStatus>(['active', 'do
|
|||
// Why: a missing review display can also mean provider state is unavailable,
|
||||
// so the passive label names the branch cue without claiming no review exists.
|
||||
const BRANCH_STATUS_LABEL = 'Branch'
|
||||
const branchStatusIconClassName = 'size-4 text-muted-foreground/70'
|
||||
// Why: branch-style SVGs are optically left-heavy; this keeps them aligned with
|
||||
// the centered activity dots in the shared status column.
|
||||
const compactReviewAndBranchStatusIconClassName = 'size-[13px] translate-x-px'
|
||||
const branchStatusIconClassName = `${compactReviewAndBranchStatusIconClassName} text-muted-foreground/70`
|
||||
|
||||
function getReviewStatusTooltip(review: WorktreeCardPrDisplay): string {
|
||||
const label = getReviewLabel(review)
|
||||
|
|
@ -84,14 +87,18 @@ export function WorktreeCardStatusSlot({
|
|||
: canShowBranchStatus
|
||||
? BRANCH_STATUS_LABEL
|
||||
: statusLabel
|
||||
const reviewStatusIconClassName = 'size-4'
|
||||
const reviewStatusIconClassName = compactReviewAndBranchStatusIconClassName
|
||||
const branchStatusIcon = <GitBranch className={branchStatusIconClassName} aria-hidden="true" />
|
||||
const passiveStatus =
|
||||
canShowReviewStatus && prDisplay ? (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span className={cn('inline-flex size-5 items-center justify-center p-0.5', className)}>
|
||||
<ReviewIcon review={prDisplay} className={reviewStatusIconClassName} />
|
||||
<ReviewIcon
|
||||
review={prDisplay}
|
||||
className={reviewStatusIconClassName}
|
||||
variant="generic"
|
||||
/>
|
||||
<span className="sr-only">{passiveStatusLabel}</span>
|
||||
</span>
|
||||
</TooltipTrigger>
|
||||
|
|
@ -135,7 +142,7 @@ export function WorktreeCardStatusSlot({
|
|||
|
||||
const actionLabel = isUnread ? 'Mark as read' : 'Mark as unread'
|
||||
const tooltip =
|
||||
showStatus && (!isUnread || (newCardStyle && (canShowBranchStatus || canShowReviewStatus)))
|
||||
showStatus && (!isUnread || newCardStyle)
|
||||
? `${passiveStatusLabel} · ${unreadTooltip}`
|
||||
: unreadTooltip
|
||||
|
||||
|
|
@ -157,36 +164,26 @@ export function WorktreeCardStatusSlot({
|
|||
)}
|
||||
aria-label={actionLabel}
|
||||
>
|
||||
{isUnread && showStatus && canShowReviewStatus && prDisplay ? (
|
||||
<>
|
||||
{newCardStyle ? (
|
||||
showStatus && canShowReviewStatus && prDisplay ? (
|
||||
<span className="inline-flex size-5 items-center justify-center p-0.5">
|
||||
<ReviewIcon review={prDisplay} className={reviewStatusIconClassName} />
|
||||
<ReviewIcon
|
||||
review={prDisplay}
|
||||
className={reviewStatusIconClassName}
|
||||
variant="generic"
|
||||
/>
|
||||
</span>
|
||||
<FilledBellIcon className="absolute -right-1 -top-1 size-[13px] text-amber-500 drop-shadow-sm" />
|
||||
</>
|
||||
) : isUnread && showStatus && canShowBranchStatus ? (
|
||||
<>
|
||||
) : showStatus && canShowBranchStatus ? (
|
||||
<span className="inline-flex size-5 items-center justify-center p-0.5">
|
||||
{branchStatusIcon}
|
||||
</span>
|
||||
<FilledBellIcon className="absolute -right-1 -top-1 size-[13px] text-amber-500 drop-shadow-sm" />
|
||||
</>
|
||||
) : showStatus ? (
|
||||
<StatusIndicator status={status} aria-hidden="true" />
|
||||
) : (
|
||||
<span className="sr-only">{actionLabel}</span>
|
||||
)
|
||||
) : isUnread ? (
|
||||
<FilledBellIcon className="size-[13px] text-amber-500 drop-shadow-sm" />
|
||||
) : showStatus && canShowReviewStatus && prDisplay ? (
|
||||
<>
|
||||
<span className="inline-flex size-5 items-center justify-center p-0.5 transition-opacity group-hover/unread:opacity-0 group-focus-within/unread:opacity-0">
|
||||
<ReviewIcon review={prDisplay} className={reviewStatusIconClassName} />
|
||||
</span>
|
||||
<Bell className="absolute size-3 text-muted-foreground/40 opacity-0 transition-opacity group-hover/unread:opacity-100 group-focus-within/unread:opacity-100" />
|
||||
</>
|
||||
) : showStatus && canShowBranchStatus ? (
|
||||
<>
|
||||
<span className="inline-flex size-5 items-center justify-center p-0.5 transition-opacity group-hover/unread:opacity-0 group-focus-within/unread:opacity-0">
|
||||
{branchStatusIcon}
|
||||
</span>
|
||||
<Bell className="absolute size-3 text-muted-foreground/40 opacity-0 transition-opacity group-hover/unread:opacity-100 group-focus-within/unread:opacity-100" />
|
||||
</>
|
||||
) : showStatus ? (
|
||||
<>
|
||||
<StatusIndicator
|
||||
|
|
|
|||
|
|
@ -46,7 +46,35 @@ describe('WorktreeTitleInlineRename', () => {
|
|||
expect(markup).not.toContain('cursor-text')
|
||||
expect(markup).not.toContain('title="Feature workspace"')
|
||||
expect(markup).toContain('tabindex="0"')
|
||||
expect(markup).toContain('font-semibold text-foreground')
|
||||
expect(markup).toContain('Unread:')
|
||||
expect(markup).toContain('Feature workspace')
|
||||
})
|
||||
|
||||
it('keeps read titles at the default foreground color unless requested', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<TooltipProvider>
|
||||
<WorktreeTitleInlineRename displayName="Feature workspace" onRename={vi.fn()} />
|
||||
</TooltipProvider>
|
||||
)
|
||||
|
||||
expect(markup).toContain('font-normal text-foreground')
|
||||
expect(markup).not.toContain('text-foreground/80')
|
||||
expect(markup).not.toContain('Unread:')
|
||||
})
|
||||
|
||||
it('dims read titles when requested by the experimental card style', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<TooltipProvider>
|
||||
<WorktreeTitleInlineRename
|
||||
displayName="Feature workspace"
|
||||
dimReadTitle
|
||||
onRename={vi.fn()}
|
||||
/>
|
||||
</TooltipProvider>
|
||||
)
|
||||
|
||||
expect(markup).toContain('font-normal text-foreground/80')
|
||||
expect(markup).not.toContain('Unread:')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ type WorktreeTitleInlineRenameProps = {
|
|||
displayName: string
|
||||
disabled?: boolean
|
||||
showUnreadEmphasis?: boolean
|
||||
dimReadTitle?: boolean
|
||||
className?: string
|
||||
editingClassName?: string
|
||||
inputClassName?: string
|
||||
|
|
@ -46,6 +47,7 @@ export function WorktreeTitleInlineRename({
|
|||
displayName,
|
||||
disabled = false,
|
||||
showUnreadEmphasis = false,
|
||||
dimReadTitle = false,
|
||||
className,
|
||||
editingClassName,
|
||||
inputClassName,
|
||||
|
|
@ -269,13 +271,19 @@ export function WorktreeTitleInlineRename({
|
|||
)
|
||||
}
|
||||
|
||||
const titleEmphasisClassName = showUnreadEmphasis
|
||||
? 'font-semibold text-foreground'
|
||||
: dimReadTitle
|
||||
? 'font-normal text-foreground/80'
|
||||
: 'font-normal text-foreground'
|
||||
|
||||
const title = (
|
||||
<span
|
||||
key={`title:${titleElementKey}`}
|
||||
ref={handleRootRef}
|
||||
className={cn(
|
||||
'block min-w-0 truncate leading-tight text-foreground focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-worktree-sidebar-ring',
|
||||
showUnreadEmphasis ? 'font-semibold' : 'font-normal',
|
||||
'block min-w-0 truncate leading-tight focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-worktree-sidebar-ring',
|
||||
titleEmphasisClassName,
|
||||
className
|
||||
)}
|
||||
data-worktree-title-inline-rename=""
|
||||
|
|
|
|||
|
|
@ -0,0 +1,29 @@
|
|||
import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { ReviewIcon } from './worktree-review-helpers'
|
||||
import type { WorktreeCardPrDisplay } from './worktree-card-pr-display'
|
||||
|
||||
const gitlabReview: WorktreeCardPrDisplay = {
|
||||
provider: 'gitlab',
|
||||
number: 456,
|
||||
title: 'Review me',
|
||||
state: 'open',
|
||||
status: 'pending'
|
||||
}
|
||||
|
||||
describe('ReviewIcon', () => {
|
||||
it('uses the provider-specific GitLab MR icon by default', () => {
|
||||
const markup = renderToStaticMarkup(<ReviewIcon review={gitlabReview} className="size-3" />)
|
||||
|
||||
expect(markup).toContain('lucide-git-merge')
|
||||
})
|
||||
|
||||
it('can use the generic review icon for compact lanes', () => {
|
||||
const markup = renderToStaticMarkup(
|
||||
<ReviewIcon review={gitlabReview} className="size-3" variant="generic" />
|
||||
)
|
||||
|
||||
expect(markup).toContain('viewBox="0 0 16 16"')
|
||||
expect(markup).not.toContain('lucide-git-merge')
|
||||
})
|
||||
})
|
||||
|
|
@ -25,12 +25,14 @@ export function getProviderName(review: WorktreeCardPrDisplay): string {
|
|||
|
||||
export function ReviewIcon({
|
||||
review,
|
||||
className
|
||||
className,
|
||||
variant = 'provider'
|
||||
}: {
|
||||
review: WorktreeCardPrDisplay
|
||||
className?: string
|
||||
variant?: 'provider' | 'generic'
|
||||
}): React.JSX.Element {
|
||||
const Icon = review.provider === 'gitlab' ? GitMerge : PullRequestIcon
|
||||
const Icon = variant === 'provider' && review.provider === 'gitlab' ? GitMerge : PullRequestIcon
|
||||
const checkTone =
|
||||
review.state !== 'merged' && review.status === 'failure'
|
||||
? 'text-rose-500/85'
|
||||
|
|
|
|||
Loading…
Reference in New Issue