From 510459fbffff4230965101acf422b33714141931 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Wed, 1 Apr 2026 11:22:18 +0200 Subject: [PATCH] refactor(web): extract allocation multi-drag session --- ...timelineAllocationMultiDragSession.test.ts | 128 ++++++++++++++++++ .../timelineAllocationMultiDragSession.ts | 75 ++++++++++ apps/web/src/hooks/useTimelineDrag.ts | 54 +++----- scripts/check-architecture-guardrails.mjs | 19 +++ .../check-architecture-guardrails.test.mjs | 10 ++ 5 files changed, 249 insertions(+), 37 deletions(-) create mode 100644 apps/web/src/hooks/timelineAllocationMultiDragSession.test.ts create mode 100644 apps/web/src/hooks/timelineAllocationMultiDragSession.ts diff --git a/apps/web/src/hooks/timelineAllocationMultiDragSession.test.ts b/apps/web/src/hooks/timelineAllocationMultiDragSession.test.ts new file mode 100644 index 0000000..bc55a19 --- /dev/null +++ b/apps/web/src/hooks/timelineAllocationMultiDragSession.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it, vi } from "vitest"; +import { beginAllocationMultiDragSession } from "./timelineAllocationMultiDragSession.js"; + +type TestDragMode = "move" | "resize-end"; + +type TestMultiDragState = { + selectedAllocationIds: string[]; + multiDragDaysDelta: number; + isMultiDragging: boolean; + multiDragMode: TestDragMode; +}; + +describe("timelineAllocationMultiDragSession", () => { + it("starts the session, updates on day-bucket changes, and completes with selected ids", () => { + const previousCleanup = vi.fn(); + const setState = vi.fn(); + const onComplete = vi.fn(); + const cleanupRef = { current: previousCleanup as (() => void) | null }; + const initialState: TestMultiDragState = { + selectedAllocationIds: ["a", "b"], + multiDragDaysDelta: 0, + isMultiDragging: false, + multiDragMode: "move", + }; + const stateRef = { + current: initialState, + }; + const handlers: { + move?: (event: MouseEvent) => void; + up?: (event: MouseEvent) => void; + } = {}; + + beginAllocationMultiDragSession({ + startMouseX: 100, + dragMode: "resize-end" as const, + documentTarget: {} as Document, + cleanupRef, + stateRef, + setState, + startState: (state, dragMode): TestMultiDragState => ({ + ...state, + isMultiDragging: true, + multiDragMode: dragMode, + multiDragDaysDelta: 0, + }), + updateState: (state, daysDelta) => (daysDelta === state.multiDragDaysDelta ? null : { ...state, multiDragDaysDelta: daysDelta }), + finalizeState: (state): TestMultiDragState => ({ ...state, isMultiDragging: false, multiDragDaysDelta: 0 }), + toDaysDelta: (deltaX) => Math.round(deltaX / 40), + onComplete, + attachDrag: (_documentTarget, onMove, onUp) => { + handlers.move = onMove; + handlers.up = onUp; + return vi.fn(); + }, + }); + + expect(previousCleanup).toHaveBeenCalledOnce(); + expect(setState).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ isMultiDragging: true, multiDragMode: "resize-end", multiDragDaysDelta: 0 }), + ); + + handlers.move?.({ clientX: 180 } as MouseEvent); + expect(setState).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ isMultiDragging: true, multiDragDaysDelta: 2 }), + ); + + const attachedCleanup = cleanupRef.current; + handlers.up?.({ clientX: 220 } as MouseEvent); + + expect(attachedCleanup).toHaveBeenCalledOnce(); + expect(cleanupRef.current).toBeNull(); + expect(setState).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ isMultiDragging: false, multiDragDaysDelta: 0 }), + ); + expect(onComplete).toHaveBeenCalledWith( + 3, + expect.objectContaining({ selectedAllocationIds: ["a", "b"], isMultiDragging: false, multiDragDaysDelta: 0 }), + ); + }); + + it("skips redundant updates and suppresses completion callbacks for zero-delta releases", () => { + const setState = vi.fn(); + const onComplete = vi.fn(); + let upHandler: ((event: MouseEvent) => void) | undefined; + let moveHandler: ((event: MouseEvent) => void) | undefined; + const initialState: TestMultiDragState = { + selectedAllocationIds: ["a", "b"], + multiDragDaysDelta: 0, + isMultiDragging: false, + multiDragMode: "move", + }; + + beginAllocationMultiDragSession({ + startMouseX: 100, + dragMode: "move" as const, + documentTarget: {} as Document, + cleanupRef: { current: null }, + stateRef: { + current: initialState, + }, + setState, + startState: (state, dragMode): TestMultiDragState => ({ + ...state, + isMultiDragging: true, + multiDragMode: dragMode, + multiDragDaysDelta: 0, + }), + updateState: (state, daysDelta) => (daysDelta === state.multiDragDaysDelta ? null : { ...state, multiDragDaysDelta: daysDelta }), + finalizeState: (state): TestMultiDragState => ({ ...state, isMultiDragging: false, multiDragDaysDelta: 0 }), + toDaysDelta: (deltaX) => Math.round(deltaX / 40), + onComplete, + attachDrag: (_documentTarget, onMove, onUp) => { + moveHandler = onMove; + upHandler = onUp; + return vi.fn(); + }, + }); + + moveHandler?.({ clientX: 110 } as MouseEvent); + upHandler?.({ clientX: 110 } as MouseEvent); + + expect(setState).toHaveBeenCalledTimes(2); + expect(onComplete).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/hooks/timelineAllocationMultiDragSession.ts b/apps/web/src/hooks/timelineAllocationMultiDragSession.ts new file mode 100644 index 0000000..da0ae38 --- /dev/null +++ b/apps/web/src/hooks/timelineAllocationMultiDragSession.ts @@ -0,0 +1,75 @@ +type MutableCurrent = { + current: T; +}; + +type AttachDocumentMouseDrag = ( + documentTarget: Document, + onMove: (event: MouseEvent) => void, + onUp: (event: MouseEvent) => void, +) => () => void; + +type MultiDragStateLike = { + selectedAllocationIds: string[]; + multiDragDaysDelta: number; + isMultiDragging: boolean; + multiDragMode: TMode; +}; + +type BeginAllocationMultiDragSessionParams = { + startMouseX: number; + dragMode: TMode; + documentTarget: Document; + cleanupRef: MutableCurrent<(() => void) | null>; + stateRef: MutableCurrent; + setState: (state: TState) => void; + startState: (state: TState, dragMode: TMode) => TState; + updateState: (state: TState, daysDelta: number) => TState | null; + finalizeState: (state: TState) => TState; + toDaysDelta: (deltaX: number) => number; + onComplete: (daysDelta: number, finalState: TState) => void; + attachDrag: AttachDocumentMouseDrag; +}; + +export function beginAllocationMultiDragSession({ + startMouseX, + dragMode, + documentTarget, + cleanupRef, + stateRef, + setState, + startState, + updateState, + finalizeState, + toDaysDelta, + onComplete, + attachDrag, +}: BeginAllocationMultiDragSessionParams) { + const initialState = startState(stateRef.current, dragMode); + stateRef.current = initialState; + setState(initialState); + cleanupRef.current?.(); + + function handleMove(event: MouseEvent) { + const updated = updateState(stateRef.current, toDaysDelta(event.clientX - startMouseX)); + if (!updated) return; + + stateRef.current = updated; + setState(updated); + } + + function handleUp(event: MouseEvent) { + cleanupRef.current?.(); + cleanupRef.current = null; + + const finalState = finalizeState(stateRef.current); + stateRef.current = finalState; + setState(finalState); + + const finalDelta = toDaysDelta(event.clientX - startMouseX); + if (finalDelta !== 0) { + onComplete(finalDelta, finalState); + } + } + + cleanupRef.current = attachDrag(documentTarget, handleMove, handleUp); +} diff --git a/apps/web/src/hooks/useTimelineDrag.ts b/apps/web/src/hooks/useTimelineDrag.ts index ac4cb10..23eb722 100644 --- a/apps/web/src/hooks/useTimelineDrag.ts +++ b/apps/web/src/hooks/useTimelineDrag.ts @@ -18,6 +18,7 @@ import { startAllocationMultiDrag, updateAllocationMultiDrag, } from "./timelineAllocationMultiDrag.js"; +import { beginAllocationMultiDragSession } from "./timelineAllocationMultiDragSession.js"; import { resolveAllocationRelease } from "./timelineAllocationRelease.js"; import { createAllocationDragState } from "./timelineAllocationDragState.js"; import { cleanupTimelineDragState } from "./timelineDragCleanup.js"; @@ -593,43 +594,22 @@ export function useTimelineDrag({ const isMultiSelected = isAllocationMultiSelected(ms, opts.allocationId); if (isMultiSelected) { - // ── Multi-drag: move/resize all selected allocations together ── - const startMouseX = e.clientX; - const dragMode = opts.mode; - const initialMultiDragState = startAllocationMultiDrag(ms, dragMode); - - setMultiSelectState(initialMultiDragState); - multiSelectRef.current = initialMultiDragState; - multiSelectCleanupRef.current?.(); - - function handleMultiMove(ev: MouseEvent) { - const deltaX = ev.clientX - startMouseX; - const daysDelta = pixelsToDays(deltaX, cellWidthRef.current); - const updated = updateAllocationMultiDrag(multiSelectRef.current, daysDelta); - if (!updated) return; - - setMultiSelectState(updated); - multiSelectRef.current = updated; - } - - function handleMultiUp(ev: MouseEvent) { - multiSelectCleanupRef.current?.(); - multiSelectCleanupRef.current = null; - - const finalDelta = pixelsToDays(ev.clientX - startMouseX, cellWidthRef.current); - const finalized = finalizeAllocationMultiDrag(multiSelectRef.current); - - setMultiSelectState(finalized); - multiSelectRef.current = finalized; - - if (finalDelta !== 0) { - // Pass IDs from ref to avoid stale closure in the callback - const ids = finalized.selectedAllocationIds; - onMultiDragCompleteRef.current?.(finalDelta, dragMode, ids); - } - } - - multiSelectCleanupRef.current = attachDocumentMouseDrag(document, handleMultiMove, handleMultiUp); + beginAllocationMultiDragSession({ + startMouseX: e.clientX, + dragMode: opts.mode, + documentTarget: document, + cleanupRef: multiSelectCleanupRef, + stateRef: multiSelectRef, + setState: setMultiSelectState, + startState: startAllocationMultiDrag, + updateState: updateAllocationMultiDrag, + finalizeState: finalizeAllocationMultiDrag, + toDaysDelta: (deltaX) => pixelsToDays(deltaX, cellWidthRef.current), + onComplete: (daysDelta, finalized) => { + onMultiDragCompleteRef.current?.(daysDelta, opts.mode, finalized.selectedAllocationIds); + }, + attachDrag: attachDocumentMouseDrag, + }); return; } diff --git a/scripts/check-architecture-guardrails.mjs b/scripts/check-architecture-guardrails.mjs index 6bb4e99..fbdab10 100644 --- a/scripts/check-architecture-guardrails.mjs +++ b/scripts/check-architecture-guardrails.mjs @@ -251,6 +251,17 @@ export const rules = [ ], forbidden: [], }, + { + file: "apps/web/src/hooks/timelineAllocationMultiDragSession.ts", + maxLines: 90, + required: [ + { + pattern: /\bexport function beginAllocationMultiDragSession\b/, + message: "timeline allocation multi-drag session helpers must keep document drag lifecycle centralized", + }, + ], + forbidden: [], + }, { file: "apps/web/src/hooks/timelineAllocationActions.ts", maxLines: 90, @@ -410,6 +421,10 @@ export const rules = [ pattern: /from "\.\/timelineAllocationMultiDrag\.js"/, message: "timeline drag must keep allocation multi-drag rules delegated to the extracted helper module", }, + { + pattern: /from "\.\/timelineAllocationMultiDragSession\.js"/, + message: "timeline drag must keep allocation multi-drag document session wiring delegated to the extracted helper module", + }, { pattern: /from "\.\/timelineAllocationDragState\.js"/, message: "timeline drag must keep allocation drag bootstrap delegated to the extracted helper module", @@ -460,6 +475,10 @@ export const rules = [ pattern: /\bfunction (?:isAllocationMultiSelected|startAllocationMultiDrag|updateAllocationMultiDrag|finalizeAllocationMultiDrag)\b/, message: "timeline drag must not re-inline extracted allocation multi-drag helper implementations", }, + { + pattern: /\bfunction handleMulti(?:Move|Up)\b/, + message: "timeline drag must not re-inline extracted allocation multi-drag session handlers", + }, { pattern: /\bfunction createAllocationDragState\b/, message: "timeline drag must not re-inline extracted allocation drag bootstrap helpers", diff --git a/scripts/check-architecture-guardrails.test.mjs b/scripts/check-architecture-guardrails.test.mjs index 4c420e6..f262592 100644 --- a/scripts/check-architecture-guardrails.test.mjs +++ b/scripts/check-architecture-guardrails.test.mjs @@ -79,6 +79,9 @@ describe("architecture guardrails", () => { const optimisticRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineOptimisticAllocations.ts"); 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 allocationMultiDragSessionRule = rules.find( + (rule) => rule.file === "apps/web/src/hooks/timelineAllocationMultiDragSession.ts", + ); const allocationActionsRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineAllocationActions.ts"); const allocationReleaseRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineAllocationRelease.ts"); const cleanupRule = rules.find((rule) => rule.file === "apps/web/src/hooks/timelineDragCleanup.ts"); @@ -98,6 +101,7 @@ describe("architecture guardrails", () => { assert.ok(optimisticRule); assert.ok(allocationFinalizeRule); assert.ok(allocationMultiDragRule); + assert.ok(allocationMultiDragSessionRule); assert.ok(allocationActionsRule); assert.ok(allocationReleaseRule); assert.ok(cleanupRule); @@ -121,6 +125,7 @@ describe("architecture guardrails", () => { "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep project and allocation drag position 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 multi-drag document session wiring 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", "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep project drag document session wiring delegated to the extracted helper module", @@ -170,6 +175,10 @@ describe("architecture guardrails", () => { "apps/web/src/hooks/timelineAllocationMultiDrag.ts: missing guardrail anchor: timeline allocation multi-drag helpers must keep reset-on-release behavior centralized", ]); + assert.deepEqual(evaluateRule(allocationMultiDragSessionRule, ""), [ + "apps/web/src/hooks/timelineAllocationMultiDragSession.ts: missing guardrail anchor: timeline allocation multi-drag session helpers must keep document drag lifecycle centralized", + ]); + assert.deepEqual(evaluateRule(allocationActionsRule, "export function buildAllocationBlockClickInfo() {}\n"), [ "apps/web/src/hooks/timelineAllocationActions.ts: missing guardrail anchor: timeline allocation action helpers must keep mutation plan derivation centralized", ]); @@ -221,6 +230,7 @@ describe("architecture guardrails", () => { "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep project and allocation drag position 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 multi-drag document session wiring 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", "apps/web/src/hooks/useTimelineDrag.ts: missing guardrail anchor: timeline drag must keep project drag document session wiring delegated to the extracted helper module",