refactor(web): extract document drag listeners

2026-04-01 10:39:28 +02:00
parent 84c5760392
commit eda8722d83
5 changed files with 91 additions and 24 deletions
@@ -0,0 +1,41 @@
+import { describe, expect, it, vi } from "vitest";
+import { attachDocumentMouseDrag } from "./timelineDocumentDrag.js";
+
+describe("timelineDocumentDrag", () => {
+  it("registers move and up listeners and removes both during cleanup", () => {
+    const addEventListener = vi.fn();
+    const removeEventListener = vi.fn();
+    const onMove = vi.fn();
+    const onUp = vi.fn();
+
+    const cleanup = attachDocumentMouseDrag({ addEventListener, removeEventListener }, onMove, onUp);
+
+    expect(addEventListener).toHaveBeenCalledTimes(2);
+    expect(addEventListener).toHaveBeenNthCalledWith(1, "mousemove", onMove);
+    expect(addEventListener).toHaveBeenNthCalledWith(2, "mouseup", onUp);
+
+    cleanup();
+
+    expect(removeEventListener).toHaveBeenCalledTimes(2);
+    expect(removeEventListener).toHaveBeenNthCalledWith(1, "mousemove", onMove);
+    expect(removeEventListener).toHaveBeenNthCalledWith(2, "mouseup", onUp);
+  });
+
+  it("keeps cleanup idempotent from the helper perspective", () => {
+    const removeEventListener = vi.fn();
+
+    const cleanup = attachDocumentMouseDrag(
+      {
+        addEventListener: vi.fn(),
+        removeEventListener,
+      },
+      vi.fn(),
+      vi.fn(),
+    );
+
+    cleanup();
+    cleanup();
+
+    expect(removeEventListener).toHaveBeenCalledTimes(4);
+  });
+});
@@ -0,0 +1,19 @@
+type MouseDragDocumentLike = {
+  addEventListener: (type: "mousemove" | "mouseup", listener: (event: MouseEvent) => void) => void;
+  removeEventListener: (type: "mousemove" | "mouseup", listener: (event: MouseEvent) => void) => void;
+};
+
+export function attachDocumentMouseDrag(
+  doc: MouseDragDocumentLike,
+  onMove: (event: MouseEvent) => void,
+  onUp: (event: MouseEvent) => void,
+): () => void {
+  function cleanup() {
+    doc.removeEventListener("mousemove", onMove);
+    doc.removeEventListener("mouseup", onUp);
+  }
+
+  doc.addEventListener("mousemove", onMove);
+  doc.addEventListener("mouseup", onUp);
+  return cleanup;
+}
@@ -24,6 +24,7 @@ import {
  updateAllocationMultiDrag,
 } from "./timelineAllocationMultiDrag.js";
 import { createAllocationDragState } from "./timelineAllocationDragState.js";
+import { attachDocumentMouseDrag } from "./timelineDocumentDrag.js";
 import { buildProjectShiftMutationInput, createProjectDragState } from "./timelineProjectDrag.js";
 import {
  createMultiSelectState,
@@ -559,12 +560,7 @@ export function useTimelineDrag({
        ev.preventDefault();
      }

-      document.addEventListener("mousemove", handleMove);
-      document.addEventListener("mouseup", handleUp);
-      projectDragCleanupRef.current = () => {
-        document.removeEventListener("mousemove", handleMove);
-        document.removeEventListener("mouseup", handleUp);
-      };
+      projectDragCleanupRef.current = attachDocumentMouseDrag(document, handleMove, handleUp);
    },
    [finalizeProjectDrag, setProjectPreviewTargets, updateProjectDragPosition],
  );
@@ -672,12 +668,7 @@ export function useTimelineDrag({
          }
        }

-        document.addEventListener("mousemove", handleMultiMove);
-        document.addEventListener("mouseup", handleMultiUp);
-        multiSelectCleanupRef.current = () => {
-          document.removeEventListener("mousemove", handleMultiMove);
-          document.removeEventListener("mouseup", handleMultiUp);
-        };
+        multiSelectCleanupRef.current = attachDocumentMouseDrag(document, handleMultiMove, handleMultiUp);
        return;
      }

@@ -796,12 +787,7 @@ export function useTimelineDrag({
        setAllocDragState(INITIAL_ALLOC_DRAG);
      }

-      document.addEventListener("mousemove", handleMove);
-      document.addEventListener("mouseup", handleUp);
-      allocDragCleanupRef.current = () => {
-        document.removeEventListener("mousemove", handleMove);
-        document.removeEventListener("mouseup", handleUp);
-      };
+      allocDragCleanupRef.current = attachDocumentMouseDrag(document, handleMove, handleUp);
    },
    [
      clearPendingOptimisticAllocation,
@@ -943,12 +929,7 @@ export function useTimelineDrag({
      setMultiSelectState(finished);
    }

-    document.addEventListener("mousemove", handleMove);
-    document.addEventListener("mouseup", handleUp);
-    multiSelectCleanupRef.current = () => {
-      document.removeEventListener("mousemove", handleMove);
-      document.removeEventListener("mouseup", handleUp);
-    };
+    multiSelectCleanupRef.current = attachDocumentMouseDrag(document, handleMove, handleUp);
  }, []);

  const clearMultiSelect = useCallback(() => {
@@ -236,6 +236,17 @@ export const rules = [
    ],
    forbidden: [],
  },
+  {
+    file: "apps/web/src/hooks/timelineDocumentDrag.ts",
+    maxLines: 50,
+    required: [
+      {
+        pattern: /\bexport function attachDocumentMouseDrag\b/,
+        message: "timeline document drag helpers must keep document mouse listener wiring centralized",
+      },
+    ],
+    forbidden: [],
+  },
  {
    file: "apps/web/src/hooks/timelineAllocationDragState.ts",
    maxLines: 80,
@@ -293,6 +304,10 @@ export const rules = [
        pattern: /from "\.\/timelineAllocationActions\.js"/,
        message: "timeline drag must keep allocation click and mutation plan derivation delegated to the extracted helper module",
      },
+      {
+        pattern: /from "\.\/timelineDocumentDrag\.js"/,
+        message: "timeline drag must keep document mouse listener lifecycle delegated to the extracted helper module",
+      },
      {
        pattern: /from "\.\/timelineAllocationMultiDrag\.js"/,
        message: "timeline drag must keep allocation multi-drag rules delegated to the extracted helper module",
@@ -323,6 +338,10 @@ export const rules = [
        pattern: /\bfunction (?:buildAllocationBlockClickInfo|buildAllocationMutationPlan)\b/,
        message: "timeline drag must not re-inline extracted allocation action helper implementations",
      },
+      {
+        pattern: /\bfunction attachDocumentMouseDrag\b/,
+        message: "timeline drag must not re-inline extracted document listener helper implementations",
+      },
      {
        pattern: /\bfunction (?:isAllocationMultiSelected|startAllocationMultiDrag|updateAllocationMultiDrag|finalizeAllocationMultiDrag)\b/,
        message: "timeline drag must not re-inline extracted allocation multi-drag helper implementations",
@@ -78,6 +78,7 @@ describe("architecture guardrails", () => {
    const allocationFinalizeRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineAllocationFinalize.ts");
    const allocationMultiDragRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineAllocationMultiDrag.ts");
    const allocationActionsRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineAllocationActions.ts");
+    const documentDragRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineDocumentDrag.ts");
    const allocationDragStateRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineAllocationDragState.ts");
    const projectDragRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineProjectDrag.ts");

@@ -90,6 +91,7 @@ describe("architecture guardrails", () => {
    assert.ok(allocationFinalizeRule);
    assert.ok(allocationMultiDragRule);
    assert.ok(allocationActionsRule);
+    assert.ok(documentDragRule);
    assert.ok(allocationDragStateRule);
    assert.ok(projectDragRule);

@@ -101,6 +103,7 @@ describe("architecture guardrails", () => {
      "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep optimistic allocation reconciliation delegated to the extracted helper module",
      "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep allocation drag completion rules delegated to the extracted helper module",
      "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep allocation click and mutation plan derivation delegated to the extracted helper module",
+      "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep document mouse listener lifecycle delegated to the extracted helper module",
      "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep allocation multi-drag rules delegated to the extracted helper module",
      "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep allocation drag bootstrap delegated to the extracted helper module",
      "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep project drag bootstrap and mutation gating delegated to the extracted helper module",
@@ -144,6 +147,10 @@ describe("architecture guardrails", () => {
      "apps/web/src/hooks/timelineAllocationActions.ts: missing guardrail anchor: timeline allocation action helpers must keep mutation plan derivation centralized",
    ]);

+    assert.deepEqual(evaluateRule(documentDragRule, ""), [
+      "apps/web/src/hooks/timelineDocumentDrag.ts: missing guardrail anchor: timeline document drag helpers must keep document mouse listener wiring centralized",
+    ]);
+
    assert.deepEqual(evaluateRule(allocationDragStateRule, ""), [
      "apps/web/src/hooks/timelineAllocationDragState.ts: missing guardrail anchor: timeline allocation drag state helpers must keep drag bootstrap centralized",
    ]);