Fix Windows sidebar status drag commit (#4657)

This commit is contained in:
Jinwoo Hong
2026-06-04 18:02:37 -04:00
committed by GitHub
parent 1eb3c2d031
commit e3c8a4ccbd
3 changed files with 279 additions and 27 deletions
@@ -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(() => {
@@ -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<string, number>()
}
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
})
})
})
@@ -10,6 +10,47 @@ export type WorktreeSidebarDropPreview = {
previewOffsetsByWorktreeId: ReadonlyMap<string, number>
}
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