From 2c623a97e10e99ea46b02ed979e11fd2df4c306e Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 6 Sep 2026 19:01:05 -0700 Subject: [PATCH 1/6] fix(web): keep sidebar drag dividers clear and reset interrupted gestures --- apps/web/src/components/Sidebar.drag.test.ts | 30 +++ apps/web/src/components/Sidebar.drag.ts | 6 +- .../src/components/Sidebar.pointer.test.ts | 173 ++++++++++++++++++ apps/web/src/components/Sidebar.pointer.ts | 131 +++++++++++++ apps/web/src/components/Sidebar.tsx | 74 +++++--- 5 files changed, 382 insertions(+), 32 deletions(-) create mode 100644 apps/web/src/components/Sidebar.pointer.test.ts create mode 100644 apps/web/src/components/Sidebar.pointer.ts diff --git a/apps/web/src/components/Sidebar.drag.test.ts b/apps/web/src/components/Sidebar.drag.test.ts index 9b02966998b4..1fb4453370a5 100644 --- a/apps/web/src/components/Sidebar.drag.test.ts +++ b/apps/web/src/components/Sidebar.drag.test.ts @@ -357,6 +357,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, diff --git a/apps/web/src/components/Sidebar.drag.ts b/apps/web/src/components/Sidebar.drag.ts index 4a222060366a..4e14b3db82f2 100644 --- a/apps/web/src/components/Sidebar.drag.ts +++ b/apps/web/src/components/Sidebar.drag.ts @@ -73,14 +73,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: [], 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..b830aedcc1c9 --- /dev/null +++ b/apps/web/src/components/Sidebar.pointer.test.ts @@ -0,0 +1,173 @@ +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("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..71cc463bd861 --- /dev/null +++ b/apps/web/src/components/Sidebar.pointer.ts @@ -0,0 +1,131 @@ +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, true); + this.document.addEventListener("pointercancel", this.pointerCancel, true); + this.document.addEventListener("keydown", this.keydown, 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 suppressClick = (event: Event) => event.stopPropagation(); + 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, true); + this.document.addEventListener("selectionchange", this.clearSelection); + this.clearSelection(); + this.props.onStart(this.coordinates()); + } + 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, true); + this.document.removeEventListener("pointerup", this.end, true); + this.document.removeEventListener("pointercancel", this.pointerCancel, true); + this.document.removeEventListener("keydown", this.keydown, 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); + // Keep the release click from opening the thread after a drag. + this.window.setTimeout(() => { + this.document.removeEventListener("click", this.suppressClick, true); + }, 0); + 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..0dba4b5924c8 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -2,7 +2,6 @@ import { useAtomValue } from "@effect/atom-react"; import * as Schema from "effect/Schema"; import { DndContext, - PointerSensor, useSensor, useSensors, type DragEndEvent, @@ -173,6 +172,7 @@ import { } from "./Sidebar.logic"; import { resolveLocalCheckoutBranchMismatch } from "./BranchToolbar.logic"; import { createSidebarCollisionDetection, createSidebarSortingStrategy } from "./Sidebar.drag"; +import { SidebarDragLifecycle, SidebarPointerSensor } from "./Sidebar.pointer"; import { createSidebarListMotion } from "./Sidebar.motion"; import { ThreadWorktreeIndicator, @@ -501,6 +501,8 @@ function SortableThreadRow(props: { id: props.id, disabled: { draggable: props.disabled }, animateLayoutChanges: animateSidebarLayoutChanges, + // Apply label clearance and row positions together, without tweening through content. + transition: null, }); // dnd-kit memoizes each field but not the bag, so the memoized row would // rerender on every shell update without this. @@ -531,6 +533,7 @@ function SortableSidebarMarker(props: { id: sidebarMarkerId(props.marker), disabled: { draggable: true }, animateLayoutChanges: animateSidebarLayoutChanges, + transition: null, }); return (
  • {props.visible ? ( -
    +
    (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) => { @@ -3188,19 +3204,14 @@ export default function Sidebar() { 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 +3260,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 +3283,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], ); @@ -3361,9 +3384,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 = @@ -4550,9 +4570,9 @@ export default function Sidebar() { modifiers={[restrictToVerticalAxis, restrictToFirstScrollableAncestor]} onDragStart={handleThreadDragStart} onDragOver={handleThreadDragOver} - onDragCancel={handleThreadDragCancel} onDragEnd={handleThreadDragEnd} > +
      Date: Sun, 6 Sep 2026 19:04:02 -0700 Subject: [PATCH 2/6] fix(web): suppress delayed release clicks after drag cancellation --- .../src/components/Sidebar.pointer.test.ts | 27 ++++++++++++++ apps/web/src/components/Sidebar.pointer.ts | 35 ++++++++++++------- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/apps/web/src/components/Sidebar.pointer.test.ts b/apps/web/src/components/Sidebar.pointer.test.ts index b830aedcc1c9..9a82edbd6dc7 100644 --- a/apps/web/src/components/Sidebar.pointer.test.ts +++ b/apps/web/src/components/Sidebar.pointer.test.ts @@ -114,6 +114,33 @@ describe("sidebar pointer lifecycle", () => { }); } + 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()); diff --git a/apps/web/src/components/Sidebar.pointer.ts b/apps/web/src/components/Sidebar.pointer.ts index 71cc463bd861..9852c6d04af9 100644 --- a/apps/web/src/components/Sidebar.pointer.ts +++ b/apps/web/src/components/Sidebar.pointer.ts @@ -35,9 +35,9 @@ export class SidebarPointerSensor { 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, true); - this.document.addEventListener("pointercancel", this.pointerCancel, true); - this.document.addEventListener("keydown", this.keydown, 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); @@ -50,7 +50,14 @@ export class SidebarPointerSensor { private coordinates = () => ({ x: this.pointer.clientX, y: this.pointer.clientY }); private preventDefault = (event: Event) => event.preventDefault(); - private suppressClick = (event: Event) => event.stopPropagation(); + 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) => { @@ -74,7 +81,7 @@ export class SidebarPointerSensor { return; } this.phase = "dragging"; - this.document.addEventListener("click", this.suppressClick, true); + this.document.addEventListener("click", this.suppressClick, { capture: true }); this.document.addEventListener("selectionchange", this.clearSelection); this.clearSelection(); this.props.onStart(this.coordinates()); @@ -103,10 +110,10 @@ export class SidebarPointerSensor { if (this.phase === "finished") return; const aborted = this.phase === "pending"; this.phase = "finished"; - this.document.removeEventListener("pointermove", this.move, true); - this.document.removeEventListener("pointerup", this.end, true); - this.document.removeEventListener("pointercancel", this.pointerCancel, true); - this.document.removeEventListener("keydown", this.keydown, true); + 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); @@ -114,10 +121,12 @@ export class SidebarPointerSensor { this.document.removeEventListener("dragstart", this.preventDefault); this.document.removeEventListener("contextmenu", this.preventDefault); this.document.removeEventListener("selectionchange", this.clearSelection); - // Keep the release click from opening the thread after a drag. - this.window.setTimeout(() => { - this.document.removeEventListener("click", this.suppressClick, true); - }, 0); + // 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. From dfb6381cdbed9efe669b568bff7e32d806cba27f Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 6 Sep 2026 19:22:29 -0700 Subject: [PATCH 3/6] fix(web): restore smooth dragging and stabilize empty pin targets --- apps/web/src/components/Sidebar.drag.test.ts | 23 +++++++++++++++ apps/web/src/components/Sidebar.drag.ts | 26 ++++++++++++++--- apps/web/src/components/Sidebar.pointer.ts | 1 + apps/web/src/components/Sidebar.tsx | 30 +++++++++++++------- apps/web/src/index.css | 15 ++++++++++ 5 files changed, 80 insertions(+), 15 deletions(-) diff --git a/apps/web/src/components/Sidebar.drag.test.ts b/apps/web/src/components/Sidebar.drag.test.ts index 1fb4453370a5..47af8f20bccb 100644 --- a/apps/web/src/components/Sidebar.drag.test.ts +++ b/apps/web/src/components/Sidebar.drag.test.ts @@ -150,6 +150,29 @@ describe("sidebar collision detection", () => { expect(detector(args)[0]?.id).toBe(sidebarMarkerId("pinned-header")); }); + it.each([114, 400])( + "keeps empty Pins selected across its opened slot from pickup y=%s", + (activationY) => { + const args = clampedArgs(); + const detector = createSidebarCollisionDetection(() => true, { + emptyPins: true, + activationY, + emptyPinCardId: "source", + boundaryLabelHeight: 24, + }); + const at = (y: number) => detector({ ...args, pointerCoordinates: { x: 130, y } })[0]?.id; + expect(at(108)).toBe(sidebarMarkerId("pinned-header")); + // The pointer crosses the old 8px cue, then moves through the visible slot. + expect(at(109)).toBe(sidebarMarkerId("pinned-header")); + expect(at(160)).toBe(sidebarMarkerId("pinned-header")); + expect(at(207)).toBe(sidebarMarkerId("pinned-header")); + expect(at(208)).toBe("source"); + // Returning to the ordinary list does not immediately re-open Pins. + expect(at(160)).toBe("source"); + expect(at(108)).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 }, diff --git a/apps/web/src/components/Sidebar.drag.ts b/apps/web/src/components/Sidebar.drag.ts index 4e14b3db82f2..ff3b6a4ccc19 100644 --- a/apps/web/src/components/Sidebar.drag.ts +++ b/apps/web/src/components/Sidebar.drag.ts @@ -18,22 +18,36 @@ type Layout = Parameters[0]; * Recreate this detector when drop eligibility changes. */ export function createSidebarCollisionDetection( isValidTarget: (id: string) => boolean, - options: { emptyPins?: boolean; activationY?: number | null } = {}, + options: { + emptyPins?: boolean; + activationY?: number | null; + emptyPinCardId?: string | null; + boundaryLabelHeight?: number; + } = {}, ): CollisionDetection { const validity = new Map(); const pinnedHeaderId = sidebarMarkerId("pinned-header"); + let overEmptyPins = false; return (args) => { let collisions = closestCenter(args); const pinnedRect = options.emptyPins ? args.droppableRects.get(pinnedHeaderId) : undefined; const pointer = args.pointerCoordinates; + const cardHeight = options.emptyPinCardId + ? args.droppableRects.get(options.emptyPinCardId)?.height + : undefined; + // Once Pins opens a slot, keep it selected until the pointer leaves that + // slot. Reusing the original 8px cue makes tiny movements collapse it. + const pinTargetHeight = overEmptyPins + ? (cardHeight ?? 82) + (options.boundaryLabelHeight ?? 0) * ((cardHeight ?? 82) / 82) + 1 + : 8; // 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 && + (overEmptyPins || pointer.y <= options.activationY - 6) && + pointer.y <= pinnedRect.top + pinTargetHeight && pointer.x >= pinnedRect.left && pointer.x <= pinnedRect.right ) { @@ -43,10 +57,14 @@ export function createSidebarCollisionDetection( } } const nearest = collisions[0]; - if (!nearest || nearest.id === args.active.id) return collisions; + if (!nearest || nearest.id === args.active.id) { + overEmptyPins = false; + return collisions; + } const id = String(nearest.id); const valid = validity.get(id) ?? isValidTarget(id); validity.set(id, valid); + overEmptyPins = valid && id === pinnedHeaderId; return valid ? collisions : collisions.filter((collision) => collision.id === args.active.id); }; } diff --git a/apps/web/src/components/Sidebar.pointer.ts b/apps/web/src/components/Sidebar.pointer.ts index 9852c6d04af9..f79bc06c64be 100644 --- a/apps/web/src/components/Sidebar.pointer.ts +++ b/apps/web/src/components/Sidebar.pointer.ts @@ -85,6 +85,7 @@ export class SidebarPointerSensor { this.document.addEventListener("selectionchange", this.clearSelection); this.clearSelection(); this.props.onStart(this.coordinates()); + return; } if (this.phase === "dragging") { if (event.cancelable) event.preventDefault(); diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 0dba4b5924c8..06f857566602 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -501,8 +501,6 @@ function SortableThreadRow(props: { id: props.id, disabled: { draggable: props.disabled }, animateLayoutChanges: animateSidebarLayoutChanges, - // Apply label clearance and row positions together, without tweening through content. - transition: null, }); // dnd-kit memoizes each field but not the bag, so the memoized row would // rerender on every shell update without this. @@ -533,7 +531,6 @@ function SortableSidebarMarker(props: { id: sidebarMarkerId(props.marker), disabled: { draggable: true }, animateLayoutChanges: animateSidebarLayoutChanges, - transition: null, }); return (
    • {props.visible ? ( -
      +
      { - 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: @@ -3368,7 +3369,12 @@ export default function Sidebar() { }).kind !== "none" ); }, - { emptyPins: pinnedKeys.length === 0, activationY: dragState.activationY }, + { + emptyPins: pinnedKeys.length === 0, + activationY: dragActivationY ?? null, + emptyPinCardId: activeKeys[0] ?? null, + boundaryLabelHeight: SIDEBAR_DRAG_LABEL_HEIGHT, + }, ); }, [ activeKeysById, @@ -3376,7 +3382,9 @@ export default function Sidebar() { serverConfigs, activeKeys, activeReorderableThreadKeys, - dragState, + draggedThreadKey, + draggedFromSection, + dragActivationY, draggableThreadKeys, pinnedKeys, sidebarListItems, diff --git a/apps/web/src/index.css b/apps/web/src/index.css index 55d4728b2794..55eaa990f29d 100644 --- a/apps/web/src/index.css +++ b/apps/web/src/index.css @@ -2056,3 +2056,18 @@ code { transform: scaleX(0.9); } } + +/* Let the sortable rows open label clearance before painting the dividers. + This runs once per pickup; rows retain dnd-kit's normal 200ms transitions. */ +.sidebar-drag-boundary-label { + animation: sidebar-drag-label-reveal 200ms step-end; +} + +@keyframes sidebar-drag-label-reveal { + from { + visibility: hidden; + } + to { + visibility: visible; + } +} From f35f2ea059e2fc6e9b7efb2ce1ea27aa74f2d736 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 6 Sep 2026 19:37:00 -0700 Subject: [PATCH 4/6] fix(web): keep dragged threads below the Pins divider --- apps/web/src/components/Sidebar.drag.test.ts | 50 +++++++++++++++++++- apps/web/src/components/Sidebar.drag.ts | 13 ++++- apps/web/src/components/Sidebar.tsx | 30 +++++++++++- 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/apps/web/src/components/Sidebar.drag.test.ts b/apps/web/src/components/Sidebar.drag.test.ts index 47af8f20bccb..f14ba4ca794d 100644 --- a/apps/web/src/components/Sidebar.drag.test.ts +++ b/apps/web/src/components/Sidebar.drag.test.ts @@ -1,7 +1,11 @@ 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 { sidebarListItemId, sidebarMarkerId, @@ -727,3 +731,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 ff3b6a4ccc19..5908006822b1 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,6 +14,17 @@ 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( diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 06f857566602..5a6739a11ed3 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -7,6 +7,7 @@ import { 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"; @@ -171,7 +172,11 @@ 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 { @@ -3015,8 +3020,15 @@ export default function Sidebar() { }, [unsnoozeThread], ); + const threadListRef = useRef(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); @@ -3198,6 +3210,16 @@ 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, @@ -4575,7 +4597,11 @@ export default function Sidebar() { Date: Sun, 6 Sep 2026 19:48:24 -0700 Subject: [PATCH 5/6] fix(web): switch sidebar sections at the visible divider --- apps/web/src/components/Sidebar.drag.test.ts | 165 ++++++++++--------- apps/web/src/components/Sidebar.drag.ts | 72 ++++---- apps/web/src/components/Sidebar.tsx | 4 +- 3 files changed, 126 insertions(+), 115 deletions(-) diff --git a/apps/web/src/components/Sidebar.drag.test.ts b/apps/web/src/components/Sidebar.drag.test.ts index f14ba4ca794d..6c32797e9536 100644 --- a/apps/web/src/components/Sidebar.drag.test.ts +++ b/apps/web/src/components/Sidebar.drag.test.ts @@ -7,6 +7,7 @@ import { restrictBelowSidebarLabel, } from "./Sidebar.drag"; import { + resolveSidebarDropTarget, sidebarListItemId, sidebarMarkerId, type SidebarListItem, @@ -123,93 +124,93 @@ 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([114, 400])( - "keeps empty Pins selected across its opened slot from pickup y=%s", - (activationY) => { - const args = clampedArgs(); + it.each([ + { 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, { - emptyPins: true, - activationY, - emptyPinCardId: "source", - boundaryLabelHeight: 24, + items, + activationY: sourceSection === "pinned" ? 200 : 600, }); - const at = (y: number) => detector({ ...args, pointerCoordinates: { x: 130, y } })[0]?.id; - expect(at(108)).toBe(sidebarMarkerId("pinned-header")); - // The pointer crosses the old 8px cue, then moves through the visible slot. - expect(at(109)).toBe(sidebarMarkerId("pinned-header")); - expect(at(160)).toBe(sidebarMarkerId("pinned-header")); - expect(at(207)).toBe(sidebarMarkerId("pinned-header")); - expect(at(208)).toBe("source"); - // Returning to the ordinary list does not immediately re-open Pins. - expect(at(160)).toBe("source"); - expect(at(108)).toBe(sidebarMarkerId("pinned-header")); + 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.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")]]); - }); - it("returns no collision if an unsupported target has no source fallback", () => { const args = collisionArgs(); const detector = createSidebarCollisionDetection(() => false); diff --git a/apps/web/src/components/Sidebar.drag.ts b/apps/web/src/components/Sidebar.drag.ts index 5908006822b1..715bf65753e4 100644 --- a/apps/web/src/components/Sidebar.drag.ts +++ b/apps/web/src/components/Sidebar.drag.ts @@ -30,52 +30,64 @@ export function restrictBelowSidebarLabel( export function createSidebarCollisionDetection( isValidTarget: (id: string) => boolean, options: { - emptyPins?: boolean; + items?: readonly SidebarListItem[]; activationY?: number | null; - emptyPinCardId?: string | null; - boundaryLabelHeight?: number; } = {}, ): CollisionDetection { const validity = new Map(); - const pinnedHeaderId = sidebarMarkerId("pinned-header"); - let overEmptyPins = false; + 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; - const cardHeight = options.emptyPinCardId - ? args.droppableRects.get(options.emptyPinCardId)?.height - : undefined; - // Once Pins opens a slot, keep it selected until the pointer leaves that - // slot. Reusing the original 8px cue makes tiny movements collapse it. - const pinTargetHeight = overEmptyPins - ? (cardHeight ?? 82) + (options.boundaryLabelHeight ?? 0) * ((cardHeight ?? 82) / 82) + 1 - : 8; - // 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 && - (overEmptyPins || pointer.y <= options.activationY - 6) && - pointer.y <= pinnedRect.top + pinTargetHeight && - 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) { - overEmptyPins = false; return collisions; } const id = String(nearest.id); const valid = validity.get(id) ?? isValidTarget(id); validity.set(id, valid); - overEmptyPins = valid && id === pinnedHeaderId; return valid ? collisions : collisions.filter((collision) => collision.id === args.active.id); }; } diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 5a6739a11ed3..6d285748e810 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -3392,10 +3392,8 @@ export default function Sidebar() { ); }, { - emptyPins: pinnedKeys.length === 0, + items: sidebarListItems, activationY: dragActivationY ?? null, - emptyPinCardId: activeKeys[0] ?? null, - boundaryLabelHeight: SIDEBAR_DRAG_LABEL_HEIGHT, }, ); }, [ From 4aa3a09070752a3f610c8d01629c2b2f6c9907d1 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 6 Sep 2026 20:03:02 -0700 Subject: [PATCH 6/6] fix(web): collapse empty sidebar drop targets after dragging --- apps/web/src/components/Sidebar.drag.test.ts | 15 ++++++++------ apps/web/src/components/Sidebar.drag.ts | 17 +++++++++++----- apps/web/src/components/Sidebar.tsx | 21 ++++++++++++-------- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/apps/web/src/components/Sidebar.drag.test.ts b/apps/web/src/components/Sidebar.drag.test.ts index 6c32797e9536..5c8c95f34400 100644 --- a/apps/web/src/components/Sidebar.drag.test.ts +++ b/apps/web/src/components/Sidebar.drag.test.ts @@ -40,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; @@ -317,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, @@ -331,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([ @@ -467,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) => { @@ -619,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"), diff --git a/apps/web/src/components/Sidebar.drag.ts b/apps/web/src/components/Sidebar.drag.ts index 715bf65753e4..0aef9dec14b1 100644 --- a/apps/web/src/components/Sidebar.drag.ts +++ b/apps/web/src/components/Sidebar.drag.ts @@ -126,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") @@ -137,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; @@ -200,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.tsx b/apps/web/src/components/Sidebar.tsx index 6d285748e810..deb00e7181c7 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -554,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; @@ -566,13 +566,18 @@ function SidebarSectionPlaceholder(props: { - {props.showHint ? props.label : null} + {props.showHint ? ( +
      + {props.label} +
      + ) : null}       ); }