From 697c963986fc086bde0ed1ca374bc9361dabcb39 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 6 Apr 2026 16:35:48 -0700 Subject: [PATCH] fix: dynamic estimateSize for worktree cards to prevent overlap (#346) Co-authored-by: Claude Opus 4.6 --- .../src/components/sidebar/WorktreeCard.tsx | 5 +- .../src/components/sidebar/WorktreeList.tsx | 27 ++++- .../sidebar/worktree-list-estimate.test.ts | 112 ++++++++++++++++++ .../sidebar/worktree-list-estimate.ts | 39 ++++++ 4 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 src/renderer/src/components/sidebar/worktree-list-estimate.test.ts create mode 100644 src/renderer/src/components/sidebar/worktree-list-estimate.ts diff --git a/src/renderer/src/components/sidebar/WorktreeCard.tsx b/src/renderer/src/components/sidebar/WorktreeCard.tsx index 9577d5d9a42..569ae6037fb 100644 --- a/src/renderer/src/components/sidebar/WorktreeCard.tsx +++ b/src/renderer/src/components/sidebar/WorktreeCard.tsx @@ -357,7 +357,10 @@ const WorktreeCard = React.memo(function WorktreeCard({ )} - {/* Meta section: Issue / PR Links / Comment */} + {/* Meta section: Issue / PR Links / Comment + ⚠ Layout coupling: the padding (py-0.5, mt-0.5), gap-[3px], and + line heights here are used to derive the pixel constants in + WorktreeList's estimateSize. Update both if changing spacing. */} {((cardProps.includes('issue') && issue) || (cardProps.includes('pr') && pr) || (cardProps.includes('comment') && worktree.comment)) && ( diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index a820b0d626f..a56a9e7a4e2 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -9,6 +9,7 @@ import { cn } from '@/lib/utils' import type { Worktree, Repo } from '../../../../shared/types' import { buildWorktreeComparator } from './smart-sort' import { matchesSearch, type Row, buildRows, getGroupKeyForWorktree } from './worktree-list-groups' +import { estimateRowHeight } from './worktree-list-estimate' const WorktreeList = React.memo(function WorktreeList() { // ── Granular selectors (each is a primitive or shallow-stable ref) ── @@ -29,10 +30,14 @@ const WorktreeList = React.memo(function WorktreeList() { const needsTabs = showActiveOnly || sortBy === 'recent' const tabsByWorktree = useAppStore((s) => (needsTabs ? s.tabsByWorktree : null)) - // PR cache is needed for PR-status grouping and for recent sorting, which - // incorporates whether the current branch has a live PR attached. + const cardProps = useAppStore((s) => s.worktreeCardProperties) + + // PR cache is needed for PR-status grouping, recent sorting, search, and + // estimateSize when the PR card property is visible. const prCache = useAppStore((s) => - groupBy === 'pr-status' || sortBy === 'recent' || searchQuery ? s.prCache : null + groupBy === 'pr-status' || sortBy === 'recent' || searchQuery || cardProps.includes('pr') + ? s.prCache + : null ) // Subscribe to issue cache only during active search to avoid unnecessary re-renders. const issueCache = useAppStore((s) => (searchQuery ? s.issueCache : null)) @@ -191,7 +196,9 @@ const WorktreeList = React.memo(function WorktreeList() { const virtualizer = useVirtualizer({ count: rows.length, getScrollElement: () => scrollRef.current, - estimateSize: (index) => (rows[index].type === 'header' ? 42 : 56 + 4), + // Dynamic height estimate — pixel constants coupled to WorktreeCard's + // Tailwind classes (see coupling comment in WorktreeCard meta section). + estimateSize: (index) => estimateRowHeight(rows[index], cardProps, repoMap, prCache), overscan: 10, getItemKey: (index) => { const row = rows[index] @@ -199,6 +206,18 @@ const WorktreeList = React.memo(function WorktreeList() { } }) + // Invalidate cached sizes when async PR data arrives or card props change, + // so the virtualizer re-measures and eliminates overlap / scroll jumps. + useEffect(() => { + if (!prCache) { + return + } + virtualizer.measure() + }, [prCache, virtualizer]) + useEffect(() => { + virtualizer.measure() + }, [cardProps, virtualizer]) + React.useEffect(() => { if (!pendingRevealWorktreeId) { return diff --git a/src/renderer/src/components/sidebar/worktree-list-estimate.test.ts b/src/renderer/src/components/sidebar/worktree-list-estimate.test.ts new file mode 100644 index 00000000000..2bc06b36635 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list-estimate.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from 'vitest' +import { estimateRowHeight } from './worktree-list-estimate' +import type { Repo, Worktree } from '../../../../shared/types' +import type { Row } from './worktree-list-groups' + +const repo: Repo = { + id: 'repo-1', + path: '/tmp/orca', + displayName: 'orca', + badgeColor: '#000000', + addedAt: 0 +} + +const worktree: Worktree = { + id: 'wt-1', + repoId: repo.id, + path: '/tmp/orca-feature', + branch: 'refs/heads/feature/cool', + head: 'abc123', + isBare: false, + isMainWorktree: false, + linkedIssue: null, + linkedPR: null, + isArchived: false, + comment: '', + isUnread: false, + displayName: 'feature/cool', + sortOrder: 0, + lastActivityAt: 0 +} + +const repoMap = new Map([[repo.id, repo]]) + +function itemRow(wt: Worktree): Row { + return { type: 'item', worktree: wt, repo } +} + +describe('estimateRowHeight', () => { + it('returns 42 for header rows', () => { + const header: Row = { + type: 'header', + key: 'test', + label: 'Test', + count: 1, + icon: () => null, + tone: '' + } + expect(estimateRowHeight(header, [], repoMap, null)).toBe(42) + }) + + it('returns base height (56) for items with no metadata', () => { + expect(estimateRowHeight(itemRow(worktree), [], repoMap, null)).toBe(56) + }) + + it('adds 22px for issue row when linkedIssue is set', () => { + const wt = { ...worktree, linkedIssue: 42 } + const base = estimateRowHeight(itemRow(worktree), ['issue'], repoMap, null) + const withIssue = estimateRowHeight(itemRow(wt), ['issue'], repoMap, null) + expect(withIssue - base).toBe(24) // 22px line + 2px mt-0.5 + }) + + it('does not add issue height when cardProps excludes issue', () => { + const wt = { ...worktree, linkedIssue: 42 } + expect(estimateRowHeight(itemRow(wt), [], repoMap, null)).toBe(56) + }) + + it('adds 22px for PR row when prCache has data', () => { + const prCache = { + '/tmp/orca::feature/cool': { data: { number: 1 } } + } + const base = estimateRowHeight(itemRow(worktree), ['pr'], repoMap, null) + const withPR = estimateRowHeight(itemRow(worktree), ['pr'], repoMap, prCache) + expect(withPR - base).toBe(24) // 22px line + 2px mt-0.5 + }) + + it('does not add PR height when prCache is null', () => { + expect(estimateRowHeight(itemRow(worktree), ['pr'], repoMap, null)).toBe(56) + }) + + it('does not add PR height when prCache entry has no data', () => { + const prCache = { + '/tmp/orca::feature/cool': { data: null } + } + expect(estimateRowHeight(itemRow(worktree), ['pr'], repoMap, prCache)).toBe(56) + }) + + it('adds 22px for comment row', () => { + const wt = { ...worktree, comment: 'todo: fix bug' } + const base = estimateRowHeight(itemRow(worktree), ['comment'], repoMap, null) + const withComment = estimateRowHeight(itemRow(wt), ['comment'], repoMap, null) + expect(withComment - base).toBe(24) // 22px line + 2px mt-0.5 + }) + + it('stacks all metadata lines correctly', () => { + const wt = { ...worktree, linkedIssue: 42, comment: 'note' } + const prCache = { + '/tmp/orca::feature/cool': { data: { number: 1 } } + } + const h = estimateRowHeight(itemRow(wt), ['issue', 'pr', 'comment'], repoMap, prCache) + // 52 base + 22 issue + 22 pr + 22 comment + 2 mt-0.5 + 4 pb-1 = 124 + expect(h).toBe(124) + }) + + it('strips refs/heads/ prefix when building PR cache key', () => { + const wt = { ...worktree, branch: 'refs/heads/my-branch' } + const prCache = { + '/tmp/orca::my-branch': { data: { number: 5 } } + } + const h = estimateRowHeight(itemRow(wt), ['pr'], repoMap, prCache) + expect(h).toBe(80) // 52 + 22 + 2 + 4 + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-list-estimate.ts b/src/renderer/src/components/sidebar/worktree-list-estimate.ts new file mode 100644 index 00000000000..b47274151f2 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-list-estimate.ts @@ -0,0 +1,39 @@ +import type { Repo } from '../../../../shared/types' +import type { Row } from './worktree-list-groups' + +// Estimate the pixel height of a virtualizer row based on which metadata lines +// will render. Pixel constants (52, 22, 2, 4) are coupled to WorktreeCard's +// Tailwind classes — see the coupling comment in WorktreeCard's meta section. +// +// Uses prCache (not wt.linkedPR) because prCache is the actual data source +// WorktreeCard checks when deciding to show the PR row. +export function estimateRowHeight( + row: Row, + cardProps: string[], + repoMap: Map, + prCache: Record | null +): number { + if (row.type === 'header') { + return 42 + } + const wt = row.worktree + let h = 52 // base: py-2 + title + subtitle + gaps + if (cardProps.includes('issue') && wt.linkedIssue) { + h += 22 + } + if (cardProps.includes('pr')) { + const repo = repoMap.get(wt.repoId) + const branch = wt.branch.replace(/^refs\/heads\//, '') + const prKey = repo && branch ? `${repo.path}::${branch}` : '' + if (prKey && prCache?.[prKey]?.data) { + h += 22 + } + } + if (cardProps.includes('comment') && wt.comment) { + h += 22 + } + if (h > 52) { + h += 2 + } + return h + 4 +}