diff --git a/apps/web/src/hooks/timelineDocumentDrag.test.ts b/apps/web/src/hooks/timelineDocumentDrag.test.ts new file mode 100644 index 0000000..dbd36f8 --- /dev/null +++ b/apps/web/src/hooks/timelineDocumentDrag.test.ts @@ -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); + }); +}); diff --git a/apps/web/src/hooks/timelineDocumentDrag.ts b/apps/web/src/hooks/timelineDocumentDrag.ts new file mode 100644 index 0000000..4288b50 --- /dev/null +++ b/apps/web/src/hooks/timelineDocumentDrag.ts @@ -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; +} diff --git a/apps/web/src/hooks/useTimelineDrag.ts b/apps/web/src/hooks/useTimelineDrag.ts index 07a4f51..a84dada 100644 --- a/apps/web/src/hooks/useTimelineDrag.ts +++ b/apps/web/src/hooks/useTimelineDrag.ts @@ -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(() => { diff --git a/scripts/check-architecture-guardrails.mjs b/scripts/check-architecture-guardrails.mjs index fce1fdb..97d5cc2 100644 --- a/scripts/check-architecture-guardrails.mjs +++ b/scripts/check-architecture-guardrails.mjs @@ -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", diff --git a/scripts/check-architecture-guardrails.test.mjs b/scripts/check-architecture-guardrails.test.mjs index 9a20177..d08c8c9 100644 --- a/scripts/check-architecture-guardrails.test.mjs +++ b/scripts/check-architecture-guardrails.test.mjs @@ -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", ]);