Fix sidebar reveal under sticky headers (#2988)
This commit is contained in:
parent
9ad94ce783
commit
5d2b801caa
|
|
@ -77,6 +77,7 @@ import {
|
|||
getActiveStickyHeaderIndex,
|
||||
getActiveStickyHeaderIndexForScroll,
|
||||
getPreviousStickyHeaderIndex,
|
||||
GROUP_HEADER_ROW_HEIGHT,
|
||||
getStickyHeaderIndexes,
|
||||
getVirtualRowTransform,
|
||||
shouldUseHeaderTopSpacing,
|
||||
|
|
@ -162,6 +163,9 @@ type ProjectGroupDeleteDialogState = {
|
|||
// terminal title changes) trigger score recalculations.
|
||||
const SORT_SETTLE_MS = 3_000
|
||||
const USER_SCROLL_MEASUREMENT_ADJUSTMENT_SUPPRESS_MS = 500
|
||||
const WORKTREE_REVEAL_TOP_CLEARANCE = 6
|
||||
export const WORKTREE_SIDEBAR_REVEAL_TOP_INSET =
|
||||
GROUP_HEADER_ROW_HEIGHT + WORKTREE_REVEAL_TOP_CLEARANCE
|
||||
const WORKTREE_SIDEBAR_SCROLL_STYLE: React.CSSProperties = {
|
||||
// Why: TanStack Virtual owns scroll correction. Native browser anchoring can
|
||||
// fight virtual row measurement/remounts and produce visible jumps.
|
||||
|
|
@ -253,12 +257,14 @@ function getMountedWorktreeBounds(
|
|||
|
||||
export function getScrollTopToRevealBounds(
|
||||
container: HTMLElement,
|
||||
bounds: Pick<VirtualItemBounds, 'start' | 'end'>
|
||||
bounds: Pick<VirtualItemBounds, 'start' | 'end'>,
|
||||
topInset = 0
|
||||
): number | null {
|
||||
const viewportTop = container.scrollTop
|
||||
const viewportBottom = viewportTop + container.clientHeight
|
||||
const viewportTopInset = Math.max(0, Math.min(container.clientHeight, topInset))
|
||||
const viewportTop = container.scrollTop + viewportTopInset
|
||||
const viewportBottom = container.scrollTop + container.clientHeight
|
||||
if (bounds.start < viewportTop) {
|
||||
return bounds.start
|
||||
return bounds.start - viewportTopInset
|
||||
}
|
||||
if (bounds.end > viewportBottom) {
|
||||
return bounds.end - container.clientHeight
|
||||
|
|
@ -275,7 +281,11 @@ function revealMountedWorktreeElement(
|
|||
if (!bounds) {
|
||||
return false
|
||||
}
|
||||
const nextScrollTop = getScrollTopToRevealBounds(container, bounds)
|
||||
const nextScrollTop = getScrollTopToRevealBounds(
|
||||
container,
|
||||
bounds,
|
||||
WORKTREE_SIDEBAR_REVEAL_TOP_INSET
|
||||
)
|
||||
if (nextScrollTop !== null) {
|
||||
container.scrollTo({ top: Math.max(0, nextScrollTop), behavior })
|
||||
}
|
||||
|
|
@ -955,6 +965,9 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
}, []),
|
||||
overscan: 10,
|
||||
gap: 6,
|
||||
// Why: the active sticky group header is rendered inside the virtual list,
|
||||
// so TanStack's scroll math needs the same top inset as the exact DOM reveal.
|
||||
scrollPaddingStart: WORKTREE_SIDEBAR_REVEAL_TOP_INSET,
|
||||
isScrollingResetDelay: USER_SCROLL_MEASUREMENT_ADJUSTMENT_SUPPRESS_MS,
|
||||
// Why: the sidebar rows are rich cards. Flushing their React render inside
|
||||
// TanStack's native scroll listener can make wheel input wait on card work;
|
||||
|
|
|
|||
|
|
@ -1,11 +1,14 @@
|
|||
import { describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
countRecordKeysByReference,
|
||||
getScrollTopToRevealBounds,
|
||||
resolvePendingSidebarReveal,
|
||||
WORKTREE_SIDEBAR_REVEAL_TOP_INSET,
|
||||
shouldAdjustWorktreeSidebarMeasuredRowScroll
|
||||
} from './WorktreeList'
|
||||
import {
|
||||
estimateRenderRowSize,
|
||||
GROUP_HEADER_ROW_HEIGHT,
|
||||
getActiveStickyHeaderIndexForScroll
|
||||
} from './worktree-list-virtual-rows'
|
||||
|
||||
|
|
@ -18,6 +21,9 @@ const makeHeaderRow = (key: string) =>
|
|||
tone: 'text-foreground'
|
||||
}) as const
|
||||
|
||||
const makeScrollContainer = (scrollTop: number, clientHeight: number): HTMLElement =>
|
||||
({ scrollTop, clientHeight }) as HTMLElement
|
||||
|
||||
describe('shouldAdjustWorktreeSidebarMeasuredRowScroll', () => {
|
||||
it('counts record keys once per object reference', () => {
|
||||
const keysSpy = vi.spyOn(Object, 'keys')
|
||||
|
|
@ -92,6 +98,68 @@ describe('shouldAdjustWorktreeSidebarMeasuredRowScroll', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('getScrollTopToRevealBounds', () => {
|
||||
it('treats the sticky header as occluding the viewport top', () => {
|
||||
const container = makeScrollContainer(100, 400)
|
||||
|
||||
expect(
|
||||
getScrollTopToRevealBounds(
|
||||
container,
|
||||
{
|
||||
start: 100,
|
||||
end: 216
|
||||
},
|
||||
GROUP_HEADER_ROW_HEIGHT
|
||||
)
|
||||
).toBe(72)
|
||||
})
|
||||
|
||||
it('includes extra reveal clearance for the highlight ring', () => {
|
||||
const container = makeScrollContainer(100, 400)
|
||||
|
||||
expect(
|
||||
getScrollTopToRevealBounds(
|
||||
container,
|
||||
{
|
||||
start: 100,
|
||||
end: 216
|
||||
},
|
||||
WORKTREE_SIDEBAR_REVEAL_TOP_INSET
|
||||
)
|
||||
).toBe(66)
|
||||
})
|
||||
|
||||
it('does not scroll when the bounds are below the sticky header', () => {
|
||||
const container = makeScrollContainer(100, 400)
|
||||
|
||||
expect(
|
||||
getScrollTopToRevealBounds(
|
||||
container,
|
||||
{
|
||||
start: 128,
|
||||
end: 244
|
||||
},
|
||||
GROUP_HEADER_ROW_HEIGHT
|
||||
)
|
||||
).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps the viewport bottom independent of the sticky header inset', () => {
|
||||
const container = makeScrollContainer(100, 400)
|
||||
|
||||
expect(
|
||||
getScrollTopToRevealBounds(
|
||||
container,
|
||||
{
|
||||
start: 430,
|
||||
end: 520
|
||||
},
|
||||
GROUP_HEADER_ROW_HEIGHT
|
||||
)
|
||||
).toBe(120)
|
||||
})
|
||||
})
|
||||
|
||||
describe('estimateRenderRowSize', () => {
|
||||
it('keeps secondary group header size stable while it is the active sticky header', () => {
|
||||
const rows = [makeHeaderRow('first'), makeHeaderRow('second')]
|
||||
|
|
|
|||
|
|
@ -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' }>
|
||||
|
|
|
|||
Loading…
Reference in New Issue