refactor: separate resizing logic from pointer (#8155)

* separate resizing logic for a single element

* replace resize logic in stats

* do not recompute width and height from points when they're already given

* correctly update linear elements' position when resized

* update snapshots

* lint

* simplify linear resizing logic

* fix initial scale for aspect ratio

* update tests for linear elements

* test typo

* separate pointer from resizing for multiple elements

* lint and simplify

* fix tests

* lint

* provide scene in param instead

* type

* refactor code

* fix floating in tests

* remove restrictions/checks on width & height

* update pointer to dimension to prevent regression

---------

Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com>

packages/excalidraw/actions/actionFlip.ts

@@ -12,7 +12,6 @@ import { resizeMultipleElements } from "../element/resizeElements";
 import type { AppClassProperties, AppState } from "../types";
 import { arrayToMap } from "../utils";
 import { CODES, KEYS } from "../keys";
-import { getCommonBoundingBox } from "../element/bounds";
 import {
   bindOrUnbindLinearElements,
   isBindingEnabled,
@@ -27,6 +26,7 @@ import {
 } from "../element/typeChecks";
 import { mutateElbowArrow } from "../element/routing";
 import { mutateElement, newElementWith } from "../element/mutateElement";
+import { getCommonBoundingBox } from "../element/bounds";
 
 export const actionFlipHorizontal = register({
   name: "flipHorizontal",
@@ -132,19 +132,14 @@ const flipElements = (
     });
   }
 
-  const { minX, minY, maxX, maxY, midX, midY } =
+  const { midX, midY } = getCommonBoundingBox(selectedElements);
-    getCommonBoundingBox(selectedElements);
 
-  resizeMultipleElements(
+  resizeMultipleElements(selectedElements, elementsMap, "nw", app.scene, {
-    elementsMap,
+    flipByX: flipDirection === "horizontal",
-    selectedElements,
+    flipByY: flipDirection === "vertical",
-    elementsMap,
+    shouldResizeFromCenter: true,
-    "nw",
+    shouldMaintainAspectRatio: true,
-    true,
+  });
-    true,
-    flipDirection === "horizontal" ? maxX : minX,
-    flipDirection === "horizontal" ? minY : maxY,
-  );
 
   bindOrUnbindLinearElements(
     selectedElements.filter(isLinearElement),

packages/excalidraw/components/App.tsx

@@ -10570,6 +10570,7 @@ class App extends React.Component<AppProps, AppState> {
         transformHandleType,
         selectedElements,
         this.scene.getElementsMapIncludingDeleted(),
+        this.scene,
         shouldRotateWithDiscreteAngle(event),
         shouldResizeFromCenter(event),
         selectedElements.some((element) => isImageElement(element))

packages/excalidraw/components/Stats/Dimension.tsx

@@ -1,8 +1,9 @@
 import type { ExcalidrawElement } from "../../element/types";
 import DragInput from "./DragInput";
 import type { DragInputCallbackType } from "./DragInput";
-import { getStepSizedValue, isPropertyEditable, resizeElement } from "./utils";
+import { getStepSizedValue, isPropertyEditable } from "./utils";
 import { MIN_WIDTH_OR_HEIGHT } from "../../constants";
+import { resizeSingleElement } from "../../element/resizeElements";
 import type Scene from "../../scene/Scene";
 import type { AppState } from "../../types";
 import { isImageElement } from "../../element/typeChecks";
@@ -30,6 +31,7 @@ const handleDimensionChange: DragInputCallbackType<
 > = ({
   accumulatedChange,
   originalElements,
+  originalElementsMap,
   shouldKeepAspectRatio,
   shouldChangeByStepSize,
   nextValue,
@@ -39,9 +41,9 @@ const handleDimensionChange: DragInputCallbackType<
   scene,
 }) => {
   const elementsMap = scene.getNonDeletedElementsMap();
-  const elements = scene.getNonDeletedElements();
   const origElement = originalElements[0];
-  if (origElement) {
+  const latestElement = elementsMap.get(origElement.id);
+  if (origElement && latestElement) {
     const keepAspectRatio =
       shouldKeepAspectRatio || _shouldKeepAspectRatio(origElement);
     const aspectRatio = origElement.width / origElement.height;
@@ -165,14 +167,17 @@ const handleDimensionChange: DragInputCallbackType<
         MIN_WIDTH_OR_HEIGHT,
       );
 
-      resizeElement(
+      resizeSingleElement(
         nextWidth,
         nextHeight,
-        keepAspectRatio,
+        latestElement,
         origElement,
         elementsMap,
-        elements,
+        originalElementsMap,
-        scene,
+        property === "width" ? "e" : "s",
+        {
+          shouldMaintainAspectRatio: keepAspectRatio,
+        },
       );
 
       return;
@@ -209,14 +214,17 @@ const handleDimensionChange: DragInputCallbackType<
     nextHeight = Math.max(MIN_WIDTH_OR_HEIGHT, nextHeight);
     nextWidth = Math.max(MIN_WIDTH_OR_HEIGHT, nextWidth);
 
-    resizeElement(
+    resizeSingleElement(
       nextWidth,
       nextHeight,
-      keepAspectRatio,
+      latestElement,
       origElement,
       elementsMap,
-      elements,
+      originalElementsMap,
-      scene,
+      property === "width" ? "e" : "s",
+      {
+        shouldMaintainAspectRatio: keepAspectRatio,
+      },
     );
   }
 };

packages/excalidraw/components/Stats/MultiDimension.tsx

@@ -2,7 +2,10 @@ import { useMemo } from "react";
 import { getCommonBounds, isTextElement } from "../../element";
 import { updateBoundElements } from "../../element/binding";
 import { mutateElement } from "../../element/mutateElement";
-import { rescalePointsInElement } from "../../element/resizeElements";
+import {
+  rescalePointsInElement,
+  resizeSingleElement,
+} from "../../element/resizeElements";
 import {
   getBoundTextElement,
   handleBindTextResize,
@@ -17,7 +20,7 @@ import type { AppState } from "../../types";
 import DragInput from "./DragInput";
 import type { DragInputCallbackType } from "./DragInput";
 import { getAtomicUnits, getStepSizedValue, isPropertyEditable } from "./utils";
-import { getElementsInAtomicUnit, resizeElement } from "./utils";
+import { getElementsInAtomicUnit } from "./utils";
 import type { AtomicUnit } from "./utils";
 import { MIN_WIDTH_OR_HEIGHT } from "../../constants";
 import { pointFrom, type GlobalPoint } from "../../../math";
@@ -150,7 +153,6 @@ const handleDimensionChange: DragInputCallbackType<
   property,
 }) => {
   const elementsMap = scene.getNonDeletedElementsMap();
-  const elements = scene.getNonDeletedElements();
   const atomicUnits = getAtomicUnits(originalElements, originalAppState);
   if (nextValue !== undefined) {
     for (const atomicUnit of atomicUnits) {
@@ -223,15 +225,17 @@ const handleDimensionChange: DragInputCallbackType<
           nextWidth = Math.max(MIN_WIDTH_OR_HEIGHT, nextWidth);
           nextHeight = Math.max(MIN_WIDTH_OR_HEIGHT, nextHeight);
 
-          resizeElement(
+          resizeSingleElement(
             nextWidth,
             nextHeight,
-            false,
+            latestElement,
             origElement,
             elementsMap,
-            elements,
+            originalElementsMap,
-            scene,
+            property === "width" ? "e" : "s",
-            false,
+            {
+              shouldInformMutation: false,
+            },
           );
         }
       }
@@ -324,14 +328,17 @@ const handleDimensionChange: DragInputCallbackType<
         nextWidth = Math.max(MIN_WIDTH_OR_HEIGHT, nextWidth);
         nextHeight = Math.max(MIN_WIDTH_OR_HEIGHT, nextHeight);
 
-        resizeElement(
+        resizeSingleElement(
           nextWidth,
           nextHeight,
-          false,
+          latestElement,
           origElement,
           elementsMap,
-          elements,
+          originalElementsMap,
-          scene,
+          property === "width" ? "e" : "s",
+          {
+            shouldInformMutation: false,
+          },
         );
       }
     }

packages/excalidraw/components/Stats/utils.ts

@@ -5,17 +5,7 @@ import {
   updateBoundElements,
 } from "../../element/binding";
 import { mutateElement } from "../../element/mutateElement";
-import {
+import { getBoundTextElement } from "../../element/textElement";
-  measureFontSizeFromWidth,
-  rescalePointsInElement,
-} from "../../element/resizeElements";
-import {
-  getApproxMinLineHeight,
-  getApproxMinLineWidth,
-  getBoundTextElement,
-  getBoundTextMaxWidth,
-  handleBindTextResize,
-} from "../../element/textElement";
 import {
   isFrameLikeElement,
   isLinearElement,
@@ -34,7 +24,6 @@ import {
 } from "../../groups";
 import type Scene from "../../scene/Scene";
 import type { AppState } from "../../types";
-import { getFontString } from "../../utils";
 
 export type StatsInputProperty =
   | "x"
@@ -121,95 +110,6 @@ export const newOrigin = (
   };
 };
 
-export const resizeElement = (
-  nextWidth: number,
-  nextHeight: number,
-  keepAspectRatio: boolean,
-  origElement: ExcalidrawElement,
-  elementsMap: NonDeletedSceneElementsMap,
-  elements: readonly NonDeletedExcalidrawElement[],
-  scene: Scene,
-  shouldInformMutation = true,
-) => {
-  const latestElement = elementsMap.get(origElement.id);
-  if (!latestElement) {
-    return;
-  }
-  let boundTextFont: { fontSize?: number } = {};
-  const boundTextElement = getBoundTextElement(latestElement, elementsMap);
-
-  if (boundTextElement) {
-    const minWidth = getApproxMinLineWidth(
-      getFontString(boundTextElement),
-      boundTextElement.lineHeight,
-    );
-    const minHeight = getApproxMinLineHeight(
-      boundTextElement.fontSize,
-      boundTextElement.lineHeight,
-    );
-    nextWidth = Math.max(nextWidth, minWidth);
-    nextHeight = Math.max(nextHeight, minHeight);
-  }
-
-  mutateElement(
-    latestElement,
-    {
-      ...newOrigin(
-        latestElement.x,
-        latestElement.y,
-        latestElement.width,
-        latestElement.height,
-        nextWidth,
-        nextHeight,
-        latestElement.angle,
-      ),
-      width: nextWidth,
-      height: nextHeight,
-      ...rescalePointsInElement(origElement, nextWidth, nextHeight, true),
-    },
-    shouldInformMutation,
-  );
-  updateBindings(latestElement, elementsMap, elements, scene, {
-    newSize: {
-      width: nextWidth,
-      height: nextHeight,
-    },
-  });
-
-  if (boundTextElement) {
-    boundTextFont = {
-      fontSize: boundTextElement.fontSize,
-    };
-    if (keepAspectRatio) {
-      const updatedElement = {
-        ...latestElement,
-        width: nextWidth,
-        height: nextHeight,
-      };
-
-      const nextFont = measureFontSizeFromWidth(
-        boundTextElement,
-        elementsMap,
-        getBoundTextMaxWidth(updatedElement, boundTextElement),
-      );
-      boundTextFont = {
-        fontSize: nextFont?.size ?? boundTextElement.fontSize,
-      };
-    }
-  }
-
-  updateBoundElements(latestElement, elementsMap, {
-    newSize: { width: nextWidth, height: nextHeight },
-  });
-
-  if (boundTextElement && boundTextFont) {
-    mutateElement(boundTextElement, {
-      fontSize: boundTextFont.fontSize,
-    });
-  }
-  handleBindTextResize(latestElement, elementsMap, "e", keepAspectRatio);
-};
-
 export const moveElement = (
   newTopLeftX: number,
   newTopLeftY: number,

packages/excalidraw/tests/resize.test.tsx

@@ -18,6 +18,8 @@ import { LinearElementEditor } from "../element/linearElementEditor";
 import { arrayToMap } from "../utils";
 import type { LocalPoint } from "../../math";
 import { pointFrom } from "../../math";
+import { resizeSingleElement } from "../element/resizeElements";
+import { getSizeFromPoints } from "../points";
 
 ReactDOM.unmountComponentAtNode(document.getElementById("root")!);
 
@@ -235,7 +237,7 @@ describe.each(["line", "freedraw"] as const)("%s element", (type) => {
   };
 
   it("resizes", async () => {
-    const element = UI.createElement(type, { points: points[type] });
+    const element = UI.createElement("freedraw", { points: points.freedraw });
     const bounds = getBoundsFromPoints(element);
 
     UI.resize(element, "ne", [30, -60]);
@@ -249,7 +251,7 @@ describe.each(["line", "freedraw"] as const)("%s element", (type) => {
   });
 
   it("flips while resizing", async () => {
-    const element = UI.createElement(type, { points: points[type] });
+    const element = UI.createElement("freedraw", { points: points.freedraw });
     const bounds = getBoundsFromPoints(element);
 
     UI.resize(element, "sw", [140, -80]);
@@ -263,7 +265,7 @@ describe.each(["line", "freedraw"] as const)("%s element", (type) => {
   });
 
   it("resizes with locked aspect ratio", async () => {
-    const element = UI.createElement(type, { points: points[type] });
+    const element = UI.createElement("freedraw", { points: points.freedraw });
     const bounds = getBoundsFromPoints(element);
 
     UI.resize(element, "ne", [30, -60], { shift: true });
@@ -280,7 +282,7 @@ describe.each(["line", "freedraw"] as const)("%s element", (type) => {
   });
 
   it("resizes from center", async () => {
-    const element = UI.createElement(type, { points: points[type] });
+    const element = UI.createElement("freedraw", { points: points.freedraw });
     const bounds = getBoundsFromPoints(element);
 
     UI.resize(element, "nw", [-20, -30], { alt: true });
@@ -294,6 +296,147 @@ describe.each(["line", "freedraw"] as const)("%s element", (type) => {
   });
 });
 
+describe("line element", () => {
+  const points: LocalPoint[] = [
+    pointFrom(0, 0),
+    pointFrom(60, -20),
+    pointFrom(20, 40),
+    pointFrom(-40, 0),
+  ];
+
+  it("resizes", async () => {
+    UI.createElement("line", { points });
+
+    const element = h.elements[0] as ExcalidrawLinearElement;
+
+    const {
+      x: prevX,
+      y: prevY,
+      width: prevWidth,
+      height: prevHeight,
+    } = element;
+
+    const nextWidth = prevWidth + 30;
+    const nextHeight = prevHeight + 30;
+
+    resizeSingleElement(
+      nextWidth,
+      nextHeight,
+      element,
+      element,
+      h.app.scene.getNonDeletedElementsMap(),
+      h.app.scene.getNonDeletedElementsMap(),
+      "ne",
+    );
+
+    expect(element.x).not.toBe(prevX);
+    expect(element.y).not.toBe(prevY);
+
+    expect(element.width).toBe(nextWidth);
+    expect(element.height).toBe(nextHeight);
+
+    expect(element.points[0]).toEqual([0, 0]);
+
+    const { width, height } = getSizeFromPoints(element.points);
+    expect(width).toBe(element.width);
+    expect(height).toBe(element.height);
+  });
+
+  it("flips while resizing", async () => {
+    UI.createElement("line", { points });
+    const element = h.elements[0] as ExcalidrawLinearElement;
+
+    const {
+      width: prevWidth,
+      height: prevHeight,
+      points: prevPoints,
+    } = element;
+
+    const nextWidth = prevWidth * -1;
+    const nextHeight = prevHeight * -1;
+
+    resizeSingleElement(
+      nextWidth,
+      nextHeight,
+      element,
+      element,
+      h.app.scene.getNonDeletedElementsMap(),
+      h.app.scene.getNonDeletedElementsMap(),
+      "se",
+    );
+
+    expect(element.width).toBe(prevWidth);
+    expect(element.height).toBe(prevHeight);
+
+    element.points.forEach((point, idx) => {
+      expect(point[0]).toBeCloseTo(prevPoints[idx][0] * -1);
+      expect(point[1]).toBeCloseTo(prevPoints[idx][1] * -1);
+    });
+  });
+
+  it("resizes with locked aspect ratio", async () => {
+    UI.createElement("line", { points });
+    const element = h.elements[0] as ExcalidrawLinearElement;
+
+    const { width: prevWidth, height: prevHeight } = element;
+
+    UI.resize(element, "ne", [30, -60], { shift: true });
+
+    const scaleHeight = element.width / prevWidth;
+    const scaleWidth = element.height / prevHeight;
+
+    expect(scaleHeight).toBeCloseTo(scaleWidth);
+  });
+
+  it("resizes from center", async () => {
+    UI.createElement("line", {
+      points: [
+        pointFrom(0, 0),
+        pointFrom(338.05644048727373, -180.4761618151104),
+        pointFrom(338.05644048727373, 180.4761618151104),
+        pointFrom(-338.05644048727373, 180.4761618151104),
+        pointFrom(-338.05644048727373, -180.4761618151104),
+      ],
+    });
+    const element = h.elements[0] as ExcalidrawLinearElement;
+
+    const {
+      x: prevX,
+      y: prevY,
+      width: prevWidth,
+      height: prevHeight,
+    } = element;
+
+    const prevSmallestX = Math.min(...element.points.map((p) => p[0]));
+    const prevBiggestX = Math.max(...element.points.map((p) => p[0]));
+
+    resizeSingleElement(
+      prevWidth + 20,
+      prevHeight,
+      element,
+      element,
+      h.app.scene.getNonDeletedElementsMap(),
+      h.app.scene.getNonDeletedElementsMap(),
+      "e",
+      {
+        shouldResizeFromCenter: true,
+      },
+    );
+
+    expect(element.width).toBeCloseTo(prevWidth + 20);
+    expect(element.height).toBeCloseTo(prevHeight);
+
+    expect(element.x).toBeCloseTo(prevX);
+    expect(element.y).toBeCloseTo(prevY);
+
+    const smallestX = Math.min(...element.points.map((p) => p[0]));
+    const biggestX = Math.max(...element.points.map((p) => p[0]));
+
+    expect(prevSmallestX - smallestX).toBeCloseTo(10);
+    expect(biggestX - prevBiggestX).toBeCloseTo(10);
+  });
+});
+
 describe("arrow element", () => {
   it("resizes with a label", async () => {
     const arrow = UI.createElement("arrow", {