mirror of
https://github.com/stablyai/orca.git
synced 2026-10-27 00:02:48 +00:00
fix(sidebar): stop multi-host header style collision on host collapse
Stamp hostId onto unscoped headers cloned into host sections so virtualizer keys stay unique, and size host rows with HostSectionHeader's inner pt-1 so sticky tier geometry matches paint when collapsing/expanding hosts. Closes #12300
This commit is contained in:
Brennan Benson
@@ -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) }
|
||||
}
|
||||
>
|
||||
|
||||
@@ -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<HostSectionRow, { type: 'header' }> =>
|
||||
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', () => {
|
||||
|
||||
@@ -142,7 +142,9 @@ function localizePendingRowsForHost(
|
||||
const localized: Extract<Row, { type: 'header' }>[] = []
|
||||
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)
|
||||
|
||||
@@ -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<Row, { type: 'header' }> {
|
||||
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<Row, { type: 'item' }> {
|
||||
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<Row, { type: 'item' }>
|
||||
}
|
||||
|
||||
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<string>): 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<HostSectionRow, { type: 'header' }> =>
|
||||
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)
|
||||
})
|
||||
})
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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. */
|
||||
|
||||
Reference in New Issue
Block a user