Fix sidebar scroll restore below sticky headers (#2976)
* Fix sidebar scroll restore below sticky headers * Split virtualized scroll anchor state
This commit is contained in:
parent
d963a03051
commit
0c69a713a2
|
|
@ -110,7 +110,7 @@ import {
|
|||
canGoBackWorktreeHistory,
|
||||
canGoForwardWorktreeHistory
|
||||
} from '@/store/slices/worktree-nav-history'
|
||||
import type { VirtualizedScrollAnchor } from './hooks/useVirtualizedScrollAnchor'
|
||||
import type { VirtualizedScrollAnchor } from './hooks/virtualizedScrollAnchorState'
|
||||
import type { RemoteWorkspacePatchResult } from '../../shared/remote-workspace-types'
|
||||
import type { OnboardingState } from '../../shared/types'
|
||||
import { FLOATING_TERMINAL_WORKTREE_ID } from '../../shared/constants'
|
||||
|
|
|
|||
|
|
@ -38,7 +38,7 @@ import { runWorktreeBatchDelete, runWorktreeDelete } from './delete-worktree-flo
|
|||
import { runSleepWorktrees } from './sleep-worktree-flow'
|
||||
import { activateAndRevealWorktree } from '@/lib/worktree-activation'
|
||||
import { tabHasLivePty } from '@/lib/tab-has-live-pty'
|
||||
import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/useVirtualizedScrollAnchor'
|
||||
import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/virtualizedScrollAnchorState'
|
||||
import { getLineageRenderInfo } from './worktree-list-groups'
|
||||
import { getWorkspaceStatus, getWorkspaceStatusVisualMeta } from './workspace-status'
|
||||
import { WorktreeOpenInSubMenu } from './WorktreeOpenInMenu'
|
||||
|
|
|
|||
|
|
@ -242,6 +242,17 @@ async function renderWorktreeListMarkup(): Promise<string> {
|
|||
}
|
||||
|
||||
describe('WorktreeList lineage child card renderer', () => {
|
||||
it('caps sidebar restore offsets so agent rows do not become the anchor', async () => {
|
||||
const { useVirtualizedScrollAnchor } = await import('@/hooks/useVirtualizedScrollAnchor')
|
||||
vi.mocked(useVirtualizedScrollAnchor).mockClear()
|
||||
setLineageFixtureState()
|
||||
await renderWorktreeListMarkup()
|
||||
|
||||
expect(useVirtualizedScrollAnchor).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ maxAnchorOffset: 4 })
|
||||
)
|
||||
})
|
||||
|
||||
it('renders nested inline agent rows before the nested child-count toggle', async () => {
|
||||
setLineageFixtureState()
|
||||
const markup = await renderWorktreeListMarkup()
|
||||
|
|
|
|||
|
|
@ -73,6 +73,7 @@ import {
|
|||
getLineageGroupKey
|
||||
} from './worktree-list-groups'
|
||||
import {
|
||||
GROUP_HEADER_ROW_HEIGHT,
|
||||
estimateRenderRowSize,
|
||||
getActiveStickyHeaderIndex,
|
||||
getActiveStickyHeaderIndexForScroll,
|
||||
|
|
@ -99,11 +100,11 @@ import {
|
|||
getVisibleWorktreeBrowserActivityTabs,
|
||||
getVisibleWorktreeTerminalActivityTabs
|
||||
} from './visible-worktree-activity-inputs'
|
||||
import { useVirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor'
|
||||
import {
|
||||
VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT,
|
||||
useVirtualizedScrollAnchor,
|
||||
type VirtualizedScrollAnchor
|
||||
} from '@/hooks/useVirtualizedScrollAnchor'
|
||||
} from '@/hooks/virtualizedScrollAnchorState'
|
||||
import { activateAndRevealWorktree } from '@/lib/worktree-activation'
|
||||
import { getShortcutPlatform } from '@/lib/shortcut-platform'
|
||||
import { SCROLL_TO_CURRENT_WORKSPACE_REVEAL_REQUEST_EVENT } from '@/lib/scroll-to-current-workspace-status'
|
||||
|
|
@ -253,12 +254,24 @@ function getMountedWorktreeBounds(
|
|||
|
||||
export function getScrollTopToRevealBounds(
|
||||
container: HTMLElement,
|
||||
bounds: Pick<VirtualItemBounds, 'start' | 'end'>
|
||||
bounds: Pick<VirtualItemBounds, 'start' | 'end'>,
|
||||
topInset = 0,
|
||||
align: 'nearest' | 'start' = 'nearest'
|
||||
): number | null {
|
||||
const viewportTop = container.scrollTop
|
||||
const visibleTop = viewportTop + topInset
|
||||
const viewportBottom = viewportTop + container.clientHeight
|
||||
if (bounds.start < viewportTop) {
|
||||
return bounds.start
|
||||
if (align === 'start') {
|
||||
const nextScrollTop = bounds.start - topInset
|
||||
return Math.abs(nextScrollTop - viewportTop) > 1 ? nextScrollTop : null
|
||||
}
|
||||
const boundsHeight = bounds.end - bounds.start
|
||||
const visibleHeight = container.clientHeight - topInset
|
||||
if (boundsHeight > visibleHeight && (bounds.start < visibleTop || bounds.end > viewportBottom)) {
|
||||
return bounds.start - topInset
|
||||
}
|
||||
if (bounds.start < visibleTop) {
|
||||
return bounds.start - topInset
|
||||
}
|
||||
if (bounds.end > viewportBottom) {
|
||||
return bounds.end - container.clientHeight
|
||||
|
|
@ -269,13 +282,14 @@ export function getScrollTopToRevealBounds(
|
|||
function revealMountedWorktreeElement(
|
||||
container: HTMLElement,
|
||||
worktreeId: string,
|
||||
behavior: ScrollBehavior
|
||||
behavior: ScrollBehavior,
|
||||
topInset = 0
|
||||
): boolean {
|
||||
const bounds = getMountedWorktreeBounds(container, worktreeId)
|
||||
if (!bounds) {
|
||||
return false
|
||||
}
|
||||
const nextScrollTop = getScrollTopToRevealBounds(container, bounds)
|
||||
const nextScrollTop = getScrollTopToRevealBounds(container, bounds, topInset)
|
||||
if (nextScrollTop !== null) {
|
||||
container.scrollTo({ top: Math.max(0, nextScrollTop), behavior })
|
||||
}
|
||||
|
|
@ -296,6 +310,8 @@ const LINEAGE_INDENT = 18
|
|||
const WORKTREE_GROUP_INDENT = 18
|
||||
const PROJECT_GROUP_HEADER_INDENT = 10
|
||||
const SIDEBAR_POINTER_DRAG_THRESHOLD_PX = 4
|
||||
const STICKY_HEADER_REVEAL_CLEARANCE_PX = 6
|
||||
const WORKTREE_SIDEBAR_MAX_ANCHOR_OFFSET_PX = 4
|
||||
|
||||
type VirtualizedWorktreeViewportProps = {
|
||||
rows: Row[]
|
||||
|
|
@ -798,6 +814,10 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
const firstHeaderIndexRef = useRef(firstHeaderIndex)
|
||||
firstHeaderIndexRef.current = firstHeaderIndex
|
||||
const stickyHeaderIndexes = useMemo(() => getStickyHeaderIndexes(renderRows), [renderRows])
|
||||
// Why: sticky group headers visually cover the top of the scrollport, so
|
||||
// reveal and anchor math must leave the workspace card visibly below it.
|
||||
const stickyHeaderTopInset =
|
||||
stickyHeaderIndexes.length > 0 ? GROUP_HEADER_ROW_HEIGHT + STICKY_HEADER_REVEAL_CLEARANCE_PX : 0
|
||||
const stickyHeaderIndexesRef = useRef(stickyHeaderIndexes)
|
||||
stickyHeaderIndexesRef.current = stickyHeaderIndexes
|
||||
const activeStickyHeaderIndexRef = useRef<number | null>(null)
|
||||
|
|
@ -1068,7 +1088,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
revealMountedWorktreeElement(
|
||||
container,
|
||||
pendingRevealWorktree.worktreeId,
|
||||
pendingRevealWorktree.behavior
|
||||
pendingRevealWorktree.behavior,
|
||||
stickyHeaderTopInset
|
||||
)
|
||||
) {
|
||||
if (pendingRevealWorktree.highlight) {
|
||||
|
|
@ -1123,7 +1144,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
settings,
|
||||
projectGroups,
|
||||
pendingRevealRetryTick,
|
||||
flashRevealedWorktree
|
||||
flashRevealedWorktree,
|
||||
stickyHeaderTopInset
|
||||
])
|
||||
|
||||
const prCacheLen = useAppStore((s) => countRecordKeysByReference(s.prCache))
|
||||
|
|
@ -1186,6 +1208,11 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
scrollOffsetRef,
|
||||
hasDirectScrollInput,
|
||||
shouldSkipRestore: shouldSkipScrollAnchorRestore,
|
||||
// Why: inline agent rows can grow after a run. Let tiny offsets survive so
|
||||
// normal measurement correction does not snap, but do not let the agent row
|
||||
// become the preserved anchor under the sticky project header.
|
||||
maxAnchorOffset: WORKTREE_SIDEBAR_MAX_ANCHOR_OFFSET_PX,
|
||||
topInset: stickyHeaderTopInset,
|
||||
totalSize,
|
||||
virtualizer
|
||||
})
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import AddRepoDialog from './AddRepoDialog'
|
|||
import ProjectAddedDialog from './ProjectAddedDialog'
|
||||
import WorktreeVisibilityDialog from './WorktreeVisibilityDialog'
|
||||
import OrcaYamlTrustDialog from './OrcaYamlTrustDialog'
|
||||
import type { VirtualizedScrollAnchor } from '@/hooks/useVirtualizedScrollAnchor'
|
||||
import type { VirtualizedScrollAnchor } from '@/hooks/virtualizedScrollAnchorState'
|
||||
|
||||
const MIN_WIDTH = 220
|
||||
const MAX_WIDTH = 500
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { toast } from 'sonner'
|
||||
import { useAppStore } from '@/store'
|
||||
import { clearWorktreeSleepIntent, markWorktreeSleepIntent } from '@/lib/worktree-sleep-intent'
|
||||
import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/useVirtualizedScrollAnchor'
|
||||
import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from '@/hooks/virtualizedScrollAnchorState'
|
||||
|
||||
/**
|
||||
* Shared "sleep worktree" flow (close all panels to free memory / CPU)
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import type { VirtualItem } from '@tanstack/react-virtual'
|
|||
import type { Row } from './worktree-list-groups'
|
||||
import { PINNED_GROUP_KEY } from './worktree-list-groups'
|
||||
|
||||
const GROUP_HEADER_ROW_HEIGHT = 28
|
||||
export const GROUP_HEADER_ROW_HEIGHT = 28
|
||||
const SECONDARY_GROUP_HEADER_TOP_MARGIN = 8
|
||||
|
||||
type WorktreeItemRow = Extract<Row, { type: 'item' }>
|
||||
|
|
|
|||
|
|
@ -12,6 +12,24 @@ describe('getScrollTopToRevealBounds', () => {
|
|||
expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 60, end: 120 })).toBe(60)
|
||||
})
|
||||
|
||||
it('scrolls upward when a mounted current workspace card is hidden under a sticky header', () => {
|
||||
expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 110, end: 180 }, 34)).toBe(
|
||||
76
|
||||
)
|
||||
})
|
||||
|
||||
it('top-aligns a tall workspace card instead of revealing only its lower agent rows', () => {
|
||||
expect(getScrollTopToRevealBounds(makeContainer(100, 100), { start: 130, end: 250 }, 34)).toBe(
|
||||
96
|
||||
)
|
||||
})
|
||||
|
||||
it('does not scroll when a mounted workspace card is already visible below a sticky header', () => {
|
||||
expect(
|
||||
getScrollTopToRevealBounds(makeContainer(100, 200), { start: 150, end: 220 }, 34)
|
||||
).toBeNull()
|
||||
})
|
||||
|
||||
it('scrolls downward to reveal a mounted current workspace card below the viewport', () => {
|
||||
expect(getScrollTopToRevealBounds(makeContainer(100, 200), { start: 250, end: 340 })).toBe(140)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from './useVirtualizedScrollAnchor'
|
||||
import { VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT } from './virtualizedScrollAnchorState'
|
||||
|
||||
/**
|
||||
* Asks a mounted virtualized scroller (matched by selector) to snapshot its
|
||||
|
|
|
|||
|
|
@ -8,13 +8,13 @@ import {
|
|||
} from 'react'
|
||||
import type { Virtualizer } from '@tanstack/react-virtual'
|
||||
import { shouldCancelVirtualizedScrollOffsetRestore } from './virtualizedScrollOffsetRestore'
|
||||
import {
|
||||
VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT,
|
||||
clampVirtualizedScrollAnchorOffset,
|
||||
type VirtualizedScrollAnchor
|
||||
} from './virtualizedScrollAnchorState'
|
||||
import { resolveVirtualizedScrollAnchorKey } from './virtualizedScrollAnchorResolution'
|
||||
|
||||
export type VirtualizedScrollAnchor = {
|
||||
fallbackKeys?: readonly string[]
|
||||
key: string
|
||||
offset: number
|
||||
} | null
|
||||
export const VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT = 'orca-record-virtualized-scroll-anchor'
|
||||
const RECORD_ANCHOR_SCROLL_IDLE_DELAY_MS = 150
|
||||
|
||||
type UseVirtualizedScrollAnchorOptions<
|
||||
|
|
@ -31,6 +31,8 @@ type UseVirtualizedScrollAnchorOptions<
|
|||
scrollElementRef: RefObject<TScrollElement | null>
|
||||
scrollOffsetRef: MutableRefObject<number>
|
||||
shouldSkipRestore?: () => boolean
|
||||
maxAnchorOffset?: number
|
||||
topInset?: number
|
||||
totalSize: number
|
||||
virtualizer: Virtualizer<TScrollElement, TItemElement>
|
||||
}
|
||||
|
|
@ -57,23 +59,23 @@ export function useVirtualizedScrollAnchor<
|
|||
scrollElementRef,
|
||||
scrollOffsetRef,
|
||||
shouldSkipRestore,
|
||||
maxAnchorOffset = Infinity,
|
||||
topInset = 0,
|
||||
totalSize,
|
||||
virtualizer
|
||||
}: UseVirtualizedScrollAnchorOptions<TRow, TScrollElement, TItemElement>): void {
|
||||
const rowIndexByKey = useMemo(() => {
|
||||
const indexByKey = new Map<string, number>()
|
||||
rows.forEach((row, index) => {
|
||||
indexByKey.set(getRowKey(row), index)
|
||||
})
|
||||
rows.forEach((row, index) => indexByKey.set(getRowKey(row), index))
|
||||
return indexByKey
|
||||
}, [getRowKey, rows])
|
||||
|
||||
const findDomAnchor = useCallback(
|
||||
(scrollElement: TScrollElement) => {
|
||||
if (!itemElementSelector || !getItemElementKey) {
|
||||
return null
|
||||
}
|
||||
const scrollRect = scrollElement.getBoundingClientRect()
|
||||
const visibleTop = scrollRect.top + topInset
|
||||
type DomAnchorItem = { element: TItemElement; key: string; rect: DOMRect }
|
||||
const visibleItems = Array.from(
|
||||
scrollElement.querySelectorAll<TItemElement>(itemElementSelector)
|
||||
|
|
@ -84,7 +86,7 @@ export function useVirtualizedScrollAnchor<
|
|||
return null
|
||||
}
|
||||
const rect = element.getBoundingClientRect()
|
||||
if (rect.height <= 0 || rect.bottom <= scrollRect.top || rect.top >= scrollRect.bottom) {
|
||||
if (rect.height <= 0 || rect.bottom <= visibleTop || rect.top >= scrollRect.bottom) {
|
||||
return null
|
||||
}
|
||||
return { element, key, rect }
|
||||
|
|
@ -99,19 +101,20 @@ export function useVirtualizedScrollAnchor<
|
|||
return {
|
||||
fallbackKeys: visibleItems.slice(1).map((item) => item.key),
|
||||
key: firstVisible.key,
|
||||
offset: Math.min(
|
||||
firstVisible.rect.height,
|
||||
Math.max(0, scrollRect.top - firstVisible.rect.top)
|
||||
offset: clampVirtualizedScrollAnchorOffset(
|
||||
Math.min(firstVisible.rect.height, visibleTop - firstVisible.rect.top),
|
||||
maxAnchorOffset
|
||||
)
|
||||
}
|
||||
},
|
||||
[getItemElementKey, itemElementSelector, rowIndexByKey]
|
||||
[getItemElementKey, itemElementSelector, maxAnchorOffset, rowIndexByKey, topInset]
|
||||
)
|
||||
|
||||
const recordVirtualScrollAnchor = useCallback(
|
||||
(scrollTop: number) => {
|
||||
const virtualItems = virtualizer.getVirtualItems()
|
||||
const firstVisible = virtualItems.find((item) => item.end > scrollTop)
|
||||
const visibleTop = scrollTop + topInset
|
||||
const firstVisible = virtualItems.find((item) => item.end > visibleTop)
|
||||
const row = firstVisible ? rows[firstVisible.index] : undefined
|
||||
if (!firstVisible || !row) {
|
||||
anchorRef.current = null
|
||||
|
|
@ -124,10 +127,10 @@ export function useVirtualizedScrollAnchor<
|
|||
.filter((row): row is TRow => row != null)
|
||||
.map(getRowKey),
|
||||
key: getRowKey(row),
|
||||
offset: Math.max(0, scrollTop - firstVisible.start)
|
||||
offset: clampVirtualizedScrollAnchorOffset(visibleTop - firstVisible.start, maxAnchorOffset)
|
||||
}
|
||||
},
|
||||
[anchorRef, getRowKey, rows, virtualizer]
|
||||
[anchorRef, getRowKey, maxAnchorOffset, rows, topInset, virtualizer]
|
||||
)
|
||||
|
||||
const recordScrollAnchor = useCallback(
|
||||
|
|
@ -259,16 +262,11 @@ export function useVirtualizedScrollAnchor<
|
|||
return
|
||||
}
|
||||
|
||||
const resolvedKey = rowIndexByKey.has(anchor.key)
|
||||
? anchor.key
|
||||
: anchor.fallbackKeys?.find((key) => rowIndexByKey.has(key))
|
||||
if (!resolvedKey) {
|
||||
return
|
||||
}
|
||||
const index = rowIndexByKey.get(resolvedKey)
|
||||
if (index === undefined) {
|
||||
const resolvedAnchor = resolveVirtualizedScrollAnchorKey(anchor, rowIndexByKey)
|
||||
if (!resolvedAnchor) {
|
||||
return
|
||||
}
|
||||
const { index, key: resolvedKey } = resolvedAnchor
|
||||
const offset = resolvedKey === anchor.key ? anchor.offset : 0
|
||||
|
||||
const restoreFromDomElement = (): boolean => {
|
||||
|
|
@ -284,7 +282,7 @@ export function useVirtualizedScrollAnchor<
|
|||
}
|
||||
const scrollRect = el.getBoundingClientRect()
|
||||
const rect = element.getBoundingClientRect()
|
||||
const desiredTop = scrollRect.top - offset
|
||||
const desiredTop = scrollRect.top + topInset - offset
|
||||
const delta = rect.top - desiredTop
|
||||
if (Math.abs(delta) > 1) {
|
||||
el.scrollTop += delta
|
||||
|
|
@ -300,7 +298,7 @@ export function useVirtualizedScrollAnchor<
|
|||
return false
|
||||
}
|
||||
const maxScrollTop = Math.max(0, el.scrollHeight - el.clientHeight)
|
||||
const nextScrollTop = Math.min(maxScrollTop, Math.max(0, item.start + offset))
|
||||
const nextScrollTop = Math.min(maxScrollTop, Math.max(0, item.start + offset - topInset))
|
||||
if (Math.abs(el.scrollTop - nextScrollTop) > 1) {
|
||||
el.scrollTop = nextScrollTop
|
||||
}
|
||||
|
|
@ -341,6 +339,7 @@ export function useVirtualizedScrollAnchor<
|
|||
scrollElementRef,
|
||||
scrollOffsetRef,
|
||||
shouldSkipRestore,
|
||||
topInset,
|
||||
totalSize,
|
||||
virtualizer,
|
||||
virtualizer.isScrolling
|
||||
|
|
|
|||
|
|
@ -0,0 +1,18 @@
|
|||
import type { VirtualizedScrollAnchor } from './virtualizedScrollAnchorState'
|
||||
|
||||
export function resolveVirtualizedScrollAnchorKey(
|
||||
anchor: VirtualizedScrollAnchor,
|
||||
rowIndexByKey: ReadonlyMap<string, number>
|
||||
): { index: number; key: string } | null {
|
||||
if (!anchor) {
|
||||
return null
|
||||
}
|
||||
const key = rowIndexByKey.has(anchor.key)
|
||||
? anchor.key
|
||||
: anchor.fallbackKeys?.find((fallbackKey) => rowIndexByKey.has(fallbackKey))
|
||||
if (!key) {
|
||||
return null
|
||||
}
|
||||
const index = rowIndexByKey.get(key)
|
||||
return index === undefined ? null : { index, key }
|
||||
}
|
||||
|
|
@ -0,0 +1,14 @@
|
|||
export type VirtualizedScrollAnchor = {
|
||||
fallbackKeys?: readonly string[]
|
||||
key: string
|
||||
offset: number
|
||||
} | null
|
||||
|
||||
export const VIRTUALIZED_SCROLL_ANCHOR_RECORD_EVENT = 'orca-record-virtualized-scroll-anchor'
|
||||
|
||||
export function clampVirtualizedScrollAnchorOffset(
|
||||
offset: number,
|
||||
maxAnchorOffset: number
|
||||
): number {
|
||||
return Math.min(maxAnchorOffset, Math.max(0, offset))
|
||||
}
|
||||
Loading…
Reference in New Issue