fix polygon behavior when adding/removing/moving points within line editor

packages/element/src/linearElementEditor.ts

@@ -88,11 +88,41 @@ import type {
   PointsPositionUpdates,
 } from "./types";
 
+/**
+ * Normalizes line points so that the start point is at [0,0]. This is
+ * expected in various parts of the codebase.
+ *
+ * Also returns the offsets - [0,0] if no normalization needed.
+ *
+ * @private
+ */
+const getNormalizedPoints = ({
+  points,
+}: {
+  points: ExcalidrawLinearElement["points"];
+}): {
+  points: LocalPoint[];
+  offsetX: number;
+  offsetY: number;
+} => {
+  const offsetX = points[0][0];
+  const offsetY = points[0][1];
+
+  return {
+    points: points.map((p) => {
+      return pointFrom(p[0] - offsetX, p[1] - offsetY);
+    }),
+    offsetX,
+    offsetY,
+  };
+};
+
 const editorMidPointsCache: {
   version: number | null;
   points: (GlobalPoint | null)[];
   zoom: number | null;
 } = { version: null, points: [], zoom: null };
+
 export class LinearElementEditor {
   public readonly elementId: ExcalidrawElement["id"] & {
     _brand: "excalidrawLinearElementId";
@@ -135,7 +165,11 @@ export class LinearElementEditor {
     };
     if (!pointsEqual(element.points[0], pointFrom(0, 0))) {
       console.error("Linear element is not normalized", Error().stack);
-      LinearElementEditor.normalizePoints(element, elementsMap);
+      mutateElement(
+        element,
+        elementsMap,
+        LinearElementEditor.getNormalizeElementPointsAndCoords(element),
+      );
     }
     this.selectedPointsIndices = null;
     this.lastUncommittedPoint = null;
@@ -1033,7 +1067,7 @@ export class LinearElementEditor {
         ]),
       );
     } else {
-      LinearElementEditor.addPoints(element, app.scene, [{ point: newPoint }]);
+      LinearElementEditor.addPoints(element, app.scene, [newPoint]);
     }
     return {
       ...appState.editingLinearElement,
@@ -1176,40 +1210,23 @@ export class LinearElementEditor {
 
   /**
    * Normalizes line points so that the start point is at [0,0]. This is
-   * expected in various parts of the codebase. Also returns new x/y to account
-   * for the potential normalization.
+   * expected in various parts of the codebase.
+   *
+   * Also returns normalized x and y coords to account for the normalization
+   * of the points.
    */
-  static getNormalizedPoints(element: ExcalidrawLinearElement): {
-    points: LocalPoint[];
-    x: number;
-    y: number;
-  } {
-    const { points } = element;
-
-    const offsetX = points[0][0];
-    const offsetY = points[0][1];
+  static getNormalizeElementPointsAndCoords(element: ExcalidrawLinearElement) {
+    const { points, offsetX, offsetY } = getNormalizedPoints(element);
 
     return {
-      points: points.map((p) => {
-        return pointFrom(p[0] - offsetX, p[1] - offsetY);
-      }),
+      points,
       x: element.x + offsetX,
       y: element.y + offsetY,
     };
   }
+
   // element-mutating methods
   // ---------------------------------------------------------------------------
-  static normalizePoints(
-    element: NonDeleted<ExcalidrawLinearElement>,
-    elementsMap: ElementsMap,
-  ) {
-    mutateElement(
-      element,
-      elementsMap,
-      LinearElementEditor.getNormalizedPoints(element),
-    );
-  }
-
   static duplicateSelectedPoints(appState: AppState, scene: Scene): AppState {
     invariant(
       appState.editingLinearElement,
@@ -1291,51 +1308,43 @@ export class LinearElementEditor {
     app: AppClassProperties,
     pointIndices: readonly number[],
   ) {
-    // Handle loop lock behavior
+    const isUncommittedPoint =
+      app.state.editingLinearElement?.lastUncommittedPoint ===
+      element.points[element.points.length - 1];
+
+    // break polygon if deleting start/end point
     if (isLineElement(element) && element.loopLock) {
       if (
         pointIndices.includes(0) ||
         (pointIndices.includes(element.points.length - 1) &&
           // don't disable polygon if cleaning up uncommitted point
-          app.state.editingLinearElement?.lastUncommittedPoint !==
-            element.points[element.points.length - 1])
+          !isUncommittedPoint)
       ) {
         app.scene.mutateElement(element, { loopLock: false });
       }
     }
 
-    let offsetX = 0;
-    let offsetY = 0;
+    const nextPoints = element.points.filter((_, idx) => {
+      return !pointIndices.includes(idx);
+    });
 
-    const isDeletingOriginPoint = pointIndices.includes(0);
-
-    // if deleting first point, make the next to be [0,0] and recalculate
-    // positions of the rest with respect to it
-    if (isDeletingOriginPoint) {
-      const firstNonDeletedPoint = element.points.find((point, idx) => {
-        return !pointIndices.includes(idx);
-      });
-      if (firstNonDeletedPoint) {
-        offsetX = firstNonDeletedPoint[0];
-        offsetY = firstNonDeletedPoint[1];
-      }
+    if (isUncommittedPoint && isLineElement(element) && element.loopLock) {
+      nextPoints[0] = pointFrom(
+        nextPoints[nextPoints.length - 1][0],
+        nextPoints[nextPoints.length - 1][1],
+      );
     }
 
-    const nextPoints = element.points.reduce((acc: LocalPoint[], p, idx) => {
-      if (!pointIndices.includes(idx)) {
-        acc.push(
-          !acc.length
-            ? pointFrom(0, 0)
-            : pointFrom(p[0] - offsetX, p[1] - offsetY),
-        );
-      }
-      return acc;
-    }, []);
+    const {
+      points: normalizedPoints,
+      offsetX,
+      offsetY,
+    } = getNormalizedPoints({ points: nextPoints });
 
     LinearElementEditor._updatePoints(
       element,
       app.scene,
-      nextPoints,
+      normalizedPoints,
       offsetX,
       offsetY,
     );
@@ -1344,16 +1353,27 @@ export class LinearElementEditor {
   static addPoints(
     element: NonDeleted<ExcalidrawLinearElement>,
     scene: Scene,
-    targetPoints: { point: LocalPoint }[],
+    addedPoints: LocalPoint[],
   ) {
-    const offsetX = 0;
-    const offsetY = 0;
+    const nextPoints = [...element.points, ...addedPoints];
+
+    if (isLineElement(element) && element.loopLock) {
+      nextPoints[0] = pointFrom(
+        nextPoints[nextPoints.length - 1][0],
+        nextPoints[nextPoints.length - 1][1],
+      );
+    }
+
+    const {
+      points: normalizedPoints,
+      offsetX,
+      offsetY,
+    } = getNormalizedPoints({ points: nextPoints });
 
-    const nextPoints = [...element.points, ...targetPoints.map((x) => x.point)];
     LinearElementEditor._updatePoints(
       element,
       scene,
-      nextPoints,
+      normalizedPoints,
       offsetX,
       offsetY,
     );
@@ -1396,12 +1416,11 @@ export class LinearElementEditor {
     // all the other points in the opposite direction by delta to
     // offset it. We do the same with actual element.x/y position, so
     // this hacks are completely transparent to the user.
-    const [deltaX, deltaY] =
+
+    const updatedOriginPoint =
       pointUpdates.get(0)?.point ?? pointFrom<LocalPoint>(0, 0);
-    const [offsetX, offsetY] = pointFrom<LocalPoint>(
-      deltaX - points[0][0],
-      deltaY - points[0][1],
-    );
+
+    const [offsetX, offsetY] = updatedOriginPoint;
 
     const nextPoints = isElbowArrow(element)
       ? [
@@ -1571,6 +1590,7 @@ export class LinearElementEditor {
         isDragging: options?.isDragging ?? false,
       });
     } else {
+      // TODO do we need to get precise coords here just to calc centers?
       const nextCoords = getElementPointsCoords(element, nextPoints);
       const prevCoords = getElementPointsCoords(element, element.points);
       const nextCenterX = (nextCoords[0] + nextCoords[2]) / 2;
@@ -1579,7 +1599,7 @@ export class LinearElementEditor {
       const prevCenterY = (prevCoords[1] + prevCoords[3]) / 2;
       const dX = prevCenterX - nextCenterX;
       const dY = prevCenterY - nextCenterY;
-      const rotated = pointRotateRads(
+      const rotatedOffset = pointRotateRads(
         pointFrom(offsetX, offsetY),
         pointFrom(dX, dY),
         element.angle,
@@ -1587,8 +1607,8 @@ export class LinearElementEditor {
       scene.mutateElement(element, {
         ...otherUpdates,
         points: nextPoints,
-        x: element.x + rotated[0],
-        y: element.y + rotated[1],
+        x: element.x + rotatedOffset[0],
+        y: element.y + rotatedOffset[1],
       });
     }
   }

packages/element/src/types.ts

@@ -296,8 +296,10 @@ export type FixedPointBinding = Merge<
   }
 >;
 
+type Index = number;
+
 export type PointsPositionUpdates = Map<
-  number,
+  Index,
   { point: LocalPoint; isDragging?: boolean }
 >;
 
@@ -335,7 +337,7 @@ export type ExcalidrawLineElement = ExcalidrawLinearElement &
 export type FixedSegment = {
   start: LocalPoint;
   end: LocalPoint;
-  index: number;
+  index: Index;
 };
 
 export type ExcalidrawArrowElement = ExcalidrawLinearElement &

packages/excalidraw/data/restore.ts

@@ -324,7 +324,8 @@ const restoreElement = (
           : element.points;
 
       if (points[0][0] !== 0 || points[0][1] !== 0) {
-        ({ points, x, y } = LinearElementEditor.getNormalizedPoints(element));
+        ({ points, x, y } =
+          LinearElementEditor.getNormalizeElementPointsAndCoords(element));
       }
 
       return restoreElementWithProperties(element, {
@@ -355,7 +356,8 @@ const restoreElement = (
           : element.points;
 
       if (points[0][0] !== 0 || points[0][1] !== 0) {
-        ({ points, x, y } = LinearElementEditor.getNormalizedPoints(element));
+        ({ points, x, y } =
+          LinearElementEditor.getNormalizeElementPointsAndCoords(element));
       }
 
       const base = {

packages/excalidraw/data/transform.ts

@@ -469,7 +469,7 @@ const bindLinearElementToElement = (
 
   Object.assign(
     linearElement,
-    LinearElementEditor.getNormalizedPoints({
+    LinearElementEditor.getNormalizeElementPointsAndCoords({
       ...linearElement,
       points: newPoints,
     }),