xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: address review findings (#5889)

This commit is contained in:
Jinjing 2026-06-20 01:15:58 -07:00 committed by GitHub
parent 04004bf2cf
commit e2c8a66ee0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 64 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx
View File

@ -203,8 +203,9 @@ describe('WorktreeCard linked PR display', () => {
<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).not.toContain('aria-label="Mark as read"')
expect(unreadMarkup).toContain('PR checks: Failed')
expect(unreadMarkup).not.toContain('Mark read')
expect(unreadMarkup).toContain('size-[13px] translate-x-px')
expect(unreadMarkup).not.toContain('lucide-bell')
expect(unreadMarkup).not.toContain('text-amber-500')

7
src/renderer/src/components/sidebar/WorktreeCard.tsx
View File

@ -1050,9 +1050,9 @@ const WorktreeCard = React.memo(function WorktreeCard({
const showConflictOperationBadge =
!!conflictOperation && conflictOperation !== 'unknown' && conflictOperation !== 'rebase'
const hasMetadataBadge = showConflictOperationBadge || hasAutomationMetadata
const showUnreadQuickAction = !affiliateListMode && showStatus
// Why: the slot owns the tiny unread/status lane; legacy keeps the bell,
// while the experimental card keeps the status glyph visible.
const showUnreadQuickAction = !affiliateListMode && showStatus && !newCardStyle
// Why: the slot owns the tiny unread/status lane; legacy keeps the bell
// toggle, while the experimental card keeps the status glyph passive.
const showCombinedStatusSlot = showStatus
const showTitleRowPrimary = compactCards && worktree.isMainWorktree && !isFolder
const showMetaRowDetails = !newCardStyle && !compactCards && (hasDetails || hasPorts)
@ -1763,6 +1763,7 @@ const WorktreeCard = React.memo(function WorktreeCard({
) : (
<WorktreeContextMenu
worktree={worktree}
newCardStyle={newCardStyle}
selectedWorktrees={selectedWorktrees}
onContextMenuSelect={handleContextMenuSelect}
>

30
src/renderer/src/components/sidebar/WorktreeCardStatusSlot.test.tsx
View File

@ -58,7 +58,7 @@ describe('WorktreeCardStatusSlot', () => {
expect(markup).toContain('text-amber-500')
})
it('keeps the status dot visible for unread rows when new card style is on', () => {
it('keeps the status dot passive for unread rows when new card style is on', () => {
const markup = renderToStaticMarkup(
<WorktreeCardStatusSlot
worktreeId="wt-1"
@ -73,8 +73,9 @@ describe('WorktreeCardStatusSlot', () => {
/>
)
expect(markup).toContain('aria-label="Mark as read"')
expect(markup).toContain('Active · Mark as read')
expect(markup).not.toContain('aria-label="Mark as read"')
expect(markup).not.toContain('Mark as read')
expect(markup).toContain('Active')
expect(markup).toContain('bg-emerald-500')
expect(markup).not.toContain('lucide-bell')
expect(markup).not.toContain('text-amber-500')
@ -307,7 +308,7 @@ describe('WorktreeCardStatusSlot', () => {
expect(markup).toContain('text-amber-500')
})
it('keeps PR status visible for unread rows when new card style is on', () => {
it('keeps PR status passive for unread rows when new card style is on', () => {
const markup = renderToStaticMarkup(
<WorktreeCardStatusSlot
worktreeId="wt-1"
@ -322,19 +323,18 @@ describe('WorktreeCardStatusSlot', () => {
/>
)
expect(markup).toContain('aria-label="Mark as read"')
expect(markup).toContain('Mark as read')
expect(markup).toContain('PR checks: Failed · Mark as read')
expect(markup).toContain(
'group/unread relative flex cursor-pointer items-center justify-center rounded transition-all size-5'
)
expect(markup).not.toContain('aria-label="Mark as read"')
expect(markup).not.toContain('Mark as read')
expect(markup).toContain('PR checks: Failed')
expect(markup).not.toContain('group/unread')
expect(markup).not.toContain('cursor-pointer')
expect(markup).toContain('text-rose-500/85')
expect(markup).not.toContain('lucide-bell')
expect(markup).not.toContain('text-amber-500')
expect(markup).not.toContain('bg-emerald-500')
})
it('keeps the branch icon visible for unread rows in new card style', () => {
it('keeps the branch icon passive for unread rows in new card style', () => {
const markup = renderToStaticMarkup(
<WorktreeCardStatusSlot
worktreeId="wt-1"
@ -348,10 +348,10 @@ describe('WorktreeCardStatusSlot', () => {
/>
)
expect(markup).toContain('Branch · Mark as read')
expect(markup).toContain(
'group/unread relative flex cursor-pointer items-center justify-center rounded transition-all size-5'
)
expect(markup).toContain('Branch')
expect(markup).not.toContain('Mark as read')
expect(markup).not.toContain('group/unread')
expect(markup).not.toContain('cursor-pointer')
expect(markup).toContain('lucide-git-branch')
expect(markup).not.toContain('lucide-bell')
expect(markup).not.toContain('text-amber-500')

10
src/renderer/src/components/sidebar/WorktreeCardStatusSlot.tsx
View File

@ -132,19 +132,19 @@ export function WorktreeCardStatusSlot({
</>
)
if (!showStatus && !showUnreadAction) {
const unreadActionEnabled = showUnreadAction && !newCardStyle
if (!showStatus && !unreadActionEnabled) {
return null
}
if (!showUnreadAction) {
if (!unreadActionEnabled) {
return passiveStatus
}
const actionLabel = isUnread ? 'Mark as read' : 'Mark as unread'
const tooltip =
showStatus && (!isUnread || newCardStyle)
? `${passiveStatusLabel} · ${unreadTooltip}`
: unreadTooltip
showStatus && !isUnread ? `${passiveStatusLabel} · ${unreadTooltip}` : unreadTooltip
return (
<>

11
src/renderer/src/components/sidebar/WorktreeContextMenu.test.ts
View File

@ -7,6 +7,7 @@ import {
shouldRemoveProjectFromContextMenu,
shouldSuppressContextMenuFollowUpClick,
shouldContinueDeleteSiblingPositionRestore,
shouldShowReadToggleContextMenuItem,
getWorktreeParentPickerAnchor,
getWorktreeParentPickerLabel,
isWorktreeParentPickerDisabled
@ -98,6 +99,16 @@ describe('shouldSuppressContextMenuFollowUpClick', () => {
})
})
describe('shouldShowReadToggleContextMenuItem', () => {
it('keeps the read toggle in legacy card menus', () => {
expect(shouldShowReadToggleContextMenuItem({ newCardStyle: false })).toBe(true)
})
it('hides the read toggle in experimental card menus', () => {
expect(shouldShowReadToggleContextMenuItem({ newCardStyle: true })).toBe(false)
})
})
describe('shouldContinueDeleteSiblingPositionRestore', () => {
it('stops once the delete row position has settled even when the row remains mounted', () => {
expect(

39
src/renderer/src/components/sidebar/WorktreeContextMenu.tsx
View File

@ -57,6 +57,7 @@ type Props = {
worktree: Worktree
children: React.ReactNode
contentClassName?: string
newCardStyle?: boolean
selectedWorktrees?: readonly Worktree[]
onContextMenuSelect?: (event: React.MouseEvent<HTMLElement>) => readonly Worktree[]
onOpenChange?: (open: boolean) => void
@ -123,6 +124,10 @@ function isWorktreeParentPickerDisabled(args: {
return args.isDeleting || args.eligibleParentCount === 0
}
function shouldShowReadToggleContextMenuItem(args: { newCardStyle: boolean }): boolean {
return !args.newCardStyle
}
function getWorktreeParentPickerAnchor(
scope: HTMLElement | null,
worktreeId: string
@ -243,6 +248,7 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
worktree,
children,
contentClassName,
newCardStyle = false,
selectedWorktrees,
onContextMenuSelect,
onOpenChange
@ -344,6 +350,7 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
(item) =>
worktreeLineageById[item.id] || workspaceLineageByChildKey[worktreeWorkspaceKey(item.id)]
)
const showReadToggle = shouldShowReadToggleContextMenuItem({ newCardStyle })
const eligibleParentCount = useMemo(
() =>
getEligibleWorktreeParents({
@ -664,19 +671,24 @@ const WorktreeContextMenu = React.memo(function WorktreeContextMenu({
? translate('auto.components.sidebar.WorktreeContextMenu.697d0f6e1b', 'Unpin')
: translate('auto.components.sidebar.WorktreeContextMenu.3baa7d6507', 'Pin')}
</DropdownMenuItem>
<DropdownMenuItem onSelect={handleToggleRead} disabled={isDeleting}>
{worktree.isUnread ? (
<BellOff className="size-3.5" />
) : (
<Bell className="size-3.5" />
)}
{worktree.isUnread
? translate('auto.components.sidebar.WorktreeContextMenu.8dacff1fe0', 'Mark Read')
: translate(
'auto.components.sidebar.WorktreeContextMenu.f50603c6b2',
'Mark Unread'
)}
</DropdownMenuItem>
{showReadToggle && (
<DropdownMenuItem onSelect={handleToggleRead} disabled={isDeleting}>
{worktree.isUnread ? (
<BellOff className="size-3.5" />
) : (
<Bell className="size-3.5" />
)}
{worktree.isUnread
? translate(
'auto.components.sidebar.WorktreeContextMenu.8dacff1fe0',
'Mark Read'
)
: translate(
'auto.components.sidebar.WorktreeContextMenu.f50603c6b2',
'Mark Unread'
)}
</DropdownMenuItem>
)}
{repo ? (
<>
<DropdownMenuSeparator />
@ -907,6 +919,7 @@ export {
getWorktreeParentPickerLabel,
isWorktreeParentPickerDisabled,
shouldRemoveProjectFromContextMenu,
shouldShowReadToggleContextMenuItem,
shouldUseNativeContextMenu,
shouldSuppressContextMenuFollowUpClick,
shouldIgnoreNestedWorktreeContextMenuScope