fix(sidebar): stop worktree drag from spazzing when cards resize mid-drag (#10725)

This commit is contained in:
Neil 2026-07-26 01:46:02 -07:00 committed by GitHub
parent 4326707285
commit 6c5af95343
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 240 additions and 14 deletions

View File

@ -194,6 +194,7 @@ import {
type WorktreeSidebarDragSession,
type WorktreeSidebarDragPoint
} from './worktree-sidebar-drag-autoscroll'
import { holdWorktreeSidebarDragRects } from './worktree-sidebar-drag-geometry'
import {
computeWorktreeSidebarDropPreview,
resolveWorktreeSidebarStatusDropCommitTarget,
@ -1404,6 +1405,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
[worktreeLineageById, worktreeMap]
)
const worktreeDragSessionRef = useRef<WorktreeSidebarDragSession | null>(null)
const heldStatusDropRectsRef = useRef<Map<string, readonly WorktreeSidebarDragRect[]>>(new Map())
const worktreePointerDragRef = useRef<WorktreePointerDrag | null>(null)
const worktreePointerAutoscrollFrameIdRef = useRef<number | null>(null)
const worktreePointerAutoscrollLastFrameTimeRef = useRef<number | null>(null)
@ -1593,6 +1595,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
pointerY: number
groupKey: string
rects: readonly WorktreeSidebarDragRect[]
liveRects?: readonly WorktreeSidebarDragRect[]
draggedIds: readonly string[]
draggingWorktreeId?: string | null
}): WorktreeSidebarDropPreview | null => {
@ -1610,6 +1613,7 @@ 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
@ -1627,6 +1631,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
pointerY,
groupKey: session.sourceGroupKey,
rects: session.rects,
liveRects: session.liveRects,
draggedIds: session.reorderUnitDraggedIds,
draggingWorktreeId: session.draggingWorktreeId
})
@ -1644,10 +1649,20 @@ 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
})
heldStatusDropRectsRef.current.set(groupKey, rects)
return computeWorktreeDropForGroup({
pointerY: args.pointerY,
groupKey,
rects: getWorktreeSidebarDragRectsForGroup(container, groupKey),
rects,
liveRects,
draggedIds: args.draggedIds,
draggingWorktreeId: worktreeDragSessionRef.current?.draggingWorktreeId ?? null
})
@ -2657,6 +2672,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
cleanupWorktreePointerDrag()
cancelWorktreeNativeAutoscroll()
worktreeDragSessionRef.current = null
heldStatusDropRectsRef.current.clear()
setWorktreeDragState(WORKTREE_ROW_DRAG_INITIAL_STATE)
}, [cancelWorktreeNativeAutoscroll, cleanupWorktreePointerDrag])
@ -3078,7 +3094,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
draggedIds: drag.draggedIds,
reorderDraggedIds: drag.reorderDraggedIds,
reorderUnitDraggedIds: drag.reorderUnitDraggedIds,
rects: drag.rects
rects: drag.rects,
liveRects: drag.rects
}
setWorktreeDragState({
draggingWorktreeId: drag.worktreeId,
@ -3493,15 +3510,17 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
}
const reorderDraggedIds = getReorderDraggedIds(draggedIds)
const reorderUnitDraggedIds = getReorderUnitDraggedIds(sourceGroupKey, reorderDraggedIds)
const rects = scrollRef.current
? getWorktreeSidebarDragRectsForGroup(scrollRef.current, sourceGroupKey)
: []
worktreeDragSessionRef.current = {
draggingWorktreeId: worktreeId,
sourceGroupKey,
draggedIds,
reorderDraggedIds,
reorderUnitDraggedIds,
rects: scrollRef.current
? getWorktreeSidebarDragRectsForGroup(scrollRef.current, sourceGroupKey)
: []
rects,
liveRects: rects
}
setWorktreeDragState({
draggingWorktreeId: worktreeId,

View File

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

View File

@ -1,5 +1,6 @@
import type { WorktreeDragGroup } from './worktree-manual-order'
import type { WorktreeDragUnitGroup } from './worktree-drag-units'
import { holdWorktreeSidebarDragRects } from './worktree-sidebar-drag-geometry'
const EDGE_ZONE_PX = 56
const MAX_OUTSIDE_EDGE_PX = 48
@ -25,7 +26,11 @@ 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.
rects: readonly WorktreeSidebarDragRect[]
liveRects: readonly WorktreeSidebarDragRect[]
}
export type WorktreeSidebarAutoscrollResult = {
@ -190,7 +195,11 @@ export function refreshWorktreeSidebarDragSession(args: {
return null
}
return { ...args.session, rects: args.rects }
return {
...args.session,
rects: holdWorktreeSidebarDragRects({ held: args.session.rects, measured: args.rects }),
liveRects: args.rects
}
}
function getVerticalEdgeIntensity(

View File

@ -0,0 +1,142 @@
import { describe, expect, it } from 'vitest'
import { computeWorktreeSidebarDropPreview } from './worktree-sidebar-drop-preview'
import { holdWorktreeSidebarDragRects } from './worktree-sidebar-drag-geometry'
import {
refreshWorktreeSidebarDragSession,
type WorktreeSidebarDragRect
} from './worktree-sidebar-drag-autoscroll'
const GROUP_IDS = ['a', 'b', 'c', 'd', 'e']
const CARD_HEIGHT = 116
const ROW_GAP = 6
// Why: a card with several agent rows expanded runs ~3.5x a collapsed one.
const EXPANDED_CARD_HEIGHT = 404
function layout(heightByWorktreeId: Readonly<Record<string, number>>): WorktreeSidebarDragRect[] {
let top = 0
return GROUP_IDS.map((worktreeId, groupIndex) => {
const height = heightByWorktreeId[worktreeId] ?? CARD_HEIGHT
const rect = { worktreeId, groupIndex, top, bottom: top + height }
top += height + ROW_GAP
return rect
})
}
const COLLAPSED = layout({})
function previewAt(args: {
pointerY: number
rects: readonly WorktreeSidebarDragRect[]
liveRects?: readonly WorktreeSidebarDragRect[]
}): { dropIndex: number; dropIndicatorY: number } | null {
const preview = computeWorktreeSidebarDropPreview({
pointerY: args.pointerY,
containerTop: 0,
scrollTop: 0,
rects: args.rects,
liveRects: args.liveRects,
groupIds: GROUP_IDS,
draggedIds: ['a'],
draggingWorktreeId: 'a'
})
return preview ? { dropIndex: preview.dropIndex, dropIndicatorY: preview.dropIndicatorY } : null
}
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)
})
it('never lets a growing card change the drop target across a whole expansion animation', () => {
const pointerY = 250
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
)
expect(new Set(live).size).toBeGreaterThan(1)
expect(new Set(stabilized)).toEqual(
new Set([previewAt({ pointerY, rects: COLLAPSED })?.dropIndex])
)
})
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('draws the indicator at the live position so a grown card does not strand it', () => {
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)
})
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]!
}))
expect(holdWorktreeSidebarDragRects({ held: COLLAPSED, measured: reordered })).toBe(reordered)
expect(holdWorktreeSidebarDragRects({ held: undefined, measured: COLLAPSED })).toBe(COLLAPSED)
expect(holdWorktreeSidebarDragRects({ held: [], measured: COLLAPSED })).toBe(COLLAPSED)
})
it('holds hit-test geometry across a session refresh while liveRects stay current', () => {
const grown = layout({ b: EXPANDED_CARD_HEIGHT })
const refreshed = refreshWorktreeSidebarDragSession({
session: {
draggingWorktreeId: 'a',
sourceGroupKey: 'repo:one',
draggedIds: ['a'],
reorderDraggedIds: ['a'],
reorderUnitDraggedIds: ['a'],
rects: COLLAPSED,
liveRects: COLLAPSED
},
groups: [{ key: 'repo:one', worktreeIds: GROUP_IDS }],
unitGroups: [
{
key: 'repo:one',
worktreeIds: GROUP_IDS,
units: GROUP_IDS.map((worktreeId) => ({ worktreeId, worktreeIds: [worktreeId] }))
}
],
rects: grown
})
expect(refreshed?.rects).toBe(COLLAPSED)
expect(refreshed?.liveRects).toBe(grown)
})
})

View File

@ -0,0 +1,34 @@
import type { WorktreeSidebarDragRect } from './worktree-sidebar-drag-autoscroll'
function getDragRowSignature(rects: readonly WorktreeSidebarDragRect[]): string {
return rects.map((rect) => `${rect.worktreeId}@${rect.groupIndex}`).join('|')
}
/**
* 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.
*
* 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.
*/
export function holdWorktreeSidebarDragRects(args: {
held: readonly WorktreeSidebarDragRect[] | undefined
measured: readonly WorktreeSidebarDragRect[]
}): readonly WorktreeSidebarDragRect[] {
if (!args.held || args.held.length === 0) {
return args.measured
}
return getDragRowSignature(args.held) === getDragRowSignature(args.measured)
? args.held
: args.measured
}

View File

@ -92,11 +92,27 @@ export function resolveWorktreeSidebarStatusDropCommitTarget(args: {
: { target: args.currentTarget, preview: args.currentPreview }
}
function getWorktreeSidebarDropIndicatorY(args: {
rects: readonly WorktreeSidebarDragRect[]
dropIndex: number
}): number {
const target = args.rects.find((rect) => rect.groupIndex === args.dropIndex)
if (target) {
return Math.max(0, target.top - 3)
}
const last = args.rects.at(-1)
return last ? last.bottom + 3 : 0
}
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
@ -108,6 +124,12 @@ 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 first = rects[0]!
@ -123,26 +145,24 @@ export function computeWorktreeSidebarDropPreview(args: {
}
let dropIndex = last.groupIndex + 1
let indicatorY = last.bottom + 3
if (boundaryDrop.kind === 'drop') {
dropIndex = boundaryDrop.dropIndex
indicatorY = boundaryDrop.indicatorY
} else {
for (const rect of rects) {
const mid = (rect.top + rect.bottom) / 2
if (localY < mid) {
dropIndex = rect.groupIndex
indicatorY = Math.max(0, rect.top - 3)
break
}
}
}
const indicatorY = getWorktreeSidebarDropIndicatorY({ rects: renderRects, dropIndex })
const previewOffsetsByWorktreeId = buildWorktreeDragPreviewOffsets({
groupIds: args.groupIds,
draggedIds: args.draggedIds,
draggingWorktreeId: args.draggingWorktreeId,
dropIndex,
rects
rects: renderRects
})
return { dropIndex, dropIndicatorY: indicatorY, previewOffsetsByWorktreeId }
}