diff --git a/apps/web/src/components/Sidebar.drag.test.ts b/apps/web/src/components/Sidebar.drag.test.ts index 9b02966998b4..5c8c95f34400 100644 --- a/apps/web/src/components/Sidebar.drag.test.ts +++ b/apps/web/src/components/Sidebar.drag.test.ts @@ -1,8 +1,13 @@ import { describe, expect, it, vi } from "vite-plus/test"; import { closestCenter, type CollisionDetection } from "@dnd-kit/core"; import { verticalListSortingStrategy, type SortingStrategy } from "@dnd-kit/sortable"; -import { createSidebarCollisionDetection, createSidebarSortingStrategy } from "./Sidebar.drag"; import { + createSidebarCollisionDetection, + createSidebarSortingStrategy, + restrictBelowSidebarLabel, +} from "./Sidebar.drag"; +import { + resolveSidebarDropTarget, sidebarListItemId, sidebarMarkerId, type SidebarListItem, @@ -35,7 +40,7 @@ function layout( ? (item.section === "pinned" || item.section === "active" ? cardHeight : 36) * scale : item.marker === "pinned-header" || item.marker === "pinned-divider" ? 0 - : (item.marker.endsWith("placeholder") ? 36 : 32) * scale; + : (item.marker.endsWith("placeholder") ? 0 : 32) * scale; const rect = { top, height, bottom: top + height, left: 0, right: 260, width: 260 }; top += height + 1; return rect; @@ -119,69 +124,92 @@ describe("sidebar collision detection", () => { expect(detector(collisionArgs())[0]?.id).toBe("blocked"); }); - function clampedArgs() { - const args = collisionArgs(); - const pinned = args.droppableRects.get(sidebarMarkerId("pinned-header"))!; - const source = args.droppableRects.get("source")!; - const collisionRect = { - ...source, - top: pinned.top - 8, - bottom: pinned.top - 8 + source.height, - }; - return { - ...args, - active: { - ...args.active, - rect: { current: { initial: source, translated: collisionRect } }, - }, - collisionRect, - pointerCoordinates: { x: pinned.left + pinned.width / 2, y: pinned.top + 8 }, - }; - } - - it("reaches empty Pins with an upward pointer while the card is clamped at the top", () => { - const args = clampedArgs(); - const detector = createSidebarCollisionDetection(() => true, { - emptyPins: true, - activationY: args.pointerCoordinates.y + 6, - }); - expect(args.droppableRects.get(sidebarMarkerId("pinned-header"))?.height).toBe(0); - expect(closestCenter(args)[0]?.id).toBe("source"); - expect(detector(args)[0]?.id).toBe(sidebarMarkerId("pinned-header")); - }); - it.each([ - { reason: "below the boundary cue", x: 130, y: 109, activationY: 140, emptyPins: true }, - { reason: "left of the list", x: -1, y: 108, activationY: 140, emptyPins: true }, - { reason: "right of the list", x: 261, y: 108, activationY: 140, emptyPins: true }, - { reason: "less than 6px upward", x: 130, y: 108, activationY: 113, emptyPins: true }, - { reason: "without an activation point", x: 130, y: 108, activationY: null, emptyPins: true }, - { reason: "with populated Pins", x: 130, y: 108, activationY: 140, emptyPins: false }, - ])("keeps ordinary collision behavior $reason", ({ x, y, activationY, emptyPins }) => { - const detector = createSidebarCollisionDetection(() => true, { emptyPins, activationY }); - const args = { ...clampedArgs(), pointerCoordinates: { x, y } }; - expect(detector(args)[0]?.id).toBe("source"); - }); - - it("keeps ordinary collision behavior without pointer coordinates", () => { - const detector = createSidebarCollisionDetection(() => true, { - emptyPins: true, - activationY: 140, - }); - expect(detector({ ...clampedArgs(), pointerCoordinates: null })[0]?.id).toBe("source"); - }); - - it("validates the empty Pins override and caches an unsupported result", () => { - const isValid = vi.fn(() => false); - const detector = createSidebarCollisionDetection(isValid, { - emptyPins: true, - activationY: 140, - }); - const args = clampedArgs(); - expect(detector(args).map((collision) => collision.id)).toEqual(["source"]); - expect(detector(args).map((collision) => collision.id)).toEqual(["source"]); - expect(isValid.mock.calls).toEqual([[sidebarMarkerId("pinned-header")]]); - }); + { sourceSection: "active", pins: 0 }, + { sourceSection: "active", pins: 1 }, + { sourceSection: "pinned", pins: 1 }, + { sourceSection: "settled", pins: 1 }, + ] as const)( + "switches on crossing the divider row from $sourceSection with $pins pins", + ({ sourceSection, pins }) => { + const items = [ + pinnedHeader, + ...(pins ? [thread("p", "pinned")] : []), + ...(sourceSection === "pinned" ? [thread("source", "pinned")] : []), + divider, + thread("a", "active"), + ...(sourceSection === "active" ? [thread("source", "active")] : []), + settledHeader, + ...(sourceSection === "settled" ? [thread("source", "settled")] : []), + ]; + const { rects, activeIndex } = layout(items, "source", "a"); + const sourceRect = rects[activeIndex]!; + let boundaryTop = 300; + const boundaryNode = { + querySelector: () => ({ + getBoundingClientRect: () => ({ + top: boundaryTop, + bottom: boundaryTop + 16, + left: 0, + right: 260, + }), + }), + } as unknown as HTMLElement; + const detector = createSidebarCollisionDetection(() => true, { + items, + activationY: sourceSection === "pinned" ? 200 : 600, + }); + const at = (center: number) => { + const collisionRect = { + ...sourceRect, + top: center - sourceRect.height / 2, + bottom: center + sourceRect.height / 2, + }; + const args = { + ...collisionArgs(), + active: { + id: "source", + data: { current: {} }, + rect: { current: { initial: sourceRect, translated: collisionRect } }, + }, + collisionRect, + pointerCoordinates: { x: 130, y: center }, + droppableRects: new Map( + items.map((item, index) => [sidebarListItemId(item), rects[index]!]), + ), + droppableContainers: items.map((item, index) => ({ + id: sidebarListItemId(item), + key: sidebarListItemId(item), + disabled: false, + data: { current: {} }, + node: { + current: + item === divider + ? boundaryNode + : item === settledHeader + ? ({ getBoundingClientRect: () => ({ top: 600 }) } as unknown as HTMLElement) + : null, + }, + rect: { current: rects[index]! }, + })), + }; + const over = detector(args)[0]; + return over ? resolveSidebarDropTarget(items, "source", String(over.id))?.section : null; + }; + expect(at(330)).toBe("active"); + expect(at(317)).toBe("active"); + expect(at(316)).toBe("pinned"); + // The preview moves the divider; a stationary pointer must not undo the drop target. + boundaryTop = 400; + expect(at(316)).toBe("pinned"); + expect(at(399)).toBe("pinned"); + expect(at(400)).toBe("active"); + boundaryTop = 300; + expect(at(400)).toBe("active"); + expect(at(317)).toBe("active"); + expect(at(316)).toBe("pinned"); + }, + ); it("returns no collision if an unsupported target has no source fallback", () => { const args = collisionArgs(); @@ -289,7 +317,7 @@ describe("sidebar drag projection", () => { } }); - it("leaves canonically sorted settled peers in place", () => { + it("preserves settled order while opening the zero-height Active target", () => { const items = [ pinnedHeader, divider, @@ -303,7 +331,10 @@ describe("sidebar drag projection", () => { "second", "first", ); - expect([...result.values()]).toEqual(items.map(() => stationary)); + expect(result.get(sidebarMarkerId("active-placeholder"))).toEqual(stationary); + expect(result.get(sidebarMarkerId("settled-header"))).toEqual({ ...stationary, y: 36 }); + expect(result.get("first")).toEqual({ ...stationary, y: 36 }); + expect(result.get("second")).toEqual(stationary); }); it.each([ @@ -357,6 +388,36 @@ describe("sidebar drag projection", () => { expect(result.get("s")?.y).toBe(32); }); + it.each(["s1", "missing-target"])( + "keeps label clearance when a settled drag is over %s", + (over) => { + const items = [ + pinnedHeader, + thread("p", "pinned"), + divider, + thread("a", "active"), + settledHeader, + thread("s1", "settled"), + thread("s2", "settled"), + ]; + const result = preview( + { + items, + settledOrder: ["s1", "s2"], + settledExpanded: true, + boundaryLabelHeight: 24, + }, + "s2", + over, + ); + expect(result.get("p")?.y).toBe(24); + expect(result.get(sidebarMarkerId("pinned-divider"))?.y).toBe(24); + expect(result.get("a")?.y).toBe(48); + expect(result.get("s1")?.y).toBe(48); + expect(result.get("s2")).toEqual(stationary); + }, + ); + it("stacks the labels with their gaps when the pinned section is empty", () => { const items = [ pinnedHeader, @@ -409,8 +470,8 @@ describe("sidebar drag projection", () => { }); it.each([ - ["p", -83, -37], - ["s", 0, 46], + ["p", -83, -1], + ["s", 0, 82], ] as const)( "replaces the empty Active target when %s enters", (active, dividerOffset, settledOffset) => { @@ -561,7 +622,7 @@ describe("sidebar drag projection", () => { expect(strategy({ ...smaller, index: 2 })?.y).toBe(-62.5); }); - it("uses measured placeholder sizing when card height differs from its default", () => { + it("uses shelf height for empty target sizing when card height differs from its default", () => { const items = [ pinnedHeader, thread("p", "pinned"), @@ -674,3 +735,47 @@ describe("sidebar drag projection", () => { expect(result.get(sidebarMarkerId("settled-header"))?.y).toBe(46); }); }); + +describe("lifted card clearance", () => { + const rect = (top: number, height: number) => ({ + top, + bottom: top + height, + height, + left: 0, + right: 260, + width: 260, + }); + const apply = (cardTop: number, cardHeight: number, y: number, listTop = 136, offset = 32) => + restrictBelowSidebarLabel( + { + transform: { ...stationary, y }, + containerNodeRect: rect(listTop, 500), + draggingNodeRect: rect(cardTop, cardHeight), + activatorEvent: null, + active: null, + activeNodeRect: null, + over: null, + overlayNodeRect: null, + scrollableAncestors: [], + scrollableAncestorRects: [], + windowRect: null, + }, + offset, + ); + + it.each([36, 82])("keeps a %ipx row below empty Pins even past the top edge", (height) => { + for (const pointerY of [150, 136, 100, 0]) { + const transform = apply(511, height, pointerY - 529); + expect(511 + transform.y).toBe(168); + } + }); + + it("preserves pointer movement below the label", () => { + expect(apply(511, 36, -200).y).toBe(-200); + }); + + it("follows the list when it scrolls and includes content preceding Pins", () => { + expect(511 + apply(511, 36, -500, 96).y).toBe(128); + expect(511 + apply(511, 36, -500, 136, 114).y).toBe(250); + }); +}); diff --git a/apps/web/src/components/Sidebar.drag.ts b/apps/web/src/components/Sidebar.drag.ts index 4a222060366a..0aef9dec14b1 100644 --- a/apps/web/src/components/Sidebar.drag.ts +++ b/apps/web/src/components/Sidebar.drag.ts @@ -1,4 +1,4 @@ -import { closestCenter, type CollisionDetection } from "@dnd-kit/core"; +import { closestCenter, type CollisionDetection, type Modifier } from "@dnd-kit/core"; import { verticalListSortingStrategy, type SortingStrategy } from "@dnd-kit/sortable"; import { resolveSidebarDropTarget, @@ -14,36 +14,77 @@ const hidden = { ...stationary, scaleY: 0 }; type ThreadItem = Extract; type Layout = Parameters[0]; +/** Keep the lifted card below the Pins label, including when Pins is empty. + * The container rect follows scrolling; the offset is measured once at pickup. */ +export function restrictBelowSidebarLabel( + { transform, containerNodeRect, draggingNodeRect }: Parameters[0], + offset: number, +) { + if (!containerNodeRect || !draggingNodeRect) return transform; + const minimumY = containerNodeRect.top + offset - draggingNodeRect.top; + return transform.y < minimumY ? { ...transform, y: minimumY } : transform; +} + /** Reject the nearest unsupported target without selecting another section. * Recreate this detector when drop eligibility changes. */ export function createSidebarCollisionDetection( isValidTarget: (id: string) => boolean, - options: { emptyPins?: boolean; activationY?: number | null } = {}, + options: { + items?: readonly SidebarListItem[]; + activationY?: number | null; + } = {}, ): CollisionDetection { const validity = new Map(); - const pinnedHeaderId = sidebarMarkerId("pinned-header"); + const sections = new Map(); + let previousPointerY = options.activationY; + let boundarySection: "pinned" | "active" | undefined; return (args) => { let collisions = closestCenter(args); - const pinnedRect = options.emptyPins ? args.droppableRects.get(pinnedHeaderId) : undefined; const pointer = args.pointerCoordinates; - // The card itself is clamped by the scroll container. An upward pointer - // gesture can still reach the empty pinned boundary without reserving a row. - if ( - pinnedRect && - pointer && - options.activationY != null && - pointer.y <= options.activationY - 6 && - pointer.y <= pinnedRect.top + 8 && - pointer.x >= pinnedRect.left && - pointer.x <= pinnedRect.right - ) { - const pinned = collisions.find((collision) => collision.id === pinnedHeaderId); - if (pinned) { - collisions = [pinned, ...collisions.filter((collision) => collision !== pinned)]; + const items = options.items; + const source = items?.find((item) => item.kind === "thread" && item.key === args.active.id); + const boundary = args.droppableContainers + .find((container) => container.id === sidebarMarkerId("pinned-divider")) + ?.node.current?.querySelector(".sidebar-drag-boundary-label") + ?.getBoundingClientRect(); + if (items && boundary && source?.kind === "thread" && pointer) { + boundarySection ??= source.section === "pinned" ? "pinned" : "active"; + // Use the visible divider row, including its sortable translation. + // Only pointer movement can change sections: opening the destination + // moves this row, but must not toggle a stationary gesture back. + const previousY = previousPointerY ?? pointer.y; + previousPointerY = pointer.y; + if (pointer.x >= boundary.left && pointer.x <= boundary.right) { + if (pointer.y < previousY && pointer.y <= boundary.bottom) boundarySection = "pinned"; + else if (pointer.y > previousY && pointer.y >= boundary.top) boundarySection = "active"; + const nextHeader = + args.droppableContainers.find( + (container) => container.id === sidebarMarkerId("snoozed-header"), + ) ?? + args.droppableContainers.find( + (container) => container.id === sidebarMarkerId("settled-header"), + ); + const activeBottom = nextHeader?.node.current?.getBoundingClientRect().top; + if (boundarySection === "pinned" || (activeBottom != null && pointer.y < activeBottom)) { + const target = collisions.find((collision) => { + const id = String(collision.id); + if (!sections.has(id)) { + sections.set( + id, + resolveSidebarDropTarget(items, String(args.active.id), id)?.section ?? null, + ); + } + return sections.get(id) === boundarySection; + }); + if (target) + collisions = [target, ...collisions.filter((collision) => collision !== target)]; + } } } const nearest = collisions[0]; - if (!nearest || nearest.id === args.active.id) return collisions; + if (!nearest || nearest.id === args.active.id) { + return collisions; + } const id = String(nearest.id); const valid = validity.get(id) ?? isValidTarget(id); validity.set(id, valid); @@ -73,14 +114,10 @@ export function createSidebarSortingStrategy(input: { function project({ rects, activeIndex, overIndex }: Layout) { const active = items[activeIndex]; - const over = items[overIndex]; + const over = items[overIndex] ?? active; if (active?.kind !== "thread" || !over || !rects[0]) return []; const target = resolveSidebarDropTarget(items, active.key, sidebarListItemId(over)); if (!target) return []; - // Settled keeps time order, so a reorder inside it previews nothing. - // Every other drag projects so the boundary labels get their space. - if (target.section === active.section && over.kind === "thread" && target.section === "settled") - return []; const groups: Record = { pinned: [], active: [], @@ -89,9 +126,13 @@ export function createSidebarSortingStrategy(input: { }; let cardHeight = input.cardHeight; let slimHeight = input.slimHeight; + let headerScale: number | undefined; for (const [index, item] of items.entries()) { if (item.kind === "marker") { - if (item.marker.endsWith("placeholder")) slimHeight ??= rects[index]?.height; + if (item.marker === "settled-header" || item.marker === "snoozed-header") { + const height = rects[index]?.height; + if (height) headerScale ??= height / 32; + } continue; } if (item.section === "pinned" || item.section === "active") @@ -100,7 +141,8 @@ export function createSidebarSortingStrategy(input: { if (item.key !== active.key) groups[item.section].push(item); } // Cards are 4.875rem + 0.25rem padding; slim rows/placeholders are h-9. - const scale = slimHeight !== undefined ? slimHeight / 36 : (cardHeight ?? 82) / 82; + const scale = + slimHeight !== undefined ? slimHeight / 36 : (headerScale ?? (cardHeight ?? 82) / 82); cardHeight ??= 82 * scale; slimHeight ??= 36 * scale; const labelHeight = (input.boundaryLabelHeight ?? 0) * scale; @@ -163,9 +205,11 @@ export function createSidebarSortingStrategy(input: { item.kind === "marker" && (item.marker === "pinned-header" || item.marker === "pinned-divider") ? labelHeight - : moved - ? fallback - : (rect?.height ?? fallback); + : item.kind === "marker" && item.marker.endsWith("placeholder") + ? slimHeight + : moved + ? fallback + : (rect?.height ?? fallback); top += height + 1; } result[activeIndex] = stationary; diff --git a/apps/web/src/components/Sidebar.pointer.test.ts b/apps/web/src/components/Sidebar.pointer.test.ts new file mode 100644 index 000000000000..9a82edbd6dc7 --- /dev/null +++ b/apps/web/src/components/Sidebar.pointer.test.ts @@ -0,0 +1,200 @@ +import type { SensorProps } from "@dnd-kit/core"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import { act, createElement, StrictMode } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { SidebarDragLifecycle, SidebarPointerSensor } from "./Sidebar.pointer"; + +class TestDocument extends EventTarget { + hidden = false; + getSelection = () => ({ removeAllRanges() {} }); +} + +let document: TestDocument; +let window: EventTarget; +const sensors: SidebarPointerSensor[] = []; + +function pointer(type: string, values: Partial = {}) { + return Object.assign(new Event(type, { cancelable: true }), { + pointerId: 1, + clientX: 10, + clientY: 10, + buttons: 1, + ...values, + }); +} + +function gesture() { + const callbacks = { + onStart: vi.fn(), + onMove: vi.fn(), + onEnd: vi.fn(), + onCancel: vi.fn(), + onAbort: vi.fn(), + onPending: vi.fn(), + }; + const onFinish = vi.fn(); + // The sensor never reads dnd-kit's layout context or active node. + const props = { + active: "thread", + event: pointer("pointerdown"), + options: { distance: 6, onAttach: vi.fn(), onFinish }, + ...callbacks, + } as unknown as SensorProps[0]["options"]>; + const sensor = new SidebarPointerSensor(props); + sensors.push(sensor); + return { sensor, onFinish, ...callbacks }; +} + +beforeEach(() => { + vi.useFakeTimers(); + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + document = new TestDocument(); + window = Object.assign(new EventTarget(), { setTimeout }); + vi.stubGlobal("document", document); + vi.stubGlobal("window", window); +}); +afterEach(() => { + for (const sensor of sensors.splice(0)) sensor.cancel(); + vi.runAllTimers(); + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); + +describe("sidebar pointer lifecycle", () => { + it("keeps a click idle and starts only after the drag threshold", () => { + const click = gesture(); + document.dispatchEvent(pointer("pointermove", { clientY: 16 })); + expect(click.onStart).not.toHaveBeenCalled(); + document.dispatchEvent(pointer("pointerup", { buttons: 0 })); + expect(click.onAbort).toHaveBeenCalledOnce(); + expect(click.onFinish).toHaveBeenCalledOnce(); + + const drag = gesture(); + document.dispatchEvent(pointer("pointermove", { clientY: 17 })); + expect(drag.onStart).toHaveBeenCalledExactlyOnceWith({ x: 10, y: 10 }); + document.dispatchEvent(pointer("pointerup", { buttons: 0 })); + expect(drag.onEnd).toHaveBeenCalledOnce(); + expect(drag.onAbort).not.toHaveBeenCalled(); + expect(drag.onFinish).toHaveBeenCalledOnce(); + }); + + const interruptions = { + blur: () => window.dispatchEvent(new Event("blur")), + hidden: () => { + document.hidden = true; + document.dispatchEvent(new Event("visibilitychange")); + }, + pagehide: () => window.dispatchEvent(new Event("pagehide")), + resize: () => window.dispatchEvent(new Event("resize")), + escape: () => document.dispatchEvent(Object.assign(new Event("keydown"), { code: "Escape" })), + pointercancel: () => document.dispatchEvent(pointer("pointercancel")), + "missed release": () => + document.dispatchEvent(pointer("pointermove", { buttons: 0, clientY: 100 })), + }; + for (const [name, interrupt] of Object.entries(interruptions)) { + it.each([false, true])(`cancels on ${name}, started=%s, and ignores late events`, (started) => { + const drag = gesture(); + if (started) document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + interrupt(); + document.dispatchEvent(pointer("pointermove", { clientY: 100 })); + document.dispatchEvent(pointer("pointerup", { buttons: 0 })); + drag.sensor.cancel(); + expect(drag.onCancel).toHaveBeenCalledOnce(); + expect(drag.onEnd).not.toHaveBeenCalled(); + expect(drag.onFinish).toHaveBeenCalledOnce(); + expect(drag.onStart).toHaveBeenCalledTimes(started ? 1 : 0); + expect(drag.onAbort).toHaveBeenCalledTimes(started ? 0 : 1); + + document.hidden = false; + const next = gesture(); + document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + document.dispatchEvent(pointer("pointerup", { buttons: 0 })); + expect(next.onStart).toHaveBeenCalledOnce(); + expect(next.onEnd).toHaveBeenCalledOnce(); + }); + } + + it("suppresses a delayed release click after cancellation, then accepts the next click", () => { + const drag = gesture(); + document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + drag.sensor.cancel(); + vi.advanceTimersByTime(1000); + const releaseClick = new Event("click"); + const releasePropagation = vi.spyOn(releaseClick, "stopPropagation"); + document.dispatchEvent(releaseClick); + expect(releasePropagation).toHaveBeenCalledOnce(); + document.dispatchEvent(pointer("pointerdown")); + const nextClick = new Event("click"); + const nextPropagation = vi.spyOn(nextClick, "stopPropagation"); + document.dispatchEvent(nextClick); + expect(nextPropagation).not.toHaveBeenCalled(); + }); + + it("allows the next click when the cancelled release happened outside the document", () => { + const drag = gesture(); + document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + drag.sensor.cancel(); + document.dispatchEvent(pointer("pointerdown")); + const click = new Event("click"); + const propagation = vi.spyOn(click, "stopPropagation"); + document.dispatchEvent(click); + expect(propagation).not.toHaveBeenCalled(); + }); + + it("does not move after cancellation during activation", () => { + const drag = gesture(); + drag.onStart.mockImplementation(() => drag.sensor.cancel()); + document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + expect(drag.onCancel).toHaveBeenCalledOnce(); + expect(drag.onMove).not.toHaveBeenCalled(); + expect(drag.onFinish).toHaveBeenCalledOnce(); + }); + + it("ignores events from other pointers", () => { + const drag = gesture(); + document.dispatchEvent(pointer("pointermove", { pointerId: 2, buttons: 0, clientY: 100 })); + document.dispatchEvent(pointer("pointercancel", { pointerId: 2 })); + document.dispatchEvent(pointer("pointerup", { pointerId: 2 })); + expect(drag.onStart).not.toHaveBeenCalled(); + expect(drag.onFinish).not.toHaveBeenCalled(); + document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + expect(drag.onStart).toHaveBeenCalledOnce(); + }); + + it.each([false, true])("cancels when search unmounts the list, started=%s", (started) => { + let renderer: ReactTestRenderer; + let drag: ReturnType | undefined; + act(() => { + renderer = create( + createElement( + StrictMode, + null, + createElement(SidebarDragLifecycle, { + onUnmount: () => drag?.sensor.cancel(), + }), + ), + ); + }); + drag = gesture(); + if (started) document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + act(() => renderer.unmount()); + document.dispatchEvent(pointer("pointermove", { clientY: 100 })); + document.dispatchEvent(pointer("pointerup", { buttons: 0 })); + expect(drag.onStart).toHaveBeenCalledTimes(started ? 1 : 0); + expect(drag.onCancel).toHaveBeenCalledOnce(); + expect(drag.onEnd).not.toHaveBeenCalled(); + expect(drag.onFinish).toHaveBeenCalledOnce(); + }); + + it("detaches a cancelled sensor before a replacement gesture starts", () => { + const previous = gesture(); + document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + previous.sensor.cancel(); + const next = gesture(); + document.dispatchEvent(pointer("pointermove", { clientY: 20 })); + document.dispatchEvent(pointer("pointerup", { buttons: 0 })); + expect(previous.onCancel).toHaveBeenCalledOnce(); + expect(previous.onEnd).not.toHaveBeenCalled(); + expect(next.onEnd).toHaveBeenCalledOnce(); + }); +}); diff --git a/apps/web/src/components/Sidebar.pointer.ts b/apps/web/src/components/Sidebar.pointer.ts new file mode 100644 index 000000000000..f79bc06c64be --- /dev/null +++ b/apps/web/src/components/Sidebar.pointer.ts @@ -0,0 +1,141 @@ +import { useLayoutEffect, type PointerEvent as ReactPointerEvent } from "react"; +import { type SensorProps } from "@dnd-kit/core"; +import { getOwnerDocument, getWindow } from "@dnd-kit/utilities"; + +// Search unmounts the drag context while its owning Sidebar remains mounted. +export function SidebarDragLifecycle({ onUnmount }: { onUnmount: () => void }) { + useLayoutEffect(() => onUnmount, [onUnmount]); + return null; +} + +type Options = { + distance: number; + onAttach: (sensor: SidebarPointerSensor) => void; + onFinish: (started: boolean) => void; +}; + +/** A sidebar gesture ends on release, cancellation, or loss of its window. + * Own the listeners so unmounting the list can cancel the sensor too. */ +export class SidebarPointerSensor { + static activators = [ + { + eventName: "onPointerDown" as const, + handler: ({ nativeEvent }: ReactPointerEvent) => + nativeEvent.isPrimary && nativeEvent.button === 0, + }, + ]; + autoScrollEnabled = true; + private phase: "pending" | "dragging" | "finished" = "pending"; + private readonly pointer: PointerEvent; + private readonly document: Document; + private readonly window: Window; + + constructor(private readonly props: SensorProps) { + this.pointer = props.event as PointerEvent; + this.document = getOwnerDocument(this.pointer.target); + this.window = getWindow(this.pointer.target); + this.document.addEventListener("pointermove", this.move, { passive: false, capture: true }); + this.document.addEventListener("pointerup", this.end, { capture: true }); + this.document.addEventListener("pointercancel", this.pointerCancel, { capture: true }); + this.document.addEventListener("keydown", this.keydown, { capture: true }); + this.document.addEventListener("visibilitychange", this.visibilityChange); + this.window.addEventListener("blur", this.cancel); + this.window.addEventListener("pagehide", this.cancel); + this.window.addEventListener("resize", this.cancel); + this.document.addEventListener("dragstart", this.preventDefault); + this.document.addEventListener("contextmenu", this.preventDefault); + props.options.onAttach(this); + props.onPending(props.active, { distance: props.options.distance }, this.coordinates()); + } + + private coordinates = () => ({ x: this.pointer.clientX, y: this.pointer.clientY }); + private preventDefault = (event: Event) => event.preventDefault(); + private clearClickSuppression = () => { + this.document.removeEventListener("click", this.suppressClick, { capture: true }); + this.document.removeEventListener("pointerdown", this.clearClickSuppression, { capture: true }); + }; + private suppressClick = (event: Event) => { + event.stopPropagation(); + this.clearClickSuppression(); + }; + private clearSelection = () => this.document.getSelection()?.removeAllRanges(); + + private move = (event: PointerEvent) => { + if (this.phase === "finished" || event.pointerId !== this.pointer.pointerId) return; + // A release outside the window can be missed. Never activate or continue + // a drag when the initiating button is no longer held. + if ((event.buttons & 1) === 0) return this.cancel(); + const coordinates = { x: event.clientX, y: event.clientY }; + if (this.phase === "pending") { + const offset = { + x: event.clientX - this.pointer.clientX, + y: event.clientY - this.pointer.clientY, + }; + if (Math.hypot(offset.x, offset.y) <= this.props.options.distance) { + this.props.onPending( + this.props.active, + { distance: this.props.options.distance }, + this.coordinates(), + offset, + ); + return; + } + this.phase = "dragging"; + this.document.addEventListener("click", this.suppressClick, { capture: true }); + this.document.addEventListener("selectionchange", this.clearSelection); + this.clearSelection(); + this.props.onStart(this.coordinates()); + return; + } + if (this.phase === "dragging") { + if (event.cancelable) event.preventDefault(); + this.props.onMove(coordinates); + } + }; + + private end = (event: PointerEvent) => { + if (event.pointerId === this.pointer.pointerId) this.finish(false); + }; + private pointerCancel = (event: PointerEvent) => { + if (event.pointerId === this.pointer.pointerId) this.cancel(); + }; + private keydown = (event: KeyboardEvent) => { + if (event.code === "Escape") this.cancel(); + }; + private visibilityChange = () => { + if (this.document.hidden) this.cancel(); + }; + cancel = () => this.finish(true); + + private finish(cancelled: boolean) { + if (this.phase === "finished") return; + const aborted = this.phase === "pending"; + this.phase = "finished"; + this.document.removeEventListener("pointermove", this.move, { capture: true }); + this.document.removeEventListener("pointerup", this.end, { capture: true }); + this.document.removeEventListener("pointercancel", this.pointerCancel, { capture: true }); + this.document.removeEventListener("keydown", this.keydown, { capture: true }); + this.document.removeEventListener("visibilitychange", this.visibilityChange); + this.window.removeEventListener("blur", this.cancel); + this.window.removeEventListener("pagehide", this.cancel); + this.window.removeEventListener("resize", this.cancel); + this.document.removeEventListener("dragstart", this.preventDefault); + this.document.removeEventListener("contextmenu", this.preventDefault); + this.document.removeEventListener("selectionchange", this.clearSelection); + // Cancellation can precede release by an arbitrary amount of time. Consume + // that release click, or let a fresh pointerdown end suppression if release + // happened outside the document. Ordinary clicks never install this guard. + if (!aborted) { + this.document.addEventListener("pointerdown", this.clearClickSuppression, { capture: true }); + } + try { + // Release the sidebar preview before dnd-kit clears its transforms. + // Its public end/cancel event can be omitted before its first layout. + this.props.options.onFinish(!aborted); + } finally { + if (aborted) this.props.onAbort(this.props.active); + if (cancelled) this.props.onCancel(); + else this.props.onEnd(); + } + } +} diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 8bea547a3215..deb00e7181c7 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -2,12 +2,12 @@ import { useAtomValue } from "@effect/atom-react"; import * as Schema from "effect/Schema"; import { DndContext, - PointerSensor, useSensor, useSensors, type DragEndEvent, type DragOverEvent, type DragStartEvent, + type Modifier, } from "@dnd-kit/core"; import { SortableContext, useSortable } from "@dnd-kit/sortable"; import { restrictToFirstScrollableAncestor, restrictToVerticalAxis } from "@dnd-kit/modifiers"; @@ -172,7 +172,12 @@ import { type SidebarSection, } from "./Sidebar.logic"; import { resolveLocalCheckoutBranchMismatch } from "./BranchToolbar.logic"; -import { createSidebarCollisionDetection, createSidebarSortingStrategy } from "./Sidebar.drag"; +import { + createSidebarCollisionDetection, + createSidebarSortingStrategy, + restrictBelowSidebarLabel, +} from "./Sidebar.drag"; +import { SidebarDragLifecycle, SidebarPointerSensor } from "./Sidebar.pointer"; import { createSidebarListMotion } from "./Sidebar.motion"; import { ThreadWorktreeIndicator, @@ -549,8 +554,8 @@ function SortableSidebarMarker(props: { ); } -// Empty targets stay mounted before pickup so starting a drag never changes -// the list's measured positions. +// Empty targets stay measurable without reserving space at rest. The sorting +// strategy opens their hint space during a drag. function SidebarSectionPlaceholder(props: { marker: "active-placeholder" | "settled-placeholder"; label: string; @@ -561,26 +566,25 @@ function SidebarSectionPlaceholder(props: { - {props.showHint ? props.label : null} + {props.showHint ? ( +
+ {props.label} +
+ ) : null} ); } -// Boundary labels appear during a drag in space the sorting strategy opens -// below each marker (SIDEBAR_DRAG_LABEL_HEIGHT), so they never sit on a row. -// The marker itself stays zero height, so nothing is reserved at rest and -// pickup measurements are unchanged. They read at full strength so the -// sections are easy to find, and the section under the lifted row takes the -// accent. They paint above the lifted row so a card dragged across a -// boundary never hides its label. -// Matches the label's h-4 below. -const SIDEBAR_DRAG_LABEL_HEIGHT = 16; +// Zero-height markers reserve no label space at rest. During a drag the +// sorting strategy opens 24px for a 16px label with 4px clearance on each side. +const SIDEBAR_DRAG_LABEL_HEIGHT = 24; function SidebarDragBoundary(props: { marker: "pinned-header" | "pinned-divider"; @@ -592,10 +596,10 @@ function SidebarDragBoundary(props: { {props.visible ? ( -
+
(null); + const dragLabelOffsetRef = useRef(0); + const restrictBelowPins = useCallback( + (args) => restrictBelowSidebarLabel(args, dragLabelOffsetRef.current), + [], + ); const listMotionRef = useRef | null>(null); const attachListMotionRef = useCallback((node: HTMLUListElement | null) => { + threadListRef.current = node; listMotionRef.current?.dispose(); listMotionRef.current = node === null ? null : createSidebarListMotion(node); listMotionRef.current?.update(false); @@ -3031,16 +3042,35 @@ export default function Sidebar() { // Hold the chosen section and order until every key write arrives. This // also covers first-time ordering, which assigns keys to keyless neighbors. // A failed write, concurrent reorder, or membership change releases the hold. - const dndSensors = useSensors( - useSensor(PointerSensor, { activationConstraint: { distance: 6 } }), - ); const [dragState, setDragState] = useState<{ readonly activeKey: string; readonly activeSection: SidebarSection; readonly occurredAt: string; readonly activationY: number | null; + readonly targetSection: SidebarSection | null; } | null>(null); - const [dragTargetSection, setDragTargetSection] = useState(null); + const dragTargetSection = dragState?.targetSection ?? null; + const dragSensorRef = useRef(null); + const finishThreadDrag = useCallback((started: boolean) => { + dragSensorRef.current = null; + if (started) { + listMotionRef.current?.release(); + setDragState(null); + } + }, []); + const attachDragSensor = useCallback((sensor: SidebarPointerSensor) => { + dragSensorRef.current = sensor; + }, []); + const cancelThreadDrag = useCallback(() => { + dragSensorRef.current?.cancel(); + }, []); + const dndSensors = useSensors( + useSensor(SidebarPointerSensor, { + distance: 6, + onAttach: attachDragSensor, + onFinish: finishThreadDrag, + }), + ); const sectionByThreadKey = useMemo(() => { const map = new Map(); const add = (list: readonly EnvironmentThreadShell[], section: SidebarSection) => { @@ -3185,22 +3215,27 @@ export default function Sidebar() { if (activeSection === undefined) return; // Stop normal section motion before dnd-kit measures the picked-up row. listMotionRef.current?.suspend(); + const list = threadListRef.current; + const header = list?.querySelector('[data-testid="sidebar-pinned-header"]'); + if (list && header) { + const listRect = list.getBoundingClientRect(); + const scale = list.offsetWidth > 0 ? listRect.width / list.offsetWidth : 1; + dragLabelOffsetRef.current = + header.getBoundingClientRect().top - listRect.top + SIDEBAR_DRAG_LABEL_HEIGHT * scale; + } else { + dragLabelOffsetRef.current = 0; + } setDragState({ activeKey, activeSection, + targetSection: activeSection, occurredAt: new Date().toISOString(), activationY: event.activatorEvent instanceof PointerEvent ? event.activatorEvent.clientY : null, }); - setDragTargetSection(activeSection); }, [sectionByThreadKey], ); - const handleThreadDragCancel = useCallback(() => { - listMotionRef.current?.release(); - setDragState(null); - setDragTargetSection(null); - }, []); // Include every visible row in the measured order. Older servers disable // pickup on their rows without changing where those rows render. const sidebarListItems = useMemo((): readonly SidebarListItem[] => { @@ -3249,6 +3284,14 @@ export default function Sidebar() { snoozedThreads.length, visibleSnoozedThreads, ]); + useEffect(() => { + if ( + dragState !== null && + !sidebarListItems.some((item) => item.kind === "thread" && item.key === dragState.activeKey) + ) { + cancelThreadDrag(); + } + }, [cancelThreadDrag, dragState, sidebarListItems]); const listMotionPaused = dragState !== null; useLayoutEffect(() => { // Drag release clears the baseline, so its commit cannot replay the @@ -3264,7 +3307,11 @@ export default function Sidebar() { const target = event.over ? resolveSidebarDropTarget(sidebarListItems, String(event.active.id), String(event.over.id)) : null; - setDragTargetSection(target?.section ?? null); + setDragState((current) => + current === null || current.activeKey !== String(event.active.id) + ? current + : { ...current, targetSection: target?.section ?? null }, + ); }, [sidebarListItems], ); @@ -3318,18 +3365,22 @@ export default function Sidebar() { }), [threads], ); + const draggedThreadKey = dragState?.activeKey; + const draggedFromSection = dragState?.activeSection; + const dragActivationY = dragState?.activationY; const dndCollisionDetection = useMemo(() => { - if (dragState === null) return createSidebarCollisionDetection(() => true); - const source = threadByKey.get(dragState.activeKey); + if (draggedThreadKey === undefined || draggedFromSection === undefined) + return createSidebarCollisionDetection(() => true); + const source = threadByKey.get(draggedThreadKey); if (source === undefined) return createSidebarCollisionDetection(() => false); return createSidebarCollisionDetection( (id) => { - const target = resolveSidebarDropTarget(sidebarListItems, dragState.activeKey, id); + const target = resolveSidebarDropTarget(sidebarListItems, draggedThreadKey, id); if (target === null) return false; return ( planSidebarThreadDrop({ - activeKey: dragState.activeKey, - activeSection: dragState.activeSection, + activeKey: draggedThreadKey, + activeSection: draggedFromSection, activePinned: source.pinnedAt != null, activeSettled: source.settledOverride === "settled", supportsSettlement: @@ -3345,7 +3396,10 @@ export default function Sidebar() { }).kind !== "none" ); }, - { emptyPins: pinnedKeys.length === 0, activationY: dragState.activationY }, + { + items: sidebarListItems, + activationY: dragActivationY ?? null, + }, ); }, [ activeKeysById, @@ -3353,7 +3407,9 @@ export default function Sidebar() { serverConfigs, activeKeys, activeReorderableThreadKeys, - dragState, + draggedThreadKey, + draggedFromSection, + dragActivationY, draggableThreadKeys, pinnedKeys, sidebarListItems, @@ -3361,9 +3417,6 @@ export default function Sidebar() { ]); const handleThreadDragEnd = useCallback( (event: DragEndEvent) => { - listMotionRef.current?.release(); - setDragState(null); - setDragTargetSection(null); const activeKey = String(event.active.id); const activeSection = sectionByThreadKey.get(activeKey); const target = @@ -4547,12 +4600,16 @@ export default function Sidebar() { +