From 6c5af95343a397bf1d1f98923b2f39d9db6ea3a6 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:46:02 -0700 Subject: [PATCH] fix(sidebar): stop worktree drag from spazzing when cards resize mid-drag (#10725) --- .../src/components/sidebar/WorktreeList.tsx | 29 +++- .../worktree-sidebar-drag-autoscroll.test.ts | 10 +- .../worktree-sidebar-drag-autoscroll.ts | 11 +- .../worktree-sidebar-drag-geometry.test.ts | 142 ++++++++++++++++++ .../sidebar/worktree-sidebar-drag-geometry.ts | 34 +++++ .../sidebar/worktree-sidebar-drop-preview.ts | 28 +++- 6 files changed, 240 insertions(+), 14 deletions(-) create mode 100644 src/renderer/src/components/sidebar/worktree-sidebar-drag-geometry.test.ts create mode 100644 src/renderer/src/components/sidebar/worktree-sidebar-drag-geometry.ts diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 857f8d7f9..c85e035b2 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -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(null) + const heldStatusDropRectsRef = useRef>(new Map()) const worktreePointerDragRef = useRef(null) const worktreePointerAutoscrollFrameIdRef = useRef(null) const worktreePointerAutoscrollLastFrameTimeRef = useRef(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, diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-drag-autoscroll.test.ts b/src/renderer/src/components/sidebar/worktree-sidebar-drag-autoscroll.test.ts index a197759a2..f26d5ef6e 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-drag-autoscroll.test.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-drag-autoscroll.test.ts @@ -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 }) }) }) diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-drag-autoscroll.ts b/src/renderer/src/components/sidebar/worktree-sidebar-drag-autoscroll.ts index 27e2a3d7d..1dd0b5d69 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-drag-autoscroll.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-drag-autoscroll.ts @@ -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( diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-drag-geometry.test.ts b/src/renderer/src/components/sidebar/worktree-sidebar-drag-geometry.test.ts new file mode 100644 index 000000000..c783d5325 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-sidebar-drag-geometry.test.ts @@ -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>): 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) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-drag-geometry.ts b/src/renderer/src/components/sidebar/worktree-sidebar-drag-geometry.ts new file mode 100644 index 000000000..77bd680bc --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-sidebar-drag-geometry.ts @@ -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 +} diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.ts b/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.ts index d3702968e..dfb7a825a 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.ts @@ -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 } }