fix(sidebar): map header drags to the nearest boundary slot instead of a dead zone (#8891)
* fix(sidebar): map header drags to the nearest boundary slot instead of a dead zone Fixes #8879 * fix(sidebar): bound header edge drops to measured content
This commit is contained in:
parent
d3fedb9ba9
commit
24dc165b3c
|
|
@ -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
|
||||
})
|
||||
},
|
||||
[]
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
})
|
||||
},
|
||||
[]
|
||||
|
|
|
|||
|
|
@ -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
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 ({
|
||||
|
|
|
|||
Loading…
Reference in New Issue