fix(sidebar): make worktree drag reorder follow the card, not the pointer (#10845)

This commit is contained in:
Neil 2026-07-26 21:21:29 -07:00 committed by GitHub
parent dad3e3c9ef
commit c3a9a5e8a6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 569 additions and 169 deletions

View File

@ -194,7 +194,12 @@ import {
type WorktreeSidebarDragSession,
type WorktreeSidebarDragPoint
} from './worktree-sidebar-drag-autoscroll'
import { holdWorktreeSidebarDragRects } from './worktree-sidebar-drag-geometry'
import {
getWorktreeSidebarDragGrab,
shouldReevaluateWorktreeSidebarDropAnchor,
type WorktreeSidebarDragGrab,
type WorktreeSidebarDropAnchor
} from './worktree-sidebar-drag-geometry'
import {
computeWorktreeSidebarDropPreview,
resolveWorktreeSidebarStatusDropCommitTarget,
@ -1405,7 +1410,10 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
[worktreeLineageById, worktreeMap]
)
const worktreeDragSessionRef = useRef<WorktreeSidebarDragSession | null>(null)
const heldStatusDropRectsRef = useRef<Map<string, readonly WorktreeSidebarDragRect[]>>(new Map())
// Why: cross-group hovers hit-test a group the session never captured, so hold
// that group's drop decision separately or a card expanding in the target group
// moves the insertion line under a still pointer.
const statusDropAnchorsRef = useRef<Map<string, WorktreeSidebarDropAnchor>>(new Map())
const worktreePointerDragRef = useRef<WorktreePointerDrag | null>(null)
const worktreePointerAutoscrollFrameIdRef = useRef<number | null>(null)
const worktreePointerAutoscrollLastFrameTimeRef = useRef<number | null>(null)
@ -1595,9 +1603,10 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
pointerY: number
groupKey: string
rects: readonly WorktreeSidebarDragRect[]
liveRects?: readonly WorktreeSidebarDragRect[]
draggedIds: readonly string[]
draggingWorktreeId?: string | null
grab?: WorktreeSidebarDragGrab | null
anchor?: WorktreeSidebarDropAnchor | null
}): WorktreeSidebarDropPreview | null => {
const container = scrollRef.current
if (!container) {
@ -1613,10 +1622,11 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
containerTop: containerRect.top,
scrollTop: container.scrollTop,
rects: args.rects,
liveRects: args.liveRects,
groupIds: group.worktreeIds,
draggedIds: args.draggedIds,
draggingWorktreeId: args.draggingWorktreeId
draggingWorktreeId: args.draggingWorktreeId,
grab: args.grab,
anchor: args.anchor
})
},
[worktreeDragUnitGroups]
@ -1624,17 +1634,34 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
const computeWorktreeDrop = useCallback(
(pointerY: number): WorktreeSidebarDropPreview | null => {
const session = worktreeDragSessionRef.current
if (!session) {
const container = scrollRef.current
if (!session || !container) {
return null
}
return computeWorktreeDropForGroup({
const scrollTop = container.scrollTop
// Why: only real pointer or scroll movement should re-decide the slot; a
// card growing under a still pointer must not move it.
const anchor = shouldReevaluateWorktreeSidebarDropAnchor({
anchor: session.anchor,
pointerY,
scrollTop
})
? null
: session.anchor
const preview = computeWorktreeDropForGroup({
pointerY,
groupKey: session.sourceGroupKey,
rects: session.rects,
liveRects: session.liveRects,
draggedIds: session.reorderUnitDraggedIds,
draggingWorktreeId: session.draggingWorktreeId
draggingWorktreeId: session.draggingWorktreeId,
grab: session.grab,
anchor
})
worktreeDragSessionRef.current = {
...session,
anchor: preview ? { beforeWorktreeId: preview.dropAnchorId, pointerY, scrollTop } : null
}
return preview
},
[computeWorktreeDropForGroup]
)
@ -1649,23 +1676,35 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
return null
}
const groupKey = getWorkspaceStatusGroupKey(args.status)
// Why: cross-group hovers re-measure a group the drag session never
// captured, so hold its geometry here too or a card expanding in the
// target group jumps the insertion line under a still pointer.
const liveRects = getWorktreeSidebarDragRectsForGroup(container, groupKey)
const rects = holdWorktreeSidebarDragRects({
held: heldStatusDropRectsRef.current.get(groupKey),
measured: liveRects
const session = worktreeDragSessionRef.current
const scrollTop = container.scrollTop
const heldAnchor = statusDropAnchorsRef.current.get(groupKey) ?? null
const anchor = shouldReevaluateWorktreeSidebarDropAnchor({
anchor: heldAnchor,
pointerY: args.pointerY,
scrollTop
})
heldStatusDropRectsRef.current.set(groupKey, rects)
return computeWorktreeDropForGroup({
? null
: heldAnchor
const preview = computeWorktreeDropForGroup({
pointerY: args.pointerY,
groupKey,
rects,
liveRects,
rects: getWorktreeSidebarDragRectsForGroup(container, groupKey),
draggedIds: args.draggedIds,
draggingWorktreeId: worktreeDragSessionRef.current?.draggingWorktreeId ?? null
draggingWorktreeId: session?.draggingWorktreeId ?? null,
grab: session?.grab ?? null,
anchor
})
if (preview) {
statusDropAnchorsRef.current.set(groupKey, {
beforeWorktreeId: preview.dropAnchorId,
pointerY: args.pointerY,
scrollTop
})
} else {
statusDropAnchorsRef.current.delete(groupKey)
}
return preview
},
[computeWorktreeDropForGroup]
)
@ -2672,7 +2711,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
cleanupWorktreePointerDrag()
cancelWorktreeNativeAutoscroll()
worktreeDragSessionRef.current = null
heldStatusDropRectsRef.current.clear()
statusDropAnchorsRef.current.clear()
setWorktreeDragState(WORKTREE_ROW_DRAG_INITIAL_STATE)
}, [cancelWorktreeNativeAutoscroll, cleanupWorktreePointerDrag])
@ -3076,7 +3115,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
const beginWorktreePointerDrag = useCallback(
(drag: WorktreePointerDrag) => {
const { preview, offsetX, offsetY } = createSidebarDragPreview({
const { preview, offsetX, offsetY, height } = createSidebarDragPreview({
sourceRow: drag.sourceRow,
pointerX: drag.currentX,
pointerY: drag.currentY,
@ -3095,7 +3134,10 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
reorderDraggedIds: drag.reorderDraggedIds,
reorderUnitDraggedIds: drag.reorderUnitDraggedIds,
rects: drag.rects,
liveRects: drag.rects
// Why: reuse the floating preview's own offset so the hit test tracks the
// card the user sees, not the raw pointer.
grab: getWorktreeSidebarDragGrab({ offsetY, height }),
anchor: null
}
setWorktreeDragState({
draggingWorktreeId: drag.worktreeId,
@ -3498,11 +3540,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
}, [runWorktreeNativeAutoscrollFrame])
const handleWorktreeCardDragStart = useCallback(
(
_event: React.DragEvent<HTMLDivElement>,
worktreeId: string,
draggedIds: readonly string[]
) => {
(event: React.DragEvent<HTMLDivElement>, worktreeId: string, draggedIds: readonly string[]) => {
const sourceGroupKey =
worktreeDragGroups.find((group) => group.worktreeIds.includes(worktreeId))?.key ?? null
if (!sourceGroupKey) {
@ -3513,6 +3551,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
const rects = scrollRef.current
? getWorktreeSidebarDragRectsForGroup(scrollRef.current, sourceGroupKey)
: []
const sourceRect = event.currentTarget.getBoundingClientRect()
worktreeDragSessionRef.current = {
draggingWorktreeId: worktreeId,
sourceGroupKey,
@ -3520,7 +3559,11 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
reorderDraggedIds,
reorderUnitDraggedIds,
rects,
liveRects: rects
grab: getWorktreeSidebarDragGrab({
offsetY: event.clientY - sourceRect.top,
height: sourceRect.height
}),
anchor: null
}
setWorktreeDragState({
draggingWorktreeId: worktreeId,

View File

@ -58,20 +58,28 @@ function getPreviewLayoutDraggedIds(
return firstVisibleDraggedId ? [firstVisibleDraggedId] : draggedIds.slice(0, 1)
}
export type WorktreeDragPreviewLayout = {
offsets: Map<string, number>
// Top of the slot the dragged card lands in, in the same coordinate space as
// `rects`. Null when the drop is a no-op. The drop indicator draws here so the
// line marks the gap the offsets actually open.
placeholderTop: number | null
}
export function buildWorktreeDragPreviewOffsets(args: {
groupIds: readonly string[]
draggedIds: readonly string[]
draggingWorktreeId?: string | null
dropIndex: number
rects: readonly WorktreeDragPreviewRect[]
}): Map<string, number> {
}): WorktreeDragPreviewLayout {
const committedNextIds = moveWorktreeIdsWithinGroup(
args.groupIds,
args.draggedIds,
args.dropIndex
)
if (arraysEqual(committedNextIds, args.groupIds)) {
return new Map()
return { offsets: new Map(), placeholderTop: null }
}
// Why: dragging a large multi-select batch should advertise the insertion
@ -83,7 +91,7 @@ export function buildWorktreeDragPreviewOffsets(args: {
)
const nextIds = moveWorktreeIdsWithinGroup(args.groupIds, layoutDraggedIds, args.dropIndex)
if (arraysEqual(nextIds, args.groupIds)) {
return new Map()
return { offsets: new Map(), placeholderTop: null }
}
const draggedSet = new Set(layoutDraggedIds)
@ -142,5 +150,8 @@ export function buildWorktreeDragPreviewOffsets(args: {
offsets.set(rect.worktreeId, offset)
}
}
return offsets
return {
offsets,
placeholderTop: targetTopById.get(layoutDraggedIds[0] ?? '') ?? null
}
}

View File

@ -69,7 +69,7 @@ describe('moveWorktreeIdsWithinGroup', () => {
describe('buildWorktreeDragPreviewOffsets', () => {
it('slides intervening rows up while dragging a row down', () => {
const offsets = buildWorktreeDragPreviewOffsets({
const { offsets } = buildWorktreeDragPreviewOffsets({
groupIds: ['a', 'b', 'c', 'd'],
draggedIds: ['b'],
dropIndex: 4,
@ -88,7 +88,7 @@ describe('buildWorktreeDragPreviewOffsets', () => {
})
it('slides intervening rows down while dragging a row up', () => {
const offsets = buildWorktreeDragPreviewOffsets({
const { offsets } = buildWorktreeDragPreviewOffsets({
groupIds: ['a', 'b', 'c'],
draggedIds: ['c'],
dropIndex: 0,
@ -106,7 +106,7 @@ describe('buildWorktreeDragPreviewOffsets', () => {
})
it('returns no preview offsets for a no-op hover', () => {
const offsets = buildWorktreeDragPreviewOffsets({
const { offsets } = buildWorktreeDragPreviewOffsets({
groupIds: ['a', 'b'],
draggedIds: ['a'],
dropIndex: 1,
@ -120,7 +120,7 @@ describe('buildWorktreeDragPreviewOffsets', () => {
})
it('uses the dragged unit height when previewing variable-height rows', () => {
const offsets = buildWorktreeDragPreviewOffsets({
const { offsets } = buildWorktreeDragPreviewOffsets({
groupIds: ['parent', 'sibling'],
draggedIds: ['parent'],
dropIndex: 2,
@ -134,7 +134,7 @@ describe('buildWorktreeDragPreviewOffsets', () => {
})
it('uses the dragged unit height when previewing a short row above a tall row', () => {
const offsets = buildWorktreeDragPreviewOffsets({
const { offsets } = buildWorktreeDragPreviewOffsets({
groupIds: ['parent', 'sibling'],
draggedIds: ['sibling'],
dropIndex: 0,
@ -148,7 +148,7 @@ describe('buildWorktreeDragPreviewOffsets', () => {
})
it('reserves one card-height slot while previewing a multi-select batch', () => {
const offsets = buildWorktreeDragPreviewOffsets({
const { offsets } = buildWorktreeDragPreviewOffsets({
groupIds: ['a', 'b', 'c', 'd', 'e'],
draggedIds: ['b', 'c', 'd'],
draggingWorktreeId: 'b',
@ -170,7 +170,7 @@ describe('buildWorktreeDragPreviewOffsets', () => {
})
it('uses the grabbed selected card as the one preview placeholder', () => {
const offsets = buildWorktreeDragPreviewOffsets({
const { offsets } = buildWorktreeDragPreviewOffsets({
groupIds: ['a', 'b', 'c', 'd', 'e'],
draggedIds: ['b', 'c', 'd'],
draggingWorktreeId: 'd',
@ -188,7 +188,7 @@ describe('buildWorktreeDragPreviewOffsets', () => {
})
it('returns no preview offsets for a no-op multi-select hover', () => {
const offsets = buildWorktreeDragPreviewOffsets({
const { offsets } = buildWorktreeDragPreviewOffsets({
groupIds: ['a', 'b', 'c', 'd', 'e'],
draggedIds: ['b', 'c', 'd'],
draggingWorktreeId: 'b',

View File

@ -22,7 +22,8 @@ const SESSION: WorktreeSidebarDragSession = {
reorderDraggedIds: ['b'],
reorderUnitDraggedIds: ['b'],
rects: [{ worktreeId: 'b', groupIndex: 1, top: 48, bottom: 88 }],
liveRects: [{ worktreeId: 'b', groupIndex: 1, top: 48, bottom: 88 }]
grab: null,
anchor: null
}
describe('getWorktreeSidebarDragAutoscroll', () => {
@ -160,7 +161,7 @@ describe('refreshWorktreeSidebarDragSession', () => {
rects
})
// Why: the row set changed ('a' mounted), so the fresh measurement is adopted.
).toEqual({ ...SESSION, rects, liveRects: rects })
).toEqual({ ...SESSION, rects })
})
it('clears when the source group is missing', () => {
@ -201,7 +202,7 @@ describe('refreshWorktreeSidebarDragSession', () => {
unitGroups: [{ key: 'repo:one', worktreeIds: ['a', 'b'], units: [] }],
rects: []
})
).toEqual({ ...SESSION, rects: [], liveRects: [] })
).toEqual({ ...SESSION, rects: [] })
})
it('keeps child-card reorder drags even when the child is not a top-level unit', () => {
@ -234,7 +235,7 @@ describe('refreshWorktreeSidebarDragSession', () => {
],
rects
})
).toEqual({ ...childSession, rects, liveRects: rects })
).toEqual({ ...childSession, rects })
})
})

View File

@ -1,6 +1,9 @@
import type { WorktreeDragGroup } from './worktree-manual-order'
import type { WorktreeDragUnitGroup } from './worktree-drag-units'
import { holdWorktreeSidebarDragRects } from './worktree-sidebar-drag-geometry'
import type {
WorktreeSidebarDragGrab,
WorktreeSidebarDropAnchor
} from './worktree-sidebar-drag-geometry'
const EDGE_ZONE_PX = 56
const MAX_OUTSIDE_EDGE_PX = 48
@ -26,11 +29,12 @@ export type WorktreeSidebarDragSession = {
draggedIds: readonly string[]
reorderDraggedIds: readonly string[]
reorderUnitDraggedIds: readonly string[]
// Why: `rects` are held stable for hit testing so resizing cards cannot move
// the drop target; `liveRects` keep the rendered indicator and row previews
// anchored to where the cards actually are right now.
// Why: one live coordinate space for both hit testing and rendering. Stability
// against mid-drag card resizes comes from holding the drop *decision*
// (`anchor`), not from freezing this geometry.
rects: readonly WorktreeSidebarDragRect[]
liveRects: readonly WorktreeSidebarDragRect[]
grab: WorktreeSidebarDragGrab | null
anchor: WorktreeSidebarDropAnchor | null
}
export type WorktreeSidebarAutoscrollResult = {
@ -195,11 +199,7 @@ export function refreshWorktreeSidebarDragSession(args: {
return null
}
return {
...args.session,
rects: holdWorktreeSidebarDragRects({ held: args.session.rects, measured: args.rects }),
liveRects: args.rects
}
return { ...args.session, rects: args.rects }
}
function getVerticalEdgeIntensity(

View File

@ -1,6 +1,12 @@
import { describe, expect, it } from 'vitest'
import { computeWorktreeSidebarDropPreview } from './worktree-sidebar-drop-preview'
import { holdWorktreeSidebarDragRects } from './worktree-sidebar-drag-geometry'
import {
getWorktreeSidebarDragGrab,
getWorktreeSidebarDragReferenceY,
resolveWorktreeSidebarDropAnchorIndex,
shouldReevaluateWorktreeSidebarDropAnchor,
type WorktreeSidebarDropAnchor
} from './worktree-sidebar-drag-geometry'
import {
refreshWorktreeSidebarDragSession,
type WorktreeSidebarDragRect
@ -23,97 +29,153 @@ function layout(heightByWorktreeId: Readonly<Record<string, number>>): WorktreeS
}
const COLLAPSED = layout({})
const GRAB = { offsetY: CARD_HEIGHT / 2, height: CARD_HEIGHT }
function previewAt(args: {
pointerY: number
rects: readonly WorktreeSidebarDragRect[]
liveRects?: readonly WorktreeSidebarDragRect[]
}): { dropIndex: number; dropIndicatorY: number } | null {
const preview = computeWorktreeSidebarDropPreview({
anchor?: WorktreeSidebarDropAnchor | null
draggingWorktreeId?: string
grab?: { offsetY: number; height: number } | null
}) {
return computeWorktreeSidebarDropPreview({
pointerY: args.pointerY,
containerTop: 0,
scrollTop: 0,
rects: args.rects,
liveRects: args.liveRects,
groupIds: GROUP_IDS,
draggedIds: ['a'],
draggingWorktreeId: 'a'
draggedIds: [args.draggingWorktreeId ?? 'a'],
draggingWorktreeId: args.draggingWorktreeId ?? 'a',
grab: args.grab === undefined ? GRAB : args.grab,
anchor: args.anchor
})
return preview ? { dropIndex: preview.dropIndex, dropIndicatorY: preview.dropIndicatorY } : null
}
/**
* Replay a drag where the pointer never moves while a card animates open, holding
* the drop decision across frames exactly as the live drag loop does.
*/
function replayStillPointer(args: {
pointerY: number
frames: readonly (readonly WorktreeSidebarDragRect[])[]
}): { dropIndexes: number[]; indicatorYs: number[] } {
let anchor: WorktreeSidebarDropAnchor | null = null
const dropIndexes: number[] = []
const indicatorYs: number[] = []
for (const rects of args.frames) {
const held = shouldReevaluateWorktreeSidebarDropAnchor({
anchor,
pointerY: args.pointerY,
scrollTop: 0
})
? null
: anchor
const preview = previewAt({ pointerY: args.pointerY, rects, anchor: held })!
anchor = { beforeWorktreeId: preview.dropAnchorId, pointerY: args.pointerY, scrollTop: 0 }
dropIndexes.push(preview.dropIndex)
indicatorYs.push(preview.dropIndicatorY)
}
return { dropIndexes, indicatorYs }
}
describe('worktree sidebar drag geometry under mid-drag card growth', () => {
it('keeps the drop target fixed while a card expands under a still pointer', () => {
const pointerY = 250
const before = previewAt({ pointerY, rects: COLLAPSED })
// Card 'b' expands its agent list while the pointer does not move at all.
const grown = layout({ b: EXPANDED_CARD_HEIGHT })
const liveDropIndex = previewAt({ pointerY, rects: grown })?.dropIndex
const held = holdWorktreeSidebarDragRects({ held: COLLAPSED, measured: grown })
const after = previewAt({ pointerY, rects: held, liveRects: grown })
// Re-measuring live would move the drop target with zero pointer movement.
expect(liveDropIndex).not.toBe(before?.dropIndex)
expect(after?.dropIndex).toBe(before?.dropIndex)
const unheld = previewAt({ pointerY, rects: grown })!.dropIndex
const { dropIndexes } = replayStillPointer({ pointerY, frames: [COLLAPSED, grown] })
// Re-deciding from the grown layout would move the target with zero input.
expect(unheld).not.toBe(dropIndexes[0])
expect(dropIndexes[1]).toBe(dropIndexes[0])
})
it('never lets a growing card change the drop target across a whole expansion animation', () => {
const pointerY = 250
it('never lets a growing card change the drop target across a whole expansion', () => {
const frames = Array.from({ length: 12 }, (_, frame) =>
layout({ b: CARD_HEIGHT + ((EXPANDED_CARD_HEIGHT - CARD_HEIGHT) * frame) / 11 })
)
const live = frames.map((rects) => previewAt({ pointerY, rects })?.dropIndex)
const stabilized = frames.map(
(rects) =>
previewAt({
pointerY,
rects: holdWorktreeSidebarDragRects({ held: COLLAPSED, measured: rects }),
liveRects: rects
})?.dropIndex
)
for (const pointerY of [150, 250, 350, 450, 550]) {
const unheld = frames.map((rects) => previewAt({ pointerY, rects })!.dropIndex)
const { dropIndexes } = replayStillPointer({ pointerY, frames })
expect(new Set(live).size).toBeGreaterThan(1)
expect(new Set(stabilized)).toEqual(
new Set([previewAt({ pointerY, rects: COLLAPSED })?.dropIndex])
)
expect(new Set(dropIndexes).size).toBe(1)
// The scenario has to be one that actually moves without the hold.
if (pointerY !== 150) {
expect(new Set(unheld).size).toBeGreaterThan(1)
}
}
})
it('still tracks the pointer normally while geometry is held', () => {
const held = holdWorktreeSidebarDragRects({
held: COLLAPSED,
measured: layout({ b: EXPANDED_CARD_HEIGHT })
})
expect(previewAt({ pointerY: 100, rects: held })?.dropIndex).toBeLessThan(
previewAt({ pointerY: 500, rects: held })?.dropIndex ?? -1
it('slides the indicator with the gap it marks while geometry is held', () => {
const frames = Array.from({ length: 12 }, (_, frame) =>
layout({ b: CARD_HEIGHT + ((EXPANDED_CARD_HEIGHT - CARD_HEIGHT) * frame) / 11 })
)
const { dropIndexes, indicatorYs } = replayStillPointer({ pointerY: 350, frames })
expect(new Set(dropIndexes).size).toBe(1)
// Held decision, live rendering: the line tracks the growing card, never freezes.
expect(indicatorYs.at(-1)!).toBeGreaterThan(indicatorYs[0]!)
expect(new Set(indicatorYs).size).toBe(frames.length)
})
it('draws the indicator at the live position so a grown card does not strand it', () => {
it('still tracks the pointer normally once it moves again', () => {
const grown = layout({ b: EXPANDED_CARD_HEIGHT })
const held = holdWorktreeSidebarDragRects({ held: COLLAPSED, measured: grown })
const preview = previewAt({ pointerY: 250, rects: held, liveRects: grown })
const dropIndex = preview?.dropIndex ?? -1
expect(preview?.dropIndicatorY).toBe(grown[dropIndex]!.top - 3)
// Held geometry alone would have parked the line ~288px above the real gap.
expect(preview?.dropIndicatorY).not.toBe(COLLAPSED[dropIndex]!.top - 3)
expect(previewAt({ pointerY: 100, rects: grown })!.dropIndex).toBeLessThan(
previewAt({ pointerY: 800, rects: grown })!.dropIndex
)
})
it('adopts fresh geometry when rows mount or change slot mid-drag', () => {
const reordered = COLLAPSED.map((rect, index) => ({
...rect,
worktreeId: GROUP_IDS[(index + 1) % GROUP_IDS.length]!
}))
it('re-evaluates on real pointer or scroll movement but not on jitter', () => {
const anchor: WorktreeSidebarDropAnchor = {
beforeWorktreeId: 'c',
pointerY: 250,
scrollTop: 40
}
expect(holdWorktreeSidebarDragRects({ held: COLLAPSED, measured: reordered })).toBe(reordered)
expect(holdWorktreeSidebarDragRects({ held: undefined, measured: COLLAPSED })).toBe(COLLAPSED)
expect(holdWorktreeSidebarDragRects({ held: [], measured: COLLAPSED })).toBe(COLLAPSED)
expect(
shouldReevaluateWorktreeSidebarDropAnchor({ anchor, pointerY: 250, scrollTop: 40 })
).toBe(false)
expect(
shouldReevaluateWorktreeSidebarDropAnchor({ anchor, pointerY: 250.2, scrollTop: 40 })
).toBe(false)
expect(
shouldReevaluateWorktreeSidebarDropAnchor({ anchor, pointerY: 254, scrollTop: 40 })
).toBe(true)
expect(
shouldReevaluateWorktreeSidebarDropAnchor({ anchor, pointerY: 250, scrollTop: 88 })
).toBe(true)
expect(
shouldReevaluateWorktreeSidebarDropAnchor({ anchor: null, pointerY: 250, scrollTop: 40 })
).toBe(true)
})
it('holds hit-test geometry across a session refresh while liveRects stay current', () => {
it('falls back to a fresh decision when the anchored card disappears mid-drag', () => {
const anchor: WorktreeSidebarDropAnchor = {
beforeWorktreeId: 'gone',
pointerY: 250,
scrollTop: 0
}
expect(resolveWorktreeSidebarDropAnchorIndex({ anchor, rects: COLLAPSED })).toBeNull()
expect(
resolveWorktreeSidebarDropAnchorIndex({
anchor: { beforeWorktreeId: 'c', pointerY: 0, scrollTop: 0 },
rects: COLLAPSED
})
).toBe(2)
// A null anchor id means end-of-group, which survives any row count change.
expect(
resolveWorktreeSidebarDropAnchorIndex({
anchor: { beforeWorktreeId: null, pointerY: 0, scrollTop: 0 },
rects: COLLAPSED
})
).toBe(COLLAPSED.length)
})
it('keeps one live coordinate space across a session refresh', () => {
const grown = layout({ b: EXPANDED_CARD_HEIGHT })
const refreshed = refreshWorktreeSidebarDragSession({
session: {
@ -123,7 +185,8 @@ describe('worktree sidebar drag geometry under mid-drag card growth', () => {
reorderDraggedIds: ['a'],
reorderUnitDraggedIds: ['a'],
rects: COLLAPSED,
liveRects: COLLAPSED
grab: GRAB,
anchor: null
},
groups: [{ key: 'repo:one', worktreeIds: GROUP_IDS }],
unitGroups: [
@ -136,7 +199,65 @@ describe('worktree sidebar drag geometry under mid-drag card growth', () => {
rects: grown
})
expect(refreshed?.rects).toBe(COLLAPSED)
expect(refreshed?.liveRects).toBe(grown)
expect(refreshed?.rects).toBe(grown)
expect(refreshed?.grab).toBe(GRAB)
})
})
describe('grab-relative hit testing', () => {
it('projects the dragged card from the pointer instead of using the bare pointer', () => {
const activeRect = { worktreeId: 'a', groupIndex: 0, top: 0, bottom: CARD_HEIGHT }
// Grabbed at the very top edge: the card sits below the pointer.
expect(
getWorktreeSidebarDragReferenceY({
localY: 300,
grab: { offsetY: 0, height: CARD_HEIGHT },
activeRect
})
).toBe(300 + CARD_HEIGHT / 2)
// Grabbed at the bottom edge: the card sits above the pointer.
expect(
getWorktreeSidebarDragReferenceY({
localY: 300,
grab: { offsetY: CARD_HEIGHT, height: CARD_HEIGHT },
activeRect
})
).toBe(300 - CARD_HEIGHT / 2)
// No grab (native HTML5 drag) degrades to the raw pointer.
expect(getWorktreeSidebarDragReferenceY({ localY: 300, grab: null, activeRect })).toBe(300)
})
it('resolves the same slot wherever a tall card was grabbed', () => {
const rects = layout({ c: EXPANDED_CARD_HEIGHT })
const tall = rects.find((rect) => rect.worktreeId === 'c')!
const height = tall.bottom - tall.top
// Park the card so it visually occupies b's slot, varying only the grab point.
const slotTop = rects[1]!.top
const dropIndexes = [0.05, 0.25, 0.5, 0.75, 0.95].map((fraction) => {
const offsetY = height * fraction
return previewAt({
pointerY: slotTop + offsetY,
rects,
draggingWorktreeId: 'c',
grab: { offsetY, height }
})!.dropIndex
})
expect(new Set(dropIndexes).size).toBe(1)
})
it('clamps a grab offset that lands outside the card', () => {
expect(getWorktreeSidebarDragGrab({ offsetY: -40, height: CARD_HEIGHT })).toEqual({
offsetY: 0,
height: CARD_HEIGHT
})
expect(getWorktreeSidebarDragGrab({ offsetY: 900, height: CARD_HEIGHT })).toEqual({
offsetY: CARD_HEIGHT,
height: CARD_HEIGHT
})
expect(getWorktreeSidebarDragGrab({ offsetY: 10, height: 0 })).toBeNull()
expect(getWorktreeSidebarDragGrab({ offsetY: Number.NaN, height: CARD_HEIGHT })).toBeNull()
})
})

View File

@ -1,34 +1,112 @@
import type { WorktreeSidebarDragRect } from './worktree-sidebar-drag-autoscroll'
function getDragRowSignature(rects: readonly WorktreeSidebarDragRect[]): string {
return rects.map((rect) => `${rect.worktreeId}@${rect.groupIndex}`).join('|')
export type WorktreeSidebarDragGrab = {
// Distance from the dragged unit's top to the grab point, captured at drag start.
offsetY: number
height: number
}
export type WorktreeSidebarDropAnchor = {
// Identity of the unit the dragged card inserts before; null means end-of-group.
beforeWorktreeId: string | null
pointerY: number
scrollTop: number
}
// Why: sub-pixel pointer jitter and scroll rounding must not count as intent.
const ANCHOR_REEVALUATE_EPSILON_PX = 0.5
/**
* Why: the pointer is not what the user is placing the card is. Hit-testing the
* bare pointer makes the same visual placement resolve differently depending on
* where the card was grabbed, which is the "unnatural" part with tall expanded
* agent cards: grab one near its bottom and it drops a slot late.
*
* Project the dragged card from the pointer and compare its center instead, so
* the drop follows where the card actually sits.
*/
export function getWorktreeSidebarDragReferenceY(args: {
localY: number
grab: WorktreeSidebarDragGrab | null
activeRect: WorktreeSidebarDragRect | null
}): number {
if (!args.grab) {
return args.localY
}
const height =
args.grab.height > 0
? args.grab.height
: args.activeRect
? args.activeRect.bottom - args.activeRect.top
: 0
return args.localY - args.grab.offsetY + height / 2
}
/**
* Why: the drop index comes from comparing the pointer against row midpoints,
* and sidebar cards keep resizing mid-drag agent statuses stream in and
* expansion panels animate open underneath the pointer. Measuring afresh every
* frame lets a card that grows while the pointer barely moves shove those
* midpoints past it, so one nudge teleports the insertion line several slots.
* Why: cards resize constantly mid-drag agent statuses stream in and expansion
* panels animate open so re-deciding the slot from geometry every frame lets a
* card growing under a still pointer move the drop target with zero input.
*
* Hold the geometry captured when the drag reached this row set, so only pointer
* movement can change the drop target. The set is held whole rather than merged
* per row: mixing held tops with freshly measured ones would describe two
* different layouts at once. When the rows themselves change mounting during
* autoscroll, or genuinely changing slot the fresh measurement is adopted
* wholesale so the coordinate space stays consistent.
*
* Only hit testing uses this; the indicator and row previews still render from
* live geometry, so a card growing mid-drag never leaves them stale.
* Hold the *decision* (insert before this card) rather than the *geometry* it was
* made from. Re-deriving that identity against live rects each frame keeps the
* indicator and row previews on one honest coordinate space, while only real
* pointer or scroll movement can pick a different neighbour. Freezing the rects
* instead the previous approach kept the target stable but let hit testing
* and rendering describe two different layouts at once.
*/
export function holdWorktreeSidebarDragRects(args: {
held: readonly WorktreeSidebarDragRect[] | undefined
measured: readonly WorktreeSidebarDragRect[]
}): readonly WorktreeSidebarDragRect[] {
if (!args.held || args.held.length === 0) {
return args.measured
export function shouldReevaluateWorktreeSidebarDropAnchor(args: {
anchor: WorktreeSidebarDropAnchor | null
pointerY: number
scrollTop: number
}): boolean {
if (!args.anchor) {
return true
}
return (
Math.abs(args.anchor.pointerY - args.pointerY) > ANCHOR_REEVALUATE_EPSILON_PX ||
Math.abs(args.anchor.scrollTop - args.scrollTop) > ANCHOR_REEVALUATE_EPSILON_PX
)
}
/**
* Resolve a held anchor back to a drop index in the current layout. Returns null
* when the anchored card is gone (deleted, filtered, or unmounted by
* virtualization), so the caller falls back to a fresh geometric decision.
*/
export function resolveWorktreeSidebarDropAnchorIndex(args: {
anchor: WorktreeSidebarDropAnchor
rects: readonly WorktreeSidebarDragRect[]
}): number | null {
if (args.anchor.beforeWorktreeId === null) {
return args.rects.length
}
const target = args.rects.find((rect) => rect.worktreeId === args.anchor.beforeWorktreeId)
return target ? target.groupIndex : null
}
export function getWorktreeSidebarDropAnchorId(args: {
rects: readonly WorktreeSidebarDragRect[]
dropIndex: number
}): string | null {
return args.rects.find((rect) => rect.groupIndex === args.dropIndex)?.worktreeId ?? null
}
/**
* Where inside the dragged card the pointer grabbed it. The floating drag preview
* is a fixed-size clone of the source row, so these are exactly the numbers that
* place it on screen reusing them keeps hit testing agreeing with what the user
* sees. Returns null for an unmeasured row, degrading to bare-pointer hit testing
* rather than to a wrong offset.
*/
export function getWorktreeSidebarDragGrab(args: {
offsetY: number
height: number
}): WorktreeSidebarDragGrab | null {
if (!Number.isFinite(args.offsetY) || !Number.isFinite(args.height) || args.height <= 0) {
return null
}
return {
offsetY: Math.min(Math.max(args.offsetY, 0), args.height),
height: args.height
}
return getDragRowSignature(args.held) === getDragRowSignature(args.measured)
? args.held
: args.measured
}

View File

@ -54,9 +54,12 @@ describe('computeWorktreeSidebarDropPreview', () => {
draggedIds: ['parent']
})
// Sibling slides up to 0 and the 282px-tall parent unit lands right after it,
// so the line marks 106 - 3. The old rule pointed at the sibling's stale
// bottom (391), a full lineage-height below where the card actually lands.
expect(preview).toMatchObject({
dropIndex: 2,
dropIndicatorY: 391
dropIndicatorY: 103
})
expect(Array.from(preview?.previewOffsetsByWorktreeId ?? [])).toEqual([['sibling', -288]])
})
@ -78,9 +81,11 @@ describe('computeWorktreeSidebarDropPreview', () => {
draggingWorktreeId: 'b'
})
// c/d/e slide up one slot, so the placeholder opens at 224 and the line marks
// 221 — not 277, which was one whole card height below the real gap.
expect(preview).toMatchObject({
dropIndex: 5,
dropIndicatorY: 277
dropIndicatorY: 221
})
expect(Array.from(preview?.previewOffsetsByWorktreeId ?? [])).toEqual([
['c', -56],
@ -108,17 +113,75 @@ describe('computeWorktreeSidebarDropPreview', () => {
expect(preview).toMatchObject({
dropIndex: 5,
dropIndicatorY: 277
dropIndicatorY: 221
})
expect(Array.from(preview?.previewOffsetsByWorktreeId ?? [])).toEqual([['e', -56]])
})
it('marks the gap the row previews open, not the displaced card top', () => {
// One expanded agent card (404px) among collapsed ones: dragging 'a' below it
// is exactly the case where the old rule put the line a card-height off.
const rects = [
{ worktreeId: 'a', groupIndex: 0, top: 0, bottom: 116 },
{ worktreeId: 'b', groupIndex: 1, top: 122, bottom: 238 },
{ worktreeId: 'expanded', groupIndex: 2, top: 244, bottom: 648 },
{ worktreeId: 'd', groupIndex: 3, top: 654, bottom: 770 }
]
const groupIds = ['a', 'b', 'expanded', 'd']
const preview = computeWorktreeSidebarDropPreview({
pointerY: 700,
containerTop: 0,
scrollTop: 0,
rects,
groupIds,
draggedIds: ['a'],
draggingWorktreeId: 'a',
grab: { offsetY: 58, height: 116 }
})!
// 'a' lands last, so every other card slides up by its 116px + 6px gap.
const offsets = preview.previewOffsetsByWorktreeId
expect(offsets.get('b')).toBe(-122)
expect(offsets.get('expanded')).toBe(-122)
expect(offsets.get('d')).toBe(-122)
// d ends at 532..648, so a's slot opens at 654 and the line marks 651.
expect(preview.dropIndicatorY).toBe(651)
// The old rule had no rect at this index and fell back to the pre-drag list
// bottom (773) — 122px below the gap the user could see opening.
})
it('resolves the same slot wherever a tall card is grabbed', () => {
const rects = [
{ worktreeId: 'a', groupIndex: 0, top: 0, bottom: 116 },
{ worktreeId: 'b', groupIndex: 1, top: 122, bottom: 238 },
{ worktreeId: 'expanded', groupIndex: 2, top: 244, bottom: 648 }
]
const groupIds = ['a', 'b', 'expanded']
const height = 404
const dropIndexes = [0.05, 0.5, 0.95].map((fraction) => {
const offsetY = height * fraction
return computeWorktreeSidebarDropPreview({
pointerY: 122 + offsetY,
containerTop: 0,
scrollTop: 0,
rects,
groupIds,
draggedIds: ['expanded'],
draggingWorktreeId: 'expanded',
grab: { offsetY, height }
})!.dropIndex
})
expect(new Set(dropIndexes).size).toBe(1)
})
})
describe('resolveWorktreeSidebarStatusDropCommitTarget', () => {
const preview = {
dropIndex: 1,
dropIndicatorY: 129,
previewOffsetsByWorktreeId: new Map<string, number>()
previewOffsetsByWorktreeId: new Map<string, number>(),
dropAnchorId: null
}
it('uses the current status target when pointerup hit-testing succeeds', () => {

View File

@ -3,11 +3,21 @@ import {
getWorktreeSidebarBoundaryDrop,
type WorktreeSidebarDragRect
} from './worktree-sidebar-drag-autoscroll'
import {
getWorktreeSidebarDragReferenceY,
getWorktreeSidebarDropAnchorId,
resolveWorktreeSidebarDropAnchorIndex,
type WorktreeSidebarDragGrab,
type WorktreeSidebarDropAnchor
} from './worktree-sidebar-drag-geometry'
export type WorktreeSidebarDropPreview = {
dropIndex: number
dropIndicatorY: number
previewOffsetsByWorktreeId: ReadonlyMap<string, number>
// Identity of the unit the drop inserts before, so the next frame can hold this
// decision through a card resize instead of re-deciding from moved geometry.
dropAnchorId: string | null
lineageParentId?: string
}
@ -92,10 +102,28 @@ export function resolveWorktreeSidebarStatusDropCommitTarget(args: {
: { target: args.currentTarget, preview: args.currentPreview }
}
/**
* Why: the line must mark the gap the row previews actually open, not the old top
* of whatever card happens to sit at `dropIndex`. Those differ by a full card
* height whenever the drop shifts that card, and by the tall card's height when
* an expanded agent session is involved which is the line landing "in the wrong
* spot". `placeholderTop` is the replayed layout's slot for the dragged card, so
* prefer it and fall back only when the drop is a no-op.
*/
function getWorktreeSidebarDropIndicatorY(args: {
rects: readonly WorktreeSidebarDragRect[]
dropIndex: number
placeholderTop: number | null
activeRect: WorktreeSidebarDragRect | null
}): number {
if (args.placeholderTop !== null) {
return Math.max(0, args.placeholderTop - 3)
}
// A no-op drop leaves the card where it is; park the line on its own top edge
// rather than jumping to a neighbour that never moves.
if (args.activeRect) {
return Math.max(0, args.activeRect.top - 3)
}
const target = args.rects.find((rect) => rect.groupIndex === args.dropIndex)
if (target) {
return Math.max(0, target.top - 3)
@ -104,18 +132,58 @@ function getWorktreeSidebarDropIndicatorY(args: {
return last ? last.bottom + 3 : 0
}
/**
* Why: with uniform rows, "first midpoint below the pointer" and "closest center
* to the dragged card" agree. With a 116px card next to a 404px expanded one they
* do not: the tall card's midpoint sits ~200px from its own top edge, so crossing
* it requires dragging far past where the card visually lands. Closest-center
* against the dragged card's projected rect is the model sortable lists use, and
* it keeps every slot reachable regardless of neighbour heights.
*/
function getWorktreeSidebarClosestCenterDropIndex(args: {
referenceY: number
rects: readonly WorktreeSidebarDragRect[]
activeIndex: number
}): number {
let overIndex = args.rects[0]!.groupIndex
let bestDistance = Number.POSITIVE_INFINITY
for (const rect of args.rects) {
const distance = Math.abs((rect.top + rect.bottom) / 2 - args.referenceY)
if (distance < bestDistance) {
bestDistance = distance
overIndex = rect.groupIndex
}
}
// Dropping onto a slot below the dragged card means landing after it.
return overIndex > args.activeIndex ? overIndex + 1 : overIndex
}
function getWorktreeSidebarPointerDropIndex(args: {
referenceY: number
rects: readonly WorktreeSidebarDragRect[]
}): number {
for (const rect of args.rects) {
if (args.referenceY < (rect.top + rect.bottom) / 2) {
return rect.groupIndex
}
}
return args.rects.at(-1)!.groupIndex + 1
}
export function computeWorktreeSidebarDropPreview(args: {
pointerY: number
containerTop: number
scrollTop: number
rects: readonly WorktreeSidebarDragRect[]
// Why: `rects` are held stable so resizing cards cannot move the drop target
// under a still pointer. The indicator and row previews still draw from live
// geometry, so a card growing mid-drag does not strand them at stale tops.
liveRects?: readonly WorktreeSidebarDragRect[]
groupIds: readonly string[]
draggedIds: readonly string[]
draggingWorktreeId?: string | null
// Where the card was grabbed, so the drop follows the card rather than the bare
// pointer. Omitted for native HTML5 drags, which have no reliable grab offset.
grab?: WorktreeSidebarDragGrab | null
// A held decision from the previous frame; honoured while the pointer is still
// so a resizing card cannot move the target under it.
anchor?: WorktreeSidebarDropAnchor | null
}): WorktreeSidebarDropPreview | null {
const rects = getWorktreeSidebarDragUnitRects({
rects: args.rects,
@ -124,14 +192,18 @@ export function computeWorktreeSidebarDropPreview(args: {
if (rects.length === 0 || args.groupIds.length === 0) {
return null
}
const liveUnitRects = args.liveRects
? getWorktreeSidebarDragUnitRects({ rects: args.liveRects, groupIds: args.groupIds })
: rects
// Why: an empty live measurement (rows unmounted by virtualization) would
// collapse the indicator to the top of the list; keep the held geometry then.
const renderRects = liveUnitRects.length === rects.length ? liveUnitRects : rects
const localY = args.pointerY - args.containerTop + args.scrollTop
const activeIndex = args.draggingWorktreeId
? rects.findIndex((rect) => rect.worktreeId === args.draggingWorktreeId)
: -1
const activeRect = activeIndex >= 0 ? rects[activeIndex]! : null
const referenceY = getWorktreeSidebarDragReferenceY({
localY,
grab: args.grab ?? null,
activeRect
})
const first = rects[0]!
const last = rects.at(-1)!
const boundaryDrop = getWorktreeSidebarBoundaryDrop({
@ -144,25 +216,36 @@ export function computeWorktreeSidebarDropPreview(args: {
return null
}
let dropIndex = last.groupIndex + 1
if (boundaryDrop.kind === 'drop') {
const heldIndex = args.anchor
? resolveWorktreeSidebarDropAnchorIndex({ anchor: args.anchor, rects })
: null
let dropIndex: number
if (heldIndex !== null) {
dropIndex = heldIndex
} else if (boundaryDrop.kind === 'drop') {
dropIndex = boundaryDrop.dropIndex
} else if (activeRect) {
dropIndex = getWorktreeSidebarClosestCenterDropIndex({ referenceY, rects, activeIndex })
} else {
for (const rect of rects) {
const mid = (rect.top + rect.bottom) / 2
if (localY < mid) {
dropIndex = rect.groupIndex
break
}
}
dropIndex = getWorktreeSidebarPointerDropIndex({ referenceY, rects })
}
const indicatorY = getWorktreeSidebarDropIndicatorY({ rects: renderRects, dropIndex })
const previewOffsetsByWorktreeId = buildWorktreeDragPreviewOffsets({
const { offsets, placeholderTop } = buildWorktreeDragPreviewOffsets({
groupIds: args.groupIds,
draggedIds: args.draggedIds,
draggingWorktreeId: args.draggingWorktreeId,
dropIndex,
rects: renderRects
rects
})
return { dropIndex, dropIndicatorY: indicatorY, previewOffsetsByWorktreeId }
return {
dropIndex,
dropIndicatorY: getWorktreeSidebarDropIndicatorY({
rects,
dropIndex,
placeholderTop,
activeRect
}),
previewOffsetsByWorktreeId: offsets,
dropAnchorId: getWorktreeSidebarDropAnchorId({ rects, dropIndex })
}
}

View File

@ -69,7 +69,7 @@ export function createSidebarDragPreview(args: {
pointerX: number
pointerY: number
draggedCount: number
}): { preview: HTMLElement; offsetX: number; offsetY: number } {
}): { preview: HTMLElement; offsetX: number; offsetY: number; height: number } {
const rect = args.sourceRow.getBoundingClientRect()
const preview = document.createElement('div')
const clone = args.sourceRow.cloneNode(true) as HTMLElement
@ -103,5 +103,5 @@ export function createSidebarDragPreview(args: {
offsetY
})
document.body.appendChild(preview)
return { preview, offsetX, offsetY }
return { preview, offsetX, offsetY, height: rect.height }
}