From ff2ed5d26a9d0e4af6899d28833542e45dd7f829 Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Thu, 8 May 2025 16:47:13 +0200 Subject: [PATCH] refactor: change movePoints pointUpdates type (#9499) --- packages/common/src/utility-types.ts | 4 + packages/element/src/binding.ts | 23 ++- packages/element/src/flowchart.ts | 18 ++- packages/element/src/linearElementEditor.ts | 148 ++++++++++-------- packages/element/src/types.ts | 5 + .../tests/linearElementEditor.test.tsx | 37 +++-- 6 files changed, 140 insertions(+), 95 deletions(-) diff --git a/packages/common/src/utility-types.ts b/packages/common/src/utility-types.ts index dd26fc397..b5b68a978 100644 --- a/packages/common/src/utility-types.ts +++ b/packages/common/src/utility-types.ts @@ -73,3 +73,7 @@ export type AllPossibleKeys = T extends any ? keyof T : never; export type DTO = { [K in keyof T as T[K] extends Function ? never : K]: T[K]; }; + +export type MapEntry> = M extends Map + ? [K, V] + : never; diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index d44123b71..58f378998 100644 --- a/packages/element/src/binding.ts +++ b/packages/element/src/binding.ts @@ -33,7 +33,7 @@ import type { LocalPoint, Radians } from "@excalidraw/math"; import type { AppState } from "@excalidraw/excalidraw/types"; -import type { Mutable } from "@excalidraw/common/utility-types"; +import type { MapEntry, Mutable } from "@excalidraw/common/utility-types"; import { getCenterForBounds, @@ -84,6 +84,7 @@ import type { ExcalidrawElbowArrowElement, FixedPoint, FixedPointBinding, + PointsPositionUpdates, } from "./types"; export type SuggestedBinding = @@ -801,28 +802,22 @@ export const updateBoundElements = ( bindableElement, elementsMap, ); + if (point) { - return { - index: - bindingProp === "startBinding" ? 0 : element.points.length - 1, - point, - }; + return [ + bindingProp === "startBinding" ? 0 : element.points.length - 1, + { point }, + ] as MapEntry; } } return null; }, ).filter( - ( - update, - ): update is NonNullable<{ - index: number; - point: LocalPoint; - isDragging?: boolean; - }> => update !== null, + (update): update is MapEntry => update !== null, ); - LinearElementEditor.movePoints(element, scene, updates, { + LinearElementEditor.movePoints(element, scene, new Map(updates), { ...(changedElement.id === element.startBinding?.elementId ? { startBinding: bindings.startBinding } : {}), diff --git a/packages/element/src/flowchart.ts b/packages/element/src/flowchart.ts index 62acd1c4e..c537cb719 100644 --- a/packages/element/src/flowchart.ts +++ b/packages/element/src/flowchart.ts @@ -462,12 +462,18 @@ const createBindingArrow = ( bindingArrow as OrderedExcalidrawElement, ); - LinearElementEditor.movePoints(bindingArrow, scene, [ - { - index: 1, - point: bindingArrow.points[1], - }, - ]); + LinearElementEditor.movePoints( + bindingArrow, + scene, + new Map([ + [ + 1, + { + point: bindingArrow.points[1], + }, + ], + ]), + ); const update = updateElbowArrowPoints( bindingArrow, diff --git a/packages/element/src/linearElementEditor.ts b/packages/element/src/linearElementEditor.ts index a6e4a1af7..a0f45f3a7 100644 --- a/packages/element/src/linearElementEditor.ts +++ b/packages/element/src/linearElementEditor.ts @@ -82,6 +82,7 @@ import type { FixedPointBinding, FixedSegment, ExcalidrawElbowArrowElement, + PointsPositionUpdates, } from "./types"; const editorMidPointsCache: { @@ -306,16 +307,22 @@ export class LinearElementEditor { event[KEYS.CTRL_OR_CMD] ? null : app.getEffectiveGridSize(), ); - LinearElementEditor.movePoints(element, scene, [ - { - index: selectedIndex, - point: pointFrom( - width + referencePoint[0], - height + referencePoint[1], - ), - isDragging: selectedIndex === lastClickedPoint, - }, - ]); + LinearElementEditor.movePoints( + element, + scene, + new Map([ + [ + selectedIndex, + { + point: pointFrom( + width + referencePoint[0], + height + referencePoint[1], + ), + isDragging: selectedIndex === lastClickedPoint, + }, + ], + ]), + ); } else { const newDraggingPointPosition = LinearElementEditor.createPointAt( element, @@ -331,26 +338,32 @@ export class LinearElementEditor { LinearElementEditor.movePoints( element, scene, - selectedPointsIndices.map((pointIndex) => { - const newPointPosition: LocalPoint = - pointIndex === lastClickedPoint - ? LinearElementEditor.createPointAt( - element, - elementsMap, - scenePointerX - linearElementEditor.pointerOffset.x, - scenePointerY - linearElementEditor.pointerOffset.y, - event[KEYS.CTRL_OR_CMD] ? null : app.getEffectiveGridSize(), - ) - : pointFrom( - element.points[pointIndex][0] + deltaX, - element.points[pointIndex][1] + deltaY, - ); - return { - index: pointIndex, - point: newPointPosition, - isDragging: pointIndex === lastClickedPoint, - }; - }), + new Map( + selectedPointsIndices.map((pointIndex) => { + const newPointPosition: LocalPoint = + pointIndex === lastClickedPoint + ? LinearElementEditor.createPointAt( + element, + elementsMap, + scenePointerX - linearElementEditor.pointerOffset.x, + scenePointerY - linearElementEditor.pointerOffset.y, + event[KEYS.CTRL_OR_CMD] + ? null + : app.getEffectiveGridSize(), + ) + : pointFrom( + element.points[pointIndex][0] + deltaX, + element.points[pointIndex][1] + deltaY, + ); + return [ + pointIndex, + { + point: newPointPosition, + isDragging: pointIndex === lastClickedPoint, + }, + ]; + }), + ), ); } @@ -451,15 +464,21 @@ export class LinearElementEditor { selectedPoint === element.points.length - 1 ) { if (isPathALoop(element.points, appState.zoom.value)) { - LinearElementEditor.movePoints(element, scene, [ - { - index: selectedPoint, - point: - selectedPoint === 0 - ? element.points[element.points.length - 1] - : element.points[0], - }, - ]); + LinearElementEditor.movePoints( + element, + scene, + new Map([ + [ + selectedPoint, + { + point: + selectedPoint === 0 + ? element.points[element.points.length - 1] + : element.points[0], + }, + ], + ]), + ); } const bindingElement = isBindingEnabled(appState) @@ -988,12 +1007,18 @@ export class LinearElementEditor { } if (lastPoint === lastUncommittedPoint) { - LinearElementEditor.movePoints(element, app.scene, [ - { - index: element.points.length - 1, - point: newPoint, - }, - ]); + LinearElementEditor.movePoints( + element, + app.scene, + new Map([ + [ + element.points.length - 1, + { + point: newPoint, + }, + ], + ]), + ); } else { LinearElementEditor.addPoints(element, app.scene, [{ point: newPoint }]); } @@ -1227,12 +1252,16 @@ export class LinearElementEditor { // potentially expanding the bounding box if (pointAddedToEnd) { const lastPoint = element.points[element.points.length - 1]; - LinearElementEditor.movePoints(element, scene, [ - { - index: element.points.length - 1, - point: pointFrom(lastPoint[0] + 30, lastPoint[1] + 30), - }, - ]); + LinearElementEditor.movePoints( + element, + scene, + new Map([ + [ + element.points.length - 1, + { point: pointFrom(lastPoint[0] + 30, lastPoint[1] + 30) }, + ], + ]), + ); } return { @@ -1307,7 +1336,7 @@ export class LinearElementEditor { static movePoints( element: NonDeleted, scene: Scene, - targetPoints: { index: number; point: LocalPoint; isDragging?: boolean }[], + pointUpdates: PointsPositionUpdates, otherUpdates?: { startBinding?: PointBinding | null; endBinding?: PointBinding | null; @@ -1321,8 +1350,7 @@ export class LinearElementEditor { // offset it. We do the same with actual element.x/y position, so // this hacks are completely transparent to the user. const [deltaX, deltaY] = - targetPoints.find(({ index }) => index === 0)?.point ?? - pointFrom(0, 0); + pointUpdates.get(0)?.point ?? pointFrom(0, 0); const [offsetX, offsetY] = pointFrom( deltaX - points[0][0], deltaY - points[0][1], @@ -1330,12 +1358,12 @@ export class LinearElementEditor { const nextPoints = isElbowArrow(element) ? [ - targetPoints.find((t) => t.index === 0)?.point ?? points[0], - targetPoints.find((t) => t.index === points.length - 1)?.point ?? + pointUpdates.get(0)?.point ?? points[0], + pointUpdates.get(points.length - 1)?.point ?? points[points.length - 1], ] : points.map((p, idx) => { - const current = targetPoints.find((t) => t.index === idx)?.point ?? p; + const current = pointUpdates.get(idx)?.point ?? p; return pointFrom( current[0] - offsetX, @@ -1351,11 +1379,7 @@ export class LinearElementEditor { offsetY, otherUpdates, { - isDragging: targetPoints.reduce( - (dragging, targetPoint): boolean => - dragging || targetPoint.isDragging === true, - false, - ), + isDragging: Array.from(pointUpdates.values()).some((t) => t.isDragging), }, ); } diff --git a/packages/element/src/types.ts b/packages/element/src/types.ts index 1c8c22811..d307b03c6 100644 --- a/packages/element/src/types.ts +++ b/packages/element/src/types.ts @@ -296,6 +296,11 @@ export type FixedPointBinding = Merge< } >; +export type PointsPositionUpdates = Map< + number, + { point: LocalPoint; isDragging?: boolean } +>; + export type Arrowhead = | "arrow" | "bar" diff --git a/packages/excalidraw/tests/linearElementEditor.test.tsx b/packages/excalidraw/tests/linearElementEditor.test.tsx index 2e32e8821..dad797334 100644 --- a/packages/excalidraw/tests/linearElementEditor.test.tsx +++ b/packages/excalidraw/tests/linearElementEditor.test.tsx @@ -1384,19 +1384,30 @@ describe("Test Linear Elements", () => { const [origStartX, origStartY] = [line.x, line.y]; act(() => { - LinearElementEditor.movePoints(line, h.app.scene, [ - { - index: 0, - point: pointFrom(line.points[0][0] + 10, line.points[0][1] + 10), - }, - { - index: line.points.length - 1, - point: pointFrom( - line.points[line.points.length - 1][0] - 10, - line.points[line.points.length - 1][1] - 10, - ), - }, - ]); + LinearElementEditor.movePoints( + line, + h.app.scene, + new Map([ + [ + 0, + { + point: pointFrom( + line.points[0][0] + 10, + line.points[0][1] + 10, + ), + }, + ], + [ + line.points.length - 1, + { + point: pointFrom( + line.points[line.points.length - 1][0] - 10, + line.points[line.points.length - 1][1] - 10, + ), + }, + ], + ]), + ); }); expect(line.x).toBe(origStartX + 10); expect(line.y).toBe(origStartY + 10);