From e3c8a4ccbd392c250bcad169830b64a8222ec0eb Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Thu, 4 Jun 2026 18:02:37 -0400 Subject: [PATCH] Fix Windows sidebar status drag commit (#4657) --- .../src/components/sidebar/WorktreeList.tsx | 192 +++++++++++++++--- .../worktree-sidebar-drop-preview.test.ts | 73 ++++++- .../sidebar/worktree-sidebar-drop-preview.ts | 41 ++++ 3 files changed, 279 insertions(+), 27 deletions(-) diff --git a/src/renderer/src/components/sidebar/WorktreeList.tsx b/src/renderer/src/components/sidebar/WorktreeList.tsx index 034b709a0af..871a68cbb6f 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.tsx @@ -163,6 +163,9 @@ import { } from './worktree-sidebar-drag-autoscroll' import { computeWorktreeSidebarDropPreview, + resolveWorktreeSidebarStatusDropCommitTarget, + type WorktreeSidebarStatusDropTarget, + type WorktreeSidebarTrackedStatusDropTarget, type WorktreeSidebarDropPreview } from './worktree-sidebar-drop-preview' import { resolveProjectGroupHeaderColor } from './project-header-color' @@ -497,6 +500,7 @@ type WorktreePointerDrag = { previewOffsetY: number frameId: number | null latestBoardDropTarget: WorkspaceKanbanCardTrackedDropTarget | null + latestStatusDropTarget: WorktreeSidebarTrackedStatusDropTarget | null } function areWorktreeDragPreviewOffsetsEqual( @@ -517,15 +521,32 @@ function areWorktreeDragPreviewOffsetsEqual( return true } +function updateLatestWorktreeStatusDropTarget( + drag: WorktreePointerDrag, + target: WorktreeSidebarStatusDropTarget, + preview: WorktreeSidebarDropPreview | null +): void { + drag.latestStatusDropTarget = + target.status || target.isPinDrop + ? { + target, + preview, + x: drag.currentX, + y: drag.currentY + } + : null +} + function getWorktreeVirtualRowTransform(start: number, previewOffset: number): string { const base = getVirtualRowTransform(start) return previewOffset === 0 ? base : `${base} translateY(${previewOffset}px)` } -function getPointerDropStatusTarget(args: { container: HTMLElement; x: number; y: number }): { - status: WorkspaceStatus | null - isPinDrop: boolean -} { +function getPointerDropStatusTarget(args: { + container: HTMLElement + x: number + y: number +}): WorktreeSidebarStatusDropTarget { const target = document.elementFromPoint(args.x, args.y) if (!(target instanceof Element) || !args.container.contains(target)) { return { status: null, isPinDrop: false } @@ -544,6 +565,23 @@ function getPointerDropStatusTarget(args: { container: HTMLElement; x: number; y } } +function shouldPreferSidebarStatusDropTarget(args: { + sourceGroupKey: string + target: WorktreeSidebarStatusDropTarget + workspaceStatuses: readonly WorkspaceStatusDefinition[] +}): boolean { + if (args.target.isPinDrop) { + return true + } + if (!args.target.status) { + return false + } + const sourceStatus = getWorkspaceStatusFromGroupKey(args.sourceGroupKey, args.workspaceStatuses) + // Why: source-group edge zones overlap adjacent status sections; the visible + // section under the pointer must win so the guide and committed drop agree. + return sourceStatus !== null && args.target.status !== sourceStatus +} + function isWorktreeItemRow(row: Row): row is WorktreeItemRow { return row.type === 'item' } @@ -1611,6 +1649,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp y: drag.currentY } if (boardTarget.status || boardTarget.isPinDrop) { + drag.latestStatusDropTarget = null setDragOverStatus(null) setPinDragOver(false) setWorktreeDragState((prev) => @@ -1630,24 +1669,30 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp return } - const drop = computeWorktreeDrop(drag.currentY) const sidebarContainer = scrollRef.current - if (!drop) { - const target = sidebarContainer - ? getPointerDropStatusTarget({ - container: sidebarContainer, - x: drag.currentX, - y: drag.currentY - }) - : { status: null, isPinDrop: false } - const statusDrop = target.status + const preferredStatusTarget = sidebarContainer + ? getPointerDropStatusTarget({ + container: sidebarContainer, + x: drag.currentX, + y: drag.currentY + }) + : { status: null, isPinDrop: false } + if ( + shouldPreferSidebarStatusDropTarget({ + sourceGroupKey: drag.sourceGroupKey, + target: preferredStatusTarget, + workspaceStatuses + }) + ) { + const statusDrop = preferredStatusTarget.status ? computeWorktreeStatusDrop({ pointerY: drag.currentY, - status: target.status, + status: preferredStatusTarget.status, draggedIds: drag.draggedIds }) : null if (statusDrop) { + updateLatestWorktreeStatusDropTarget(drag, preferredStatusTarget, statusDrop) clearWorkspaceKanbanSidebarDropTargetVisual() setDragOverStatus(null) setPinDragOver(false) @@ -1664,6 +1709,55 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp ) return } + updateLatestWorktreeStatusDropTarget(drag, preferredStatusTarget, statusDrop) + setDragOverStatus(preferredStatusTarget.status) + setPinDragOver(preferredStatusTarget.isPinDrop) + setWorktreeDragState((prev) => + prev.dropIndex === null && + prev.dropIndicatorY === null && + prev.pointerY === drag.currentY && + prev.previewOffsetsByWorktreeId.size === 0 + ? prev + : { + ...prev, + dropIndex: null, + dropIndicatorY: null, + previewOffsetsByWorktreeId: EMPTY_WORKTREE_DRAG_PREVIEW_OFFSETS, + pointerY: drag.currentY + } + ) + return + } + + const drop = computeWorktreeDrop(drag.currentY) + if (!drop) { + const target = preferredStatusTarget + const statusDrop = target.status + ? computeWorktreeStatusDrop({ + pointerY: drag.currentY, + status: target.status, + draggedIds: drag.draggedIds + }) + : null + if (statusDrop) { + updateLatestWorktreeStatusDropTarget(drag, target, statusDrop) + clearWorkspaceKanbanSidebarDropTargetVisual() + setDragOverStatus(null) + setPinDragOver(false) + setWorktreeDragState((prev) => + prev.dropIndex === statusDrop.dropIndex && + prev.dropIndicatorY === statusDrop.dropIndicatorY && + prev.pointerY === drag.currentY && + areWorktreeDragPreviewOffsetsEqual( + prev.previewOffsetsByWorktreeId, + statusDrop.previewOffsetsByWorktreeId + ) + ? prev + : { ...prev, ...statusDrop, pointerY: drag.currentY } + ) + return + } + updateLatestWorktreeStatusDropTarget(drag, target, statusDrop) setDragOverStatus(target.status) setPinDragOver(target.isPinDrop) setWorktreeDragState((prev) => @@ -1682,6 +1776,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp ) return } + drag.latestStatusDropTarget = null clearWorkspaceKanbanSidebarDropTargetVisual() setDragOverStatus(null) setPinDragOver(false) @@ -1701,7 +1796,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp computeWorktreeDrop, computeWorktreeStatusDrop, refreshWorktreeDragSession, - shouldShowWorkspaceBoardDropIndicator + shouldShowWorkspaceBoardDropIndicator, + workspaceStatuses ]) const scheduleWorktreePointerDragFrame = useCallback( @@ -1846,7 +1942,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp previewOffsetX: 0, previewOffsetY: 0, frameId: null, - latestBoardDropTarget: null + latestBoardDropTarget: null, + latestStatusDropTarget: null } }, [ @@ -1919,6 +2016,44 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp groups: getWorkspaceKanbanSidebarDropGroups() }) } else { + const preferredStatusTarget = scrollRef.current + ? getPointerDropStatusTarget({ + container: scrollRef.current, + x: event.clientX, + y: event.clientY + }) + : { status: null, isPinDrop: false } + if ( + shouldPreferSidebarStatusDropTarget({ + sourceGroupKey: drag.sourceGroupKey, + target: preferredStatusTarget, + workspaceStatuses + }) + ) { + const statusDrop = preferredStatusTarget.status + ? computeWorktreeStatusDrop({ + pointerY: event.clientY, + status: preferredStatusTarget.status, + draggedIds: drag.draggedIds + }) + : null + if (preferredStatusTarget.isPinDrop) { + onPinWorktrees(drag.draggedIds) + } else if (preferredStatusTarget.status) { + if (statusDrop) { + onMoveWorktreesToStatusAtIndex({ + worktreeIds: drag.draggedIds, + status: preferredStatusTarget.status, + dropIndex: statusDrop.dropIndex, + groups: worktreeDragGroups + }) + } else { + onMoveWorktreesToStatus(drag.draggedIds, preferredStatusTarget.status) + } + } + clearWorktreeDrag() + return + } const drop = computeWorktreeDrop(event.clientY) if (drop) { onReorderWorktrees({ @@ -1932,20 +2067,24 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp }) }) } else if (scrollRef.current) { - const container = scrollRef.current - const target = getPointerDropStatusTarget({ - container, + const currentTarget = preferredStatusTarget + const currentPreview = currentTarget.status + ? computeWorktreeStatusDrop({ + pointerY: event.clientY, + status: currentTarget.status, + draggedIds: drag.draggedIds + }) + : null + const { target, preview: statusDrop } = resolveWorktreeSidebarStatusDropCommitTarget({ + currentTarget, + currentPreview, + latestTrackedTarget: drag.latestStatusDropTarget, x: event.clientX, y: event.clientY }) if (target.isPinDrop) { onPinWorktrees(drag.draggedIds) } else if (target.status) { - const statusDrop = computeWorktreeStatusDrop({ - pointerY: event.clientY, - status: target.status, - draggedIds: drag.draggedIds - }) if (statusDrop) { onMoveWorktreesToStatusAtIndex({ worktreeIds: drag.draggedIds, @@ -1992,7 +2131,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp scheduleWorktreePointerDragFrame, shouldShowWorkspaceBoardDropIndicator, worktreeDragGroups, - worktreeDragUnitGroups + worktreeDragUnitGroups, + workspaceStatuses ]) useEffect(() => { diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.test.ts b/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.test.ts index 7ff3e4a1742..6b42038074c 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.test.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest' -import { computeWorktreeSidebarDropPreview } from './worktree-sidebar-drop-preview' +import { + computeWorktreeSidebarDropPreview, + resolveWorktreeSidebarStatusDropCommitTarget +} from './worktree-sidebar-drop-preview' const rects = [ { worktreeId: 'done-a', groupIndex: 0, top: 80, bottom: 120 }, @@ -36,3 +39,71 @@ describe('computeWorktreeSidebarDropPreview', () => { ).toBeNull() }) }) + +describe('resolveWorktreeSidebarStatusDropCommitTarget', () => { + const preview = { + dropIndex: 1, + dropIndicatorY: 129, + previewOffsetsByWorktreeId: new Map() + } + + it('uses the current status target when pointerup hit-testing succeeds', () => { + expect( + resolveWorktreeSidebarStatusDropCommitTarget({ + currentTarget: { status: 'completed', isPinDrop: false }, + currentPreview: preview, + latestTrackedTarget: { + target: { status: 'in-progress', isPinDrop: false }, + preview: null, + x: 100, + y: 100 + }, + x: 100, + y: 100 + }) + ).toEqual({ + target: { status: 'completed', isPinDrop: false }, + preview + }) + }) + + it('reuses the latest status target when pointerup hit-testing blanks at the same point', () => { + expect( + resolveWorktreeSidebarStatusDropCommitTarget({ + currentTarget: { status: null, isPinDrop: false }, + currentPreview: null, + latestTrackedTarget: { + target: { status: 'completed', isPinDrop: false }, + preview, + x: 100, + y: 100 + }, + x: 102, + y: 101 + }) + ).toEqual({ + target: { status: 'completed', isPinDrop: false }, + preview + }) + }) + + it('does not reuse a stale status target after the pointer has moved away', () => { + expect( + resolveWorktreeSidebarStatusDropCommitTarget({ + currentTarget: { status: null, isPinDrop: false }, + currentPreview: null, + latestTrackedTarget: { + target: { status: 'completed', isPinDrop: false }, + preview, + x: 100, + y: 100 + }, + x: 140, + y: 100 + }) + ).toEqual({ + target: { status: null, isPinDrop: false }, + preview: null + }) + }) +}) 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 e6fed4fb738..703b1378114 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-drop-preview.ts @@ -10,6 +10,47 @@ export type WorktreeSidebarDropPreview = { previewOffsetsByWorktreeId: ReadonlyMap } +export type WorktreeSidebarStatusDropTarget = { + status: string | null + isPinDrop: boolean +} + +export type WorktreeSidebarTrackedStatusDropTarget = { + target: WorktreeSidebarStatusDropTarget + preview: WorktreeSidebarDropPreview | null + x: number + y: number +} + +const STATUS_DROP_TARGET_FALLBACK_TOLERANCE_PX = 6 + +function hasWorktreeSidebarStatusDropTarget(target: WorktreeSidebarStatusDropTarget): boolean { + return target.isPinDrop || target.status !== null +} + +export function resolveWorktreeSidebarStatusDropCommitTarget(args: { + currentTarget: WorktreeSidebarStatusDropTarget + currentPreview: WorktreeSidebarDropPreview | null + latestTrackedTarget: WorktreeSidebarTrackedStatusDropTarget | null + x: number + y: number +}): { + target: WorktreeSidebarStatusDropTarget + preview: WorktreeSidebarDropPreview | null +} { + if (hasWorktreeSidebarStatusDropTarget(args.currentTarget)) { + return { target: args.currentTarget, preview: args.currentPreview } + } + const latest = args.latestTrackedTarget + if (!latest || !hasWorktreeSidebarStatusDropTarget(latest.target)) { + return { target: args.currentTarget, preview: args.currentPreview } + } + const distance = Math.hypot(args.x - latest.x, args.y - latest.y) + return distance <= STATUS_DROP_TARGET_FALLBACK_TOLERANCE_PX + ? { target: latest.target, preview: latest.preview } + : { target: args.currentTarget, preview: args.currentPreview } +} + export function computeWorktreeSidebarDropPreview(args: { pointerY: number containerTop: number