From a4789d718bde6e46f3b7fd425d40d103a8c47357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Wed, 1 Apr 2026 10:50:21 +0200 Subject: [PATCH] refactor(web): centralize multi-select release handling --- .../web/src/hooks/timelineMultiSelect.test.ts | 64 +++++++++++++++++++ apps/web/src/hooks/timelineMultiSelect.ts | 21 ++++++ apps/web/src/hooks/useTimelineDrag.ts | 20 ++---- scripts/check-architecture-guardrails.mjs | 4 ++ .../check-architecture-guardrails.test.mjs | 1 + 5 files changed, 94 insertions(+), 16 deletions(-) diff --git a/apps/web/src/hooks/timelineMultiSelect.test.ts b/apps/web/src/hooks/timelineMultiSelect.test.ts index 8f41e2d..96c134e 100644 --- a/apps/web/src/hooks/timelineMultiSelect.test.ts +++ b/apps/web/src/hooks/timelineMultiSelect.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { + completeMultiSelectDraft, createMultiSelectState, finalizeMultiSelectDraft, updateMultiSelectDraft, @@ -98,4 +99,67 @@ describe("timelineMultiSelect", () => { currentY: 295, }); }); + + it("returns the provided empty state when the right-click drag never really started", () => { + const state = createMultiSelectState(120, 240, { + selectedAllocationIds: ["alloc-1"], + selectedResourceIds: ["res-1"], + dateRange: null, + multiDragDaysDelta: 1, + isMultiDragging: true, + multiDragMode: "move", + }); + const emptyState: TestMultiSelectState = { + isSelecting: false, + startX: 0, + startY: 0, + currentX: 0, + currentY: 0, + selectedAllocationIds: [], + selectedResourceIds: [], + dateRange: null, + multiDragDaysDelta: 0, + isMultiDragging: false, + multiDragMode: "move", + }; + + expect(completeMultiSelectDraft(state, 123, 243, emptyState)).toEqual({ + nextState: emptyState, + shouldClear: true, + }); + }); + + it("keeps the finished rectangle when the drag cleared the minimum distance", () => { + const state = createMultiSelectState(120, 240, { + selectedAllocationIds: [], + selectedResourceIds: [], + dateRange: null, + multiDragDaysDelta: 0, + isMultiDragging: false, + multiDragMode: "move", + }); + const emptyState: TestMultiSelectState = { + isSelecting: false, + startX: 0, + startY: 0, + currentX: 0, + currentY: 0, + selectedAllocationIds: [], + selectedResourceIds: [], + dateRange: null, + multiDragDaysDelta: 0, + isMultiDragging: false, + multiDragMode: "move", + }; + + expect(completeMultiSelectDraft(state, 150, 295, emptyState)).toEqual({ + nextState: { + ...state, + isSelecting: false, + currentX: 150, + currentY: 295, + }, + shouldClear: false, + }); + }); }); diff --git a/apps/web/src/hooks/timelineMultiSelect.ts b/apps/web/src/hooks/timelineMultiSelect.ts index 6a064e2..5c5eac9 100644 --- a/apps/web/src/hooks/timelineMultiSelect.ts +++ b/apps/web/src/hooks/timelineMultiSelect.ts @@ -65,3 +65,24 @@ export function finalizeMultiSelectDraft( currentY, }; } + +export function completeMultiSelectDraft( + state: TState, + currentX: number, + currentY: number, + emptyState: TState, + minDistancePx = 5, +): { nextState: TState; shouldClear: boolean } { + const finished = finalizeMultiSelectDraft(state, currentX, currentY, minDistancePx); + if (!finished) { + return { + nextState: emptyState, + shouldClear: true, + }; + } + + return { + nextState: finished, + shouldClear: false, + }; +} diff --git a/apps/web/src/hooks/useTimelineDrag.ts b/apps/web/src/hooks/useTimelineDrag.ts index 123f18b..9d48641 100644 --- a/apps/web/src/hooks/useTimelineDrag.ts +++ b/apps/web/src/hooks/useTimelineDrag.ts @@ -23,8 +23,8 @@ import { createAllocationDragState } from "./timelineAllocationDragState.js"; import { attachDocumentMouseDrag } from "./timelineDocumentDrag.js"; import { buildProjectShiftMutationInput, createProjectDragState } from "./timelineProjectDrag.js"; import { + completeMultiSelectDraft, createMultiSelectState, - finalizeMultiSelectDraft, updateMultiSelectDraft, } from "./timelineMultiSelect.js"; import { reconcileOptimisticEntries } from "./timelineOptimisticAllocations.js"; @@ -896,21 +896,9 @@ export function useTimelineDrag({ const ms = multiSelectRef.current; if (!ms.isSelecting) return; - const finished = finalizeMultiSelectDraft(ms, ev.clientX, ev.clientY); - if (!finished) { - // Minimal movement → not a drag selection, reset. - // Let existing onContextMenu handlers on allocation blocks handle right-click. - multiSelectRef.current = INITIAL_MULTI_SELECT; - setMultiSelectState(INITIAL_MULTI_SELECT); - return; - } - - // Keep the rectangle coordinates for the parent to compute intersection. - // isSelecting is set to false to indicate the drag is done, but the - // rectangle data (startX/Y, currentX/Y) is preserved so TimelineView - // can resolve which allocations/resources fall within the selection. - multiSelectRef.current = finished; - setMultiSelectState(finished); + const result = completeMultiSelectDraft(ms, ev.clientX, ev.clientY, INITIAL_MULTI_SELECT); + multiSelectRef.current = result.nextState; + setMultiSelectState(result.nextState); } multiSelectCleanupRef.current = attachDocumentMouseDrag(document, handleMove, handleUp); diff --git a/scripts/check-architecture-guardrails.mjs b/scripts/check-architecture-guardrails.mjs index a77f0f7..032432b 100644 --- a/scripts/check-architecture-guardrails.mjs +++ b/scripts/check-architecture-guardrails.mjs @@ -150,6 +150,10 @@ export const rules = [ pattern: /\bexport function finalizeMultiSelectDraft\b/, message: "timeline multi-select helpers must keep minimal-drag reset logic centralized", }, + { + pattern: /\bexport function completeMultiSelectDraft\b/, + message: "timeline multi-select helpers must keep right-click release completion centralized", + }, ], forbidden: [], }, diff --git a/scripts/check-architecture-guardrails.test.mjs b/scripts/check-architecture-guardrails.test.mjs index 2f4f4b6..282ce07 100644 --- a/scripts/check-architecture-guardrails.test.mjs +++ b/scripts/check-architecture-guardrails.test.mjs @@ -124,6 +124,7 @@ describe("architecture guardrails", () => { assert.deepEqual(evaluateRule(multiSelectRule, "export function createMultiSelectState() {}\n"), [ "apps/web/src/hooks/timelineMultiSelect.ts: missing guardrail anchor: timeline multi-select helpers must keep minimal-drag reset logic centralized", + "apps/web/src/hooks/timelineMultiSelect.ts: missing guardrail anchor: timeline multi-select helpers must keep right-click release completion centralized", ]); assert.deepEqual(evaluateRule(rangeRule, "export function updateRangeSelectionDraft() {}\n"), [