diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 422b765f082..2566b22b2df 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -102,6 +102,7 @@ import { getRenderRowKey, getStickyHeaderIndexes, getVirtualRowTransform, + HOST_STICKY_GROUP_TOP_PX, pruneStaleVirtualRowElementCache, shouldUseHeaderTopSpacing, type RenderRow @@ -4108,9 +4109,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp if (row.type === 'header') { const isActiveStickyHeader = activeStickyHeaderIndexRef.current === vItem.index - // Why: when a host card is pinned, the group tier pins flush beneath it, not at the viewport top. - const stickyTopClass = - activeStickyHostIndexRef.current !== null ? 'top-[35px]' : '-top-px' + // Why: when a host card is pinned, the group tier pins flush beneath it (HOST_STICKY_GROUP_TOP_PX), not at the viewport top. + const stickyUnderHost = activeStickyHostIndexRef.current !== null const hasHeaderTopSpacing = shouldUseHeaderTopSpacing({ rows: renderRows, index: vItem.index, @@ -4224,12 +4224,17 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp // Why: drop the inter-group spacer once the header pins so it sits flush at top (see getActiveStickyHeaderIndexForScroll). hasHeaderTopSpacing && !isActiveStickyHeader && 'pt-1', isActiveStickyHeader - ? cn('sticky z-20 bg-worktree-sidebar', stickyTopClass) + ? cn( + 'sticky z-20 bg-worktree-sidebar', + stickyUnderHost ? undefined : '-top-px' + ) : 'absolute top-0' )} style={ isActiveStickyHeader - ? undefined + ? stickyUnderHost + ? { top: HOST_STICKY_GROUP_TOP_PX } + : undefined : { transform: getVirtualRowTransform(vItem.start) } } > diff --git a/src/renderer/src/components/sidebar/host-section-rows.test.ts b/src/renderer/src/components/sidebar/host-section-rows.test.ts index 702356e26a6..46924f0a9cb 100644 --- a/src/renderer/src/components/sidebar/host-section-rows.test.ts +++ b/src/renderer/src/components/sidebar/host-section-rows.test.ts @@ -364,6 +364,16 @@ describe('addHostSectionRows', () => { expect.objectContaining({ count: 1, hostId: 'ssh:ssh-1' }) ]) ) + // Why: unscoped "all" headers are cloned per host; hostId keeps virtualizer keys unique (#12300). + expect( + sectioned + .filter( + (row): row is Extract => + row.type === 'header' && row.key === 'all' + ) + .map((row) => row.hostId) + .sort() + ).toEqual(['local', 'ssh:ssh-1']) }) it('keeps collapsed pinned rows attributable to their owning hosts', () => { diff --git a/src/renderer/src/components/sidebar/host-section-rows.ts b/src/renderer/src/components/sidebar/host-section-rows.ts index 7ca084d7520..e53789f7d4d 100644 --- a/src/renderer/src/components/sidebar/host-section-rows.ts +++ b/src/renderer/src/components/sidebar/host-section-rows.ts @@ -142,7 +142,9 @@ function localizePendingRowsForHost( const localized: Extract[] = [] for (const row of rows) { if (!row.hostWorktreeCounts) { - localized.push(row) + // Why: project-group (and other unscoped) headers are cloned per host section; + // stamp hostId so virtualizer keys stay unique across hosts (#12300). + localized.push({ ...row, hostId }) continue } const count = row.hostWorktreeCounts.get(hostId) diff --git a/src/renderer/src/components/sidebar/worktree-list-host-collapse-12300.test.ts b/src/renderer/src/components/sidebar/worktree-list-host-collapse-12300.test.ts new file mode 100644 index 00000000000..a75def14ac7 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list-host-collapse-12300.test.ts @@ -0,0 +1,219 @@ +import { describe, expect, it } from 'vitest' +import type { Repo, Worktree } from '../../../../shared/types' +import { addHostSectionRows, type HostSectionRow } from './host-section-rows' +import type { Row } from './worktree-list-groups' +import { + estimateRenderRowSize, + getRenderRowKey, + HOST_STICKY_PINNED_HEIGHT, + type RenderRow +} from './worktree-list-virtual-rows' + +function repo(id: string, connectionId?: string | null): Repo { + return { + id, + path: `/${id}`, + displayName: id, + badgeColor: '#000000', + addedAt: 0, + connectionId + } +} + +function worktree(id: string, repoId: string): Worktree { + return { + id, + repoId, + path: `/${repoId}/${id}`, + branch: `refs/heads/${id}`, + head: 'abc123', + isBare: false, + isMainWorktree: false, + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + comment: '', + linkedGitLabMR: null, + linkedGitLabIssue: null, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0, + displayName: id + } +} + +function projectGroupHeader(groupId: string, label: string): Extract { + return { + type: 'header', + key: `project-group:${groupId}`, + label, + count: 1, + tone: 'text-foreground', + projectGroup: { + id: groupId, + name: label, + parentPath: null, + parentGroupId: null, + createdFrom: 'manual', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + }, + projectGroupDepth: 0 + } +} + +function item(wt: Worktree, r: Repo): Extract { + return { + type: 'item', + key: wt.id, + rowKey: `all:${wt.id}`, + sectionKey: 'all', + worktree: wt, + repo: r, + depth: 0, + groupDepth: 0, + lineageTrail: [], + isLastLineageChild: true, + lineageChildCount: 0 + } as Extract +} + +const localAi = repo('local-ai') +const localOther = repo('local-other') +const homeAi = repo('home-ai', 'home') +const mac3Ai = repo('mac3-ai', 'mac3') +const mac3Other = repo('mac3-other', 'mac3') + +const hostOptions = [ + { + id: 'local' as const, + kind: 'local' as const, + label: 'Local Mac', + detail: 'This computer', + health: 'local' as const + }, + { + id: 'ssh:home' as const, + kind: 'ssh' as const, + label: 'Home', + detail: 'Orca server', + health: 'available' as const + }, + { + id: 'ssh:mac3' as const, + kind: 'ssh' as const, + label: 'mac3', + detail: 'Orca server', + health: 'available' as const + } +] + +// Multi-host Projects filter: same project-group headers appear under each host. +const baseRows: Row[] = [ + projectGroupHeader('ai', 'ai'), + item(worktree('wt-local-ai', 'local-ai'), localAi), + item(worktree('wt-home-ai', 'home-ai'), homeAi), + item(worktree('wt-mac3-ai', 'mac3-ai'), mac3Ai), + projectGroupHeader('other', 'other'), + item(worktree('wt-local-other', 'local-other'), localOther), + item(worktree('wt-mac3-other', 'mac3-other'), mac3Other) +] + +function section(collapsedHostKeys?: ReadonlySet): HostSectionRow[] { + return addHostSectionRows({ + rows: baseRows, + hostOptions, + workspaceHostScope: 'all', + visibleWorkspaceHostIds: ['local', 'ssh:home', 'ssh:mac3'], + defaultHostId: 'local', + collapsedHostKeys, + preferProjectGrouping: true + }) +} + +function assertUniqueRenderKeys(rows: readonly HostSectionRow[]): void { + const keys = rows.map((row) => getRenderRowKey(row)) + expect(new Set(keys).size).toBe(keys.length) +} + +describe('host collapse multi-host style uniqueness (#12300)', () => { + it('stamps hostId so project-group headers stay unique across host sections', () => { + const expanded = section() + assertUniqueRenderKeys(expanded) + + const projectHeaders = expanded.filter( + (row): row is Extract => + row.type === 'header' && row.key.startsWith('project-group:') + ) + expect(projectHeaders.length).toBeGreaterThanOrEqual(2) + expect(projectHeaders.every((row) => row.hostId != null)).toBe(true) + expect(projectHeaders.map((row) => getRenderRowKey(row))).toEqual( + expect.arrayContaining([ + 'hdr:local:project-group:ai', + 'hdr:ssh:home:project-group:ai', + 'hdr:ssh:mac3:project-group:ai', + 'hdr:local:project-group:other', + 'hdr:ssh:mac3:project-group:other' + ]) + ) + }) + + it('keeps unique keys after collapsing and re-expanding a host section', () => { + const collapsedHome = section(new Set(['host:ssh:home'])) + assertUniqueRenderKeys(collapsedHome) + expect( + collapsedHome.find((row) => row.type === 'host-header' && row.hostId === 'ssh:home') + ).toMatchObject({ collapsed: true, count: 1 }) + // Collapsed host omits its project rows but keeps the host card. + expect( + collapsedHome.some( + (row) => + row.type === 'header' && row.key === 'project-group:ai' && row.hostId === 'ssh:home' + ) + ).toBe(false) + + const reexpanded = section() + assertUniqueRenderKeys(reexpanded) + expect( + reexpanded.some( + (row) => + row.type === 'header' && row.key === 'project-group:ai' && row.hostId === 'ssh:home' + ) + ).toBe(true) + }) + + it('sizes host headers with inner padding so sticky geometry matches paint', () => { + const rows = section() as RenderRow[] + const firstHost = rows.findIndex((row) => row.type === 'host-header') + const secondHost = rows.findIndex( + (row, index) => index > firstHost && row.type === 'host-header' + ) + expect(estimateRenderRowSize(rows, firstHost, firstHost, null)).toBe(HOST_STICKY_PINNED_HEIGHT) + // Secondary host keeps the inter-group top margin on top of the inner pt-1. + expect(estimateRenderRowSize(rows, secondHost, firstHost, null)).toBe( + HOST_STICKY_PINNED_HEIGHT + 4 + ) + }) + + it('does not section when a single host is filtered (control)', () => { + const localOnly = addHostSectionRows({ + rows: baseRows.filter( + (row) => + row.type === 'header' || + (row.type === 'item' && row.repo != null && !row.repo.connectionId) + ), + hostOptions, + workspaceHostScope: 'all', + visibleWorkspaceHostIds: ['local'], + defaultHostId: 'local', + preferProjectGrouping: true + }) + expect(localOnly.some((row) => row.type === 'host-header')).toBe(false) + assertUniqueRenderKeys(localOnly) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-list-virtual-rows.test.ts b/src/renderer/src/components/sidebar/worktree-list-virtual-rows.test.ts index 67b6147765e..07216831f62 100644 --- a/src/renderer/src/components/sidebar/worktree-list-virtual-rows.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-virtual-rows.test.ts @@ -2,8 +2,10 @@ import { describe, expect, it } from 'vitest' import type { VirtualItem } from '@tanstack/react-virtual' import type { ExecutionHostId } from '../../../../shared/execution-host' import { + HOST_STICKY_GROUP_TOP_PX, HOST_STICKY_PINNED_HEIGHT, buildLineageRowRekeyMap, + estimateRenderRowSize, extractWorktreeVirtualRowIndexes, getActiveStickyIndexesForScroll, getRenderRowKey, @@ -193,6 +195,111 @@ describe('getActiveStickyIndexesForScroll', () => { expect(result.hostIndex).toBe(0) expect(result.groupIndex).toBe(1) }) + + it('estimates host headers with HostSectionHeader inner padding so sticky geometry matches paint (#12300)', () => { + // Why: HostSectionHeader is always pt-1 + h-8. Under-estimating the first host + // by 4px pulled later group starts forward and made sticky tiers collide. + const multiGroupRows: RenderRow[] = [ + hostRow('ssh:a'), + groupRow('a1'), + itemStub('wt-1'), + hostRow('ssh:b'), + groupRow('b1'), + itemStub('wt-2') + ] + expect(estimateRenderRowSize(multiGroupRows, 0, 0, null)).toBe(HOST_STICKY_PINNED_HEIGHT) + expect(estimateRenderRowSize(multiGroupRows, 3, 0, null)).toBe(HOST_STICKY_PINNED_HEIGHT + 4) + expect(HOST_STICKY_GROUP_TOP_PX).toBe(HOST_STICKY_PINNED_HEIGHT - 1) + + const multiSticky = getStickyHeaderIndexes(multiGroupRows) + const gap = 6 + const hostSize = estimateRenderRowSize(multiGroupRows, 0, 0, null) + const groupStart = hostSize + gap + const virtualItems = [ + virtualItem(0, 0), + virtualItem(1, groupStart), + virtualItem(2, groupStart + 100), + virtualItem(3, groupStart + 200), + virtualItem(4, groupStart + 300), + virtualItem(5, groupStart + 400) + ] + + // Cold first paint: host pin only; first group stays in flow. + const cold = getActiveStickyIndexesForScroll({ + rows: multiGroupRows, + rangeStartIndex: 0, + scrollOffset: 0, + stickyHeaderIndexes: multiSticky, + virtualItems + }) + expect(cold).toEqual({ hostIndex: 0, groupIndex: null }) + + // Group must not pin until scrollOffset + HOST_STICKY_PINNED_HEIGHT reaches start. + const earlyScroll = groupStart - HOST_STICKY_PINNED_HEIGHT - 1 + const early = getActiveStickyIndexesForScroll({ + rows: multiGroupRows, + rangeStartIndex: 1, + scrollOffset: earlyScroll, + stickyHeaderIndexes: multiSticky, + virtualItems + }) + expect(early.groupIndex).toBeNull() + + const onSlot = getActiveStickyIndexesForScroll({ + rows: multiGroupRows, + rangeStartIndex: 1, + scrollOffset: groupStart - HOST_STICKY_PINNED_HEIGHT, + stickyHeaderIndexes: multiSticky, + virtualItems + }) + expect(onSlot).toEqual({ hostIndex: 0, groupIndex: 1 }) + }) + + it('keeps single-tier cold first render free of double group pins', () => { + const flatRows: RenderRow[] = [ + groupRow('project-group:infra'), + itemStub('wt-1'), + groupRow('project-group:projects'), + itemStub('wt-2') + ] + const flatSticky = getStickyHeaderIndexes(flatRows) + const sizes = flatRows.map((_, index) => estimateRenderRowSize(flatRows, index, 0, null)) + let start = 0 + const flatItems = sizes.map((size, index) => { + const item = virtualItem(index, start) + start += size + 6 + return item + }) + + const cold = getActiveStickyIndexesForScroll({ + rows: flatRows, + rangeStartIndex: 0, + scrollOffset: 0, + stickyHeaderIndexes: flatSticky, + virtualItems: flatItems + }) + expect(cold).toEqual({ hostIndex: null, groupIndex: 0 }) + + // Second group must not share the pin until it reaches the top. + const mid = getActiveStickyIndexesForScroll({ + rows: flatRows, + rangeStartIndex: 1, + scrollOffset: flatItems[2].start - 1, + stickyHeaderIndexes: flatSticky, + virtualItems: flatItems + }) + expect(mid.groupIndex).toBe(0) + }) + + it('scopes host-stamped project-group header keys so multi-host sections stay unique', () => { + expect(getRenderRowKey(groupRow('project-group:infra', 'local'))).toBe( + 'hdr:local:project-group:infra' + ) + expect(getRenderRowKey(groupRow('project-group:infra', 'ssh:builder'))).toBe( + 'hdr:ssh:builder:project-group:infra' + ) + expect(getRenderRowKey(groupRow('project-group:infra'))).toBe('hdr:project-group:infra') + }) }) describe('extractWorktreeVirtualRowIndexes', () => { diff --git a/src/renderer/src/components/sidebar/worktree-list-virtual-rows.ts b/src/renderer/src/components/sidebar/worktree-list-virtual-rows.ts index 69b8c71c010..8b9599766ee 100644 --- a/src/renderer/src/components/sidebar/worktree-list-virtual-rows.ts +++ b/src/renderer/src/components/sidebar/worktree-list-virtual-rows.ts @@ -5,6 +5,9 @@ import { getLineageGroupKey, PINNED_GROUP_KEY } from './worktree-list-groups' export const GROUP_HEADER_ROW_HEIGHT = 28 export const HOST_HEADER_ROW_HEIGHT = 32 +// Why: HostSectionHeader always wraps the h-8 card in pt-1; sticky drops only the +// inter-group virtual-row spacer, not this inner padding (#12300). +export const HOST_HEADER_INNER_TOP_PADDING = 4 const SECONDARY_GROUP_HEADER_TOP_MARGIN = 4 const IMPORTED_WORKTREES_LINE_ROW_HEIGHT = 36 const PENDING_CREATION_ROW_HEIGHT = 56 @@ -93,6 +96,7 @@ export function estimateRenderRowSize( if (row?.type === 'host-header') { return ( HOST_HEADER_ROW_HEIGHT + + HOST_HEADER_INNER_TOP_PADDING + (shouldUseHeaderTopSpacing({ rows, index, @@ -173,10 +177,12 @@ export function getStickyHeaderIndexes(rows: readonly RenderRow[]): number[] { return indexes } -// Why: the pinned host card is h-8 (32px) inside a pt-1 (4px) wrapper; the -// group tier pins one pixel up to sit flush beneath it. Keep in sync with -// HostSectionHeader's layout. -export const HOST_STICKY_PINNED_HEIGHT = 36 +// Why: sticky host keeps HostSectionHeader's inner pt-1 + h-8 card (36px) and +// only drops the inter-group virtual-row spacer. Group tier pins flush under that. +export const HOST_STICKY_PINNED_HEIGHT = HOST_HEADER_ROW_HEIGHT + HOST_HEADER_INNER_TOP_PADDING + +/** One pixel up under the pinned host so the group sticky sits flush. */ +export const HOST_STICKY_GROUP_TOP_PX = HOST_STICKY_PINNED_HEIGHT - 1 export type ActiveStickyIndexes = { /** Pinned host card (tier 1), or null outside host sections. */