mirror of
https://github.com/stablyai/orca.git
synced 2026-09-25 00:02:35 +00:00
fix: dynamic estimateSize for worktree cards to prevent overlap (#346)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
85dd635598
commit
697c963986
@@ -357,7 +357,10 @@ const WorktreeCard = React.memo(function WorktreeCard({
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* 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)) && (
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
})
|
||||
})
|
||||
@@ -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<string, Repo>,
|
||||
prCache: Record<string, { data: unknown }> | 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
|
||||
}
|
||||
Reference in New Issue
Block a user