diff --git a/src/renderer/src/components/sidebar/project-group-header-drag.ts b/src/renderer/src/components/sidebar/project-group-header-drag.ts index 63c2a723d..e2a879aeb 100644 --- a/src/renderer/src/components/sidebar/project-group-header-drag.ts +++ b/src/renderer/src/components/sidebar/project-group-header-drag.ts @@ -67,7 +67,8 @@ export function useProjectGroupHeaderDrag({ containerTop: containerRect.top, scrollTop: container.scrollTop, rects: session.headerRects, - sidebarProjectGroupHeaderIds: session.sidebarProjectGroupHeaderIds + sidebarProjectGroupHeaderIds: session.sidebarProjectGroupHeaderIds, + contentBottom: container.scrollHeight }) }, [] diff --git a/src/renderer/src/components/sidebar/project-group-header-drop.test.ts b/src/renderer/src/components/sidebar/project-group-header-drop.test.ts index 58bd015df..8fa20c47b 100644 --- a/src/renderer/src/components/sidebar/project-group-header-drop.test.ts +++ b/src/renderer/src/components/sidebar/project-group-header-drop.test.ts @@ -137,7 +137,9 @@ describe('computeProjectGroupHeaderDropPreview', () => { expect(preview).toEqual({ dropIndex: 1, dropIndicatorY: 96 }) }) - it('does not create a drop slot inside an expanded Project Group section', () => { + it('snaps a drop inside the last expanded Project Group section to its bottom boundary', () => { + const INDICATOR_GAP = 4 + const sectionBottom = 380 const preview = computeProjectGroupHeaderDropPreview({ pointerY: 350, containerTop: 0, @@ -150,14 +152,83 @@ describe('computeProjectGroupHeaderDropPreview', () => { headerIndex: 2, top: 300, bottom: 328, - sectionBottom: 380 + sectionBottom } ] }) + // Only boundary available is 'c's section bottom → drop after 'c' (slot 3). + expect(preview).toEqual({ dropIndex: 3, dropIndicatorY: sectionBottom + INDICATOR_GAP }) + }) + + it('rejects a drop below the measured content when the estimated section overshoots', () => { + const preview = computeProjectGroupHeaderDropPreview({ + pointerY: 360, + containerTop: 0, + scrollTop: 0, + sidebarProjectGroupHeaderIds: ['a', 'b', 'c'], + rects: [ + { + groupId: 'c', + bucketKey: 'root', + headerIndex: 2, + top: 300, + bottom: 328, + sectionBottom: 380 + } + ], + // Estimate ends at 380 but the list renders to 340; 360 is below content. + contentBottom: 340 + }) + expect(preview).toBeNull() }) + it('rejects an edge-zone final drop below measured content when the estimate overshoots', () => { + const preview = computeProjectGroupHeaderDropPreview({ + pointerY: 389, + containerTop: 0, + scrollTop: 0, + sidebarProjectGroupHeaderIds: ['a', 'b', 'c'], + rects: [ + { + groupId: 'c', + bucketKey: 'root', + headerIndex: 2, + top: 300, + bottom: 328, + sectionBottom: 380 + } + ], + // 389 crosses the estimated final boundary but remains below real content. + contentBottom: 350 + }) + + expect(preview).toBeNull() + }) + + it('snaps a within-content drop even when the estimated section overshoots', () => { + const preview = computeProjectGroupHeaderDropPreview({ + pointerY: 335, + containerTop: 0, + scrollTop: 0, + sidebarProjectGroupHeaderIds: ['a', 'b', 'c'], + rects: [ + { + groupId: 'c', + bucketKey: 'root', + headerIndex: 2, + top: 300, + bottom: 328, + sectionBottom: 380 + } + ], + contentBottom: 340 + }) + + expect(preview).toEqual({ dropIndex: 3, dropIndicatorY: 384 }) + }) + it('uses the whole Project Group section for the final boundary slot', () => { const preview = computeProjectGroupHeaderDropPreview({ pointerY: 400, diff --git a/src/renderer/src/components/sidebar/project-group-header-drop.ts b/src/renderer/src/components/sidebar/project-group-header-drop.ts index 512b63671..813823727 100644 --- a/src/renderer/src/components/sidebar/project-group-header-drop.ts +++ b/src/renderer/src/components/sidebar/project-group-header-drop.ts @@ -184,6 +184,7 @@ export function computeProjectGroupHeaderDropPreview(args: { scrollTop: number rects: readonly ProjectGroupHeaderDragRect[] sidebarProjectGroupHeaderIds: readonly string[] + contentBottom?: number }): ProjectGroupHeaderDropPreview | null { const { rects, sidebarProjectGroupHeaderIds } = args return computeWorktreeSidebarHeaderDropPreview({ @@ -192,6 +193,7 @@ export function computeProjectGroupHeaderDropPreview(args: { scrollTop: args.scrollTop, rects, headerCount: sidebarProjectGroupHeaderIds.length, - getId: (rect) => rect.groupId + getId: (rect) => rect.groupId, + contentBottom: args.contentBottom }) } diff --git a/src/renderer/src/components/sidebar/project-header-drag.ts b/src/renderer/src/components/sidebar/project-header-drag.ts index e4fccc57c..4d13ddfa0 100644 --- a/src/renderer/src/components/sidebar/project-header-drag.ts +++ b/src/renderer/src/components/sidebar/project-header-drag.ts @@ -72,13 +72,13 @@ export function useRepoHeaderDrag({ if (!session || !container) { return null } - const containerRect = container.getBoundingClientRect() return computeProjectHeaderDropPreview({ pointerY, - containerTop: containerRect.top, + containerTop: container.getBoundingClientRect().top, scrollTop: container.scrollTop, rects: session.headerRects, - sidebarRepoHeaderIds: session.sidebarRepoHeaderIds + sidebarRepoHeaderIds: session.sidebarRepoHeaderIds, + contentBottom: container.scrollHeight }) }, [] diff --git a/src/renderer/src/components/sidebar/project-header-drop.test.ts b/src/renderer/src/components/sidebar/project-header-drop.test.ts index c943ca975..dee5572e0 100644 --- a/src/renderer/src/components/sidebar/project-header-drop.test.ts +++ b/src/renderer/src/components/sidebar/project-header-drop.test.ts @@ -139,7 +139,9 @@ describe('computeProjectHeaderDropPreview', () => { expect(preview).toEqual({ dropIndex: 3, dropIndicatorY: 383 }) }) - it('does not create a drop slot inside an expanded project section', () => { + it('snaps a drop inside the last expanded project section to its bottom boundary', () => { + const INDICATOR_GAP = 4 + const sectionBottom = 380 const preview = computeProjectHeaderDropPreview({ pointerY: 350, containerTop: 0, @@ -152,15 +154,18 @@ describe('computeProjectHeaderDropPreview', () => { headerIndex: 2, top: 300, bottom: 328, - sectionBottom: 380 + sectionBottom } ] }) - expect(preview).toBeNull() + // Only boundary available is 'c's section bottom → drop after 'c' (slot 3). + expect(preview).toEqual({ dropIndex: 3, dropIndicatorY: sectionBottom + INDICATOR_GAP }) }) - it('does not create a drop slot in the contents between sibling project headers', () => { + it('snaps a drop between sibling project headers to the nearer boundary', () => { + const INDICATOR_GAP = 4 + const nextHeaderTop = 220 const preview = computeProjectHeaderDropPreview({ pointerY: 150, containerTop: 0, @@ -173,13 +178,135 @@ describe('computeProjectHeaderDropPreview', () => { headerIndex: 0, top: 100, bottom: 128, - sectionBottom: 220 + sectionBottom: nextHeaderTop }, - { repoId: 'b', bucketKey: 'ungrouped', headerIndex: 1, top: 220, bottom: 248 } + { repoId: 'b', bucketKey: 'ungrouped', headerIndex: 1, top: nextHeaderTop, bottom: 248 } ] }) - expect(preview).toBeNull() + // pointerY 150 sits in 'a's body; nearer boundary is 'b's top (216 vs 224). + expect(preview).toEqual({ dropIndex: 1, dropIndicatorY: nextHeaderTop - INDICATOR_GAP }) + }) + + describe('nearest-boundary choice across an interior gap', () => { + // The gap models estimate-vs-actual drift: sectionBottom is an estimated + // row offset (worktree-header-section-boundaries.ts, no virtualizer gap:6 or + // measured sizes) while the next header's top is actual vItem.start geometry, + // so they diverge in tall sections. Near the real boundary the pointer snaps + // to actual geometry (beforeNext); the estimate governs only deep-body drops. + const INDICATOR_GAP = 4 + const prevSectionBottom = 200 + const nextHeaderTop = 240 + const sectionBottomSlotY = prevSectionBottom + INDICATOR_GAP // 204 + const nextHeaderSlotY = nextHeaderTop - INDICATOR_GAP // 236 + const midpointY = (sectionBottomSlotY + nextHeaderSlotY) / 2 // 220 + const gapRects = [ + { + repoId: 'a', + bucketKey: 'ungrouped', + headerIndex: 0, + top: 100, + bottom: 128, + sectionBottom: prevSectionBottom + }, + { + repoId: 'b', + bucketKey: 'ungrouped', + headerIndex: 1, + top: nextHeaderTop, + bottom: 268, + sectionBottom: 340 + } + ] as const + + it('snaps to the previous section bottom when the pointer is nearer to it', () => { + const preview = computeProjectHeaderDropPreview({ + pointerY: sectionBottomSlotY + 1, // 205, closer to 204 than 236 + containerTop: 0, + scrollTop: 0, + sidebarRepoHeaderIds: ['a', 'b'], + rects: gapRects.map((rect) => ({ ...rect })) + }) + + expect(preview).toEqual({ dropIndex: 1, dropIndicatorY: sectionBottomSlotY }) + }) + + it('snaps to the next header top when the pointer is nearer to it', () => { + const preview = computeProjectHeaderDropPreview({ + pointerY: nextHeaderSlotY - 1, // 235, closer to 236 than 204 + containerTop: 0, + scrollTop: 0, + sidebarRepoHeaderIds: ['a', 'b'], + rects: gapRects.map((rect) => ({ ...rect })) + }) + + expect(preview).toEqual({ dropIndex: 1, dropIndicatorY: nextHeaderSlotY }) + }) + + it('breaks the midpoint tie toward the next header boundary', () => { + const preview = computeProjectHeaderDropPreview({ + pointerY: midpointY, // 220, equidistant → next header wins + containerTop: 0, + scrollTop: 0, + sidebarRepoHeaderIds: ['a', 'b'], + rects: gapRects.map((rect) => ({ ...rect })) + }) + + expect(preview).toEqual({ dropIndex: 1, dropIndicatorY: nextHeaderSlotY }) + }) + }) + + describe('content bound for the last section', () => { + const INDICATOR_GAP = 4 + const estimatedSectionBottom = 380 + const lastRects = [ + { + repoId: 'c', + bucketKey: 'ungrouped', + headerIndex: 2, + top: 300, + bottom: 328, + sectionBottom: estimatedSectionBottom + } + ] as const + const lastPreview = (pointerY: number, contentBottom: number) => + computeProjectHeaderDropPreview({ + pointerY, + containerTop: 0, + scrollTop: 0, + sidebarRepoHeaderIds: ['a', 'b', 'c'], + rects: lastRects.map((rect) => ({ ...rect })), + contentBottom + }) + + it('rejects a drop below the measured content when the estimate overshoots', () => { + // Estimate says the section ends at 380, but the list actually renders to + // 340; a pointer at 360 is below real content → no fabricated final slot. + expect(lastPreview(360, 340)).toBeNull() + }) + + it('rejects an edge-zone final drop below measured content when the estimate overshoots', () => { + // 389 crosses the estimated final boundary (380 + 8px padding) while still + // sitting below the measured list end, so the content bound must win. + expect(lastPreview(389, 350)).toBeNull() + }) + + it('still snaps within the measured content when the estimate overshoots', () => { + // 335 is inside the real last section (ends at 340) → drop after 'c'. + expect(lastPreview(335, 340)).toEqual({ + dropIndex: 3, + dropIndicatorY: estimatedSectionBottom + INDICATOR_GAP + }) + }) + + it('snaps within the measured content when actual content undershoots the estimate', () => { + // Real content taller than the estimate (420 > 380): a 360 drop is well + // inside the section → still snaps to the final slot. + expect(lastPreview(360, 420)).toEqual({ + dropIndex: 3, + dropIndicatorY: estimatedSectionBottom + INDICATOR_GAP + }) + }) }) }) diff --git a/src/renderer/src/components/sidebar/project-header-drop.ts b/src/renderer/src/components/sidebar/project-header-drop.ts index 478b2b1b7..00b7169b9 100644 --- a/src/renderer/src/components/sidebar/project-header-drop.ts +++ b/src/renderer/src/components/sidebar/project-header-drop.ts @@ -184,6 +184,7 @@ export function computeProjectHeaderDropPreview(args: { scrollTop: number rects: readonly ProjectHeaderDragRect[] sidebarRepoHeaderIds: readonly string[] + contentBottom?: number }): ProjectHeaderDropPreview | null { const { rects, sidebarRepoHeaderIds } = args return computeWorktreeSidebarHeaderDropPreview({ @@ -192,7 +193,8 @@ export function computeProjectHeaderDropPreview(args: { scrollTop: args.scrollTop, rects, headerCount: sidebarRepoHeaderIds.length, - getId: (rect) => rect.repoId + getId: (rect) => rect.repoId, + contentBottom: args.contentBottom }) } diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-header-drop-preview.ts b/src/renderer/src/components/sidebar/worktree-sidebar-header-drop-preview.ts index 386a3e317..00dca75e4 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-header-drop-preview.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-header-drop-preview.ts @@ -23,12 +23,22 @@ export function computeWorktreeSidebarHeaderDropPreview< rects: readonly TRect[] headerCount: number getId: (rect: TRect) => string + // Measured scroll-content height (same coordinate space as localY and the + // virtualizer-derived rect tops). Bounds the interior snap so it cannot + // fabricate a slot below the real list end. Optional: geometry-only unit + // tests omit it; live drags always pass container.scrollHeight. + contentBottom?: number }): WorktreeSidebarHeaderDropPreview | null { if (args.rects.length === 0 || args.headerCount === 0) { return null } const localY = args.pointerY - args.containerTop + args.scrollTop + // Why: every preview branch, including estimated edge slots, must stay + // inside the measured list content rather than fabricate a reorder below it. + if (args.contentBottom !== undefined && localY > args.contentBottom) { + return null + } const first = args.rects[0]! const last = args.rects.at(-1)! const lastBoundaryBottom = Math.max(last.bottom, last.sectionBottom ?? last.bottom) @@ -59,21 +69,74 @@ export function computeWorktreeSidebarHeaderDropPreview< } const hoveredRect = args.rects.find((rect) => localY >= rect.top && localY <= rect.bottom) - if (!hoveredRect) { + if (hoveredRect) { + const mid = (hoveredRect.top + hoveredRect.bottom) / 2 + const dropIndex = localY < mid ? hoveredRect.headerIndex : hoveredRect.headerIndex + 1 + const nextRect = + localY < mid ? hoveredRect : args.rects.find((rect) => rect.headerIndex >= dropIndex) + const indicatorY = nextRect + ? Math.max(0, nextRect.top - INDICATOR_GAP_PX) + : Math.max(hoveredRect.bottom, hoveredRect.sectionBottom ?? hoveredRect.bottom) + + INDICATOR_GAP_PX + + return { + dropIndex, + dropIndicatorY: Math.max(args.scrollTop, indicatorY) + } + } + + // localY is in a section body or interior gap, not a header band. Snap to the + // nearer boundary slot instead of returning null: this interior dead zone was + // accidental scope of 22d5989ed (#6609 only required correct reorder indices), + // and vanishing here makes the drop a silent no-op. + const boundary = pickNearestHeaderBoundarySlot(args.rects, localY) + if (!boundary) { return null } - - const mid = (hoveredRect.top + hoveredRect.bottom) / 2 - const dropIndex = localY < mid ? hoveredRect.headerIndex : hoveredRect.headerIndex + 1 - const nextRect = - localY < mid ? hoveredRect : args.rects.find((rect) => rect.headerIndex >= dropIndex) - const indicatorY = nextRect - ? Math.max(0, nextRect.top - INDICATOR_GAP_PX) - : Math.max(hoveredRect.bottom, hoveredRect.sectionBottom ?? hoveredRect.bottom) + - INDICATOR_GAP_PX - return { - dropIndex, - dropIndicatorY: Math.max(args.scrollTop, indicatorY) + dropIndex: boundary.dropIndex, + dropIndicatorY: Math.max(args.scrollTop, boundary.indicatorY) } } + +type WorktreeSidebarHeaderBoundarySlot = { + dropIndex: number + indicatorY: number +} + +function pickNearestHeaderBoundarySlot( + rects: readonly WorktreeSidebarHeaderDragRect[], + localY: number +): WorktreeSidebarHeaderBoundarySlot | null { + let prevRect: WorktreeSidebarHeaderDragRect | undefined + let nextRect: WorktreeSidebarHeaderDragRect | undefined + for (const rect of rects) { + if (rect.top <= localY) { + prevRect = rect + } else if (nextRect === undefined) { + nextRect = rect + } + } + + const afterPrev: WorktreeSidebarHeaderBoundarySlot | null = prevRect + ? { + dropIndex: prevRect.headerIndex + 1, + indicatorY: + Math.max(prevRect.bottom, prevRect.sectionBottom ?? prevRect.bottom) + INDICATOR_GAP_PX + } + : null + const beforeNext: WorktreeSidebarHeaderBoundarySlot | null = nextRect + ? { dropIndex: nextRect.headerIndex, indicatorY: Math.max(0, nextRect.top - INDICATOR_GAP_PX) } + : null + + if (!afterPrev) { + return beforeNext + } + if (!beforeNext) { + return afterPrev + } + // Ties (localY at the span midpoint) resolve to the next header's boundary. + return Math.abs(localY - beforeNext.indicatorY) <= Math.abs(localY - afterPrev.indicatorY) + ? beforeNext + : afterPrev +} diff --git a/tests/e2e/project-group-manual-sort.spec.ts b/tests/e2e/project-group-manual-sort.spec.ts index ac1ee7e55..e402ee354 100644 --- a/tests/e2e/project-group-manual-sort.spec.ts +++ b/tests/e2e/project-group-manual-sort.spec.ts @@ -300,7 +300,7 @@ test.describe('Project Group manual sorting', () => { .toEqual([projects.alphaId, projects.charlieId, projects.bravoId]) }) - test('dropping a project over another project body does not reorder projects', async ({ + test('dropping a project over another project body snaps to that section boundary', async ({ orcaPage }) => { await waitForSessionReady(orcaPage) @@ -314,6 +314,10 @@ test.describe('Project Group manual sorting', () => { }) .toEqual([projects.alphaId, projects.bravoId, projects.charlieId]) + // Dragging alpha into bravo's expanded body drops the pointer 32px below + // bravo's header. Both nearest boundaries (bravo's section bottom and + // charlie's top) map to the slot after bravo, so alpha lands between bravo + // and charlie deterministically regardless of the exact section height. await dragProjectIntoProjectBody({ page: orcaPage, draggedProjectId: projects.alphaId, @@ -323,9 +327,9 @@ test.describe('Project Group manual sorting', () => { await expect .poll(() => getProjectHeaderOrder(orcaPage, projects), { timeout: 12_000, - message: 'Dropping on a project body should not persist a project reorder' + message: 'Dropping into a project body should snap to the nearest boundary slot' }) - .toEqual([projects.alphaId, projects.bravoId, projects.charlieId]) + .toEqual([projects.bravoId, projects.alphaId, projects.charlieId]) }) test('dragging a duplicate-ranked Project Group header reorders the visible headers', async ({