fix: make getBoundTextElement and related helpers pure (#7601)

* fix: make getBoundTextElement pure * updating args * fix * pass boundTextElement to getBoundTextMaxWidth * fix labelled arrows * lint * pass elementsMap to removeElementsFromFrame * pass elementsMap to getMaximumGroups, alignElements and distributeElements * lint * pass allElementsMap to renderElement * lint * feat: make more typesafe * fix: remove unnecessary assertion * fix: remove unused params --------- Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com>
2025-05-03 10:00:07 -04:00 · 2024-01-26 11:29:07 +05:30 · 2024-01-26 11:29:07 +05:30 · 10bd08ef19
commit 10bd08ef19
parent 2789d08154
34 changed files with 385 additions and 143 deletions
--- a/packages/excalidraw/element/binding.ts
+++ b/packages/excalidraw/element/binding.ts
@ -321,9 +321,9 @@ export const updateBoundElements = (
  const simultaneouslyUpdatedElementIds = getSimultaneouslyUpdatedElementIds(
    simultaneouslyUpdated,
  );
-
+  const scene = Scene.getScene(changedElement)!;
  getNonDeletedElements(
-    Scene.getScene(changedElement)!,
+    scene,
    boundLinearElements.map((el) => el.id),
  ).forEach((element) => {
    if (!isLinearElement(element)) {
@ -362,9 +362,12 @@ export const updateBoundElements = (
      endBinding,
      changedElement as ExcalidrawBindableElement,
    );
-    const boundText = getBoundTextElement(element);
+    const boundText = getBoundTextElement(
+      element,
+      scene.getNonDeletedElementsMap(),
+    );
    if (boundText) {
-      handleBindTextResize(element, false);
+      handleBindTextResize(element, scene.getNonDeletedElementsMap(), false);
    }
  });
 };
--- a/packages/excalidraw/element/bounds.ts
+++ b/packages/excalidraw/element/bounds.ts
@ -6,6 +6,7 @@ import {
  NonDeleted,
  ExcalidrawTextElementWithContainer,
  ElementsMapOrArray,
+  ElementsMap,
 } from "./types";
 import { distance2d, rotate, rotatePoint } from "../math";
 import rough from "roughjs/bin/rough";
@ -74,13 +75,16 @@ export class ElementBounds {
    ) {
      return cachedBounds.bounds;
    }
-
-    const bounds = ElementBounds.calculateBounds(element);
+    const scene = Scene.getScene(element);
+    const bounds = ElementBounds.calculateBounds(
+      element,
+      scene?.getNonDeletedElementsMap() || new Map(),
+    );

    // hack to ensure that downstream checks could retrieve element Scene
    // so as to have correctly calculated bounds
    // FIXME remove when we get rid of all the id:Scene / element:Scene mapping
-    const shouldCache = Scene.getScene(element);
+    const shouldCache = !!scene;

    if (shouldCache) {
      ElementBounds.boundsCache.set(element, {
@ -92,7 +96,10 @@ export class ElementBounds {
    return bounds;
  }

-  private static calculateBounds(element: ExcalidrawElement): Bounds {
+  private static calculateBounds(
+    element: ExcalidrawElement,
+    elementsMap: ElementsMap,
+  ): Bounds {
    let bounds: Bounds;

    const [x1, y1, x2, y2, cx, cy] = getElementAbsoluteCoords(element);
@ -111,7 +118,7 @@ export class ElementBounds {
        maxY + element.y,
      ];
    } else if (isLinearElement(element)) {
-      bounds = getLinearElementRotatedBounds(element, cx, cy);
+      bounds = getLinearElementRotatedBounds(element, cx, cy, elementsMap);
    } else if (element.type === "diamond") {
      const [x11, y11] = rotate(cx, y1, cx, cy, element.angle);
      const [x12, y12] = rotate(cx, y2, cx, cy, element.angle);
@ -154,16 +161,17 @@ export const getElementAbsoluteCoords = (
  element: ExcalidrawElement,
  includeBoundText: boolean = false,
 ): [number, number, number, number, number, number] => {
+  const elementsMap =
+    Scene.getScene(element)?.getElementsMapIncludingDeleted() || new Map();
  if (isFreeDrawElement(element)) {
    return getFreeDrawElementAbsoluteCoords(element);
  } else if (isLinearElement(element)) {
    return LinearElementEditor.getElementAbsoluteCoords(
      element,
+      elementsMap,
      includeBoundText,
    );
  } else if (isTextElement(element)) {
-    const elementsMap =
-      Scene.getScene(element)?.getElementsMapIncludingDeleted();
    const container = elementsMap
      ? getContainerElement(element, elementsMap)
      : null;
@ -677,7 +685,10 @@ const getLinearElementRotatedBounds = (
  element: ExcalidrawLinearElement,
  cx: number,
  cy: number,
+  elementsMap: ElementsMap,
 ): Bounds => {
+  const boundTextElement = getBoundTextElement(element, elementsMap);
+
  if (element.points.length < 2) {
    const [pointX, pointY] = element.points[0];
    const [x, y] = rotate(
@ -689,7 +700,6 @@ const getLinearElementRotatedBounds = (
    );

    let coords: Bounds = [x, y, x, y];
-    const boundTextElement = getBoundTextElement(element);
    if (boundTextElement) {
      const coordsWithBoundText = LinearElementEditor.getMinMaxXYWithBoundText(
        element,
@ -714,7 +724,6 @@ const getLinearElementRotatedBounds = (
    rotate(element.x + x, element.y + y, cx, cy, element.angle);
  const res = getMinMaxXYFromCurvePathOps(ops, transformXY);
  let coords: Bounds = [res[0], res[1], res[2], res[3]];
-  const boundTextElement = getBoundTextElement(element);
  if (boundTextElement) {
    const coordsWithBoundText = LinearElementEditor.getMinMaxXYWithBoundText(
      element,
--- a/packages/excalidraw/element/collision.ts
+++ b/packages/excalidraw/element/collision.ts
@ -28,6 +28,7 @@ import {
  StrokeRoundness,
  ExcalidrawFrameLikeElement,
  ExcalidrawIframeLikeElement,
+  ElementsMap,
 } from "./types";

 import {
@ -78,6 +79,7 @@ export const hitTest = (
  frameNameBoundsCache: FrameNameBoundsCache,
  x: number,
  y: number,
+  elementsMap: ElementsMap,
 ): boolean => {
  // How many pixels off the shape boundary we still consider a hit
  const threshold = 10 / appState.zoom.value;
@ -95,7 +97,7 @@ export const hitTest = (
    );
  }

-  const boundTextElement = getBoundTextElement(element);
+  const boundTextElement = getBoundTextElement(element, elementsMap);
  if (boundTextElement) {
    const isHittingBoundTextElement = hitTest(
      boundTextElement,
@ -103,6 +105,7 @@ export const hitTest = (
      frameNameBoundsCache,
      x,
      y,
+      elementsMap,
    );
    if (isHittingBoundTextElement) {
      return true;
@ -122,15 +125,16 @@ export const isHittingElementBoundingBoxWithoutHittingElement = (
  frameNameBoundsCache: FrameNameBoundsCache,
  x: number,
  y: number,
+  elementsMap: ElementsMap,
 ): boolean => {
  const threshold = 10 / appState.zoom.value;

  // So that bound text element hit is considered within bounding box of container even if its outside actual bounding box of element
  // eg for linear elements text can be outside the element bounding box
-  const boundTextElement = getBoundTextElement(element);
+  const boundTextElement = getBoundTextElement(element, elementsMap);
  if (
    boundTextElement &&
-    hitTest(boundTextElement, appState, frameNameBoundsCache, x, y)
+    hitTest(boundTextElement, appState, frameNameBoundsCache, x, y, elementsMap)
  ) {
    return false;
  }
--- a/packages/excalidraw/element/dragElements.ts
+++ b/packages/excalidraw/element/dragElements.ts
@ -57,7 +57,10 @@ export const dragSelectedElements = (
      // skip arrow labels since we calculate its position during render
      !isArrowElement(element)
    ) {
-      const textElement = getBoundTextElement(element);
+      const textElement = getBoundTextElement(
+        element,
+        scene.getNonDeletedElementsMap(),
+      );
      if (textElement) {
        updateElementCoords(pointerDownState, textElement, adjustedOffset);
      }
--- a/packages/excalidraw/element/linearElementEditor.ts
+++ b/packages/excalidraw/element/linearElementEditor.ts
@ -5,6 +5,7 @@ import {
  PointBinding,
  ExcalidrawBindableElement,
  ExcalidrawTextElementWithContainer,
+  ElementsMap,
 } from "./types";
 import {
  distance2d,
@ -193,6 +194,7 @@ export class LinearElementEditor {
      pointSceneCoords: { x: number; y: number }[],
    ) => void,
    linearElementEditor: LinearElementEditor,
+    elementsMap: ElementsMap,
  ): boolean {
    if (!linearElementEditor) {
      return false;
@ -272,9 +274,9 @@ export class LinearElementEditor {
        );
      }

-      const boundTextElement = getBoundTextElement(element);
+      const boundTextElement = getBoundTextElement(element, elementsMap);
      if (boundTextElement) {
-        handleBindTextResize(element, false);
+        handleBindTextResize(element, elementsMap, false);
      }

      // suggest bindings for first and last point if selected
@ -404,9 +406,10 @@ export class LinearElementEditor {

  static getEditorMidPoints = (
    element: NonDeleted<ExcalidrawLinearElement>,
+    elementsMap: ElementsMap,
    appState: InteractiveCanvasAppState,
  ): typeof editorMidPointsCache["points"] => {
-    const boundText = getBoundTextElement(element);
+    const boundText = getBoundTextElement(element, elementsMap);

    // Since its not needed outside editor unless 2 pointer lines or bound text
    if (
@ -465,6 +468,7 @@ export class LinearElementEditor {
    linearElementEditor: LinearElementEditor,
    scenePointer: { x: number; y: number },
    appState: AppState,
+    elementsMap: ElementsMap,
  ) => {
    const { elementId } = linearElementEditor;
    const element = LinearElementEditor.getElement(elementId);
@ -503,7 +507,7 @@ export class LinearElementEditor {
    }
    let index = 0;
    const midPoints: typeof editorMidPointsCache["points"] =
-      LinearElementEditor.getEditorMidPoints(element, appState);
+      LinearElementEditor.getEditorMidPoints(element, elementsMap, appState);
    while (index < midPoints.length) {
      if (midPoints[index] !== null) {
        const distance = distance2d(
@ -581,6 +585,7 @@ export class LinearElementEditor {
    linearElementEditor: LinearElementEditor,
    appState: AppState,
    midPoint: Point,
+    elementsMap: ElementsMap,
  ) {
    const element = LinearElementEditor.getElement(
      linearElementEditor.elementId,
@ -588,7 +593,11 @@ export class LinearElementEditor {
    if (!element) {
      return -1;
    }
-    const midPoints = LinearElementEditor.getEditorMidPoints(element, appState);
+    const midPoints = LinearElementEditor.getEditorMidPoints(
+      element,
+      elementsMap,
+      appState,
+    );
    let index = 0;
    while (index < midPoints.length) {
      if (LinearElementEditor.arePointsEqual(midPoint, midPoints[index])) {
@ -605,6 +614,7 @@ export class LinearElementEditor {
    history: History,
    scenePointer: { x: number; y: number },
    linearElementEditor: LinearElementEditor,
+    elementsMap: ElementsMap,
  ): {
    didAddPoint: boolean;
    hitElement: NonDeleted<ExcalidrawElement> | null;
@ -630,6 +640,7 @@ export class LinearElementEditor {
      linearElementEditor,
      scenePointer,
      appState,
+      elementsMap,
    );
    let segmentMidpointIndex = null;
    if (segmentMidpoint) {
@ -637,6 +648,7 @@ export class LinearElementEditor {
        linearElementEditor,
        appState,
        segmentMidpoint,
+        elementsMap,
      );
    }
    if (event.altKey && appState.editingLinearElement) {
@ -1418,6 +1430,7 @@ export class LinearElementEditor {

  static getElementAbsoluteCoords = (
    element: ExcalidrawLinearElement,
+    elementsMap: ElementsMap,
    includeBoundText: boolean = false,
  ): [number, number, number, number, number, number] => {
    let coords: [number, number, number, number, number, number];
@ -1462,7 +1475,7 @@ export class LinearElementEditor {
    if (!includeBoundText) {
      return coords;
    }
-    const boundTextElement = getBoundTextElement(element);
+    const boundTextElement = getBoundTextElement(element, elementsMap);
    if (boundTextElement) {
      coords = LinearElementEditor.getMinMaxXYWithBoundText(
        element,
--- a/packages/excalidraw/element/newElement.ts
+++ b/packages/excalidraw/element/newElement.ts
@ -342,7 +342,7 @@ export const refreshTextDimensions = (
    text = wrapText(
      text,
      getFontString(textElement),
-      getBoundTextMaxWidth(container),
+      getBoundTextMaxWidth(container, textElement),
    );
  }
  const dimensions = getAdjustedDimensions(textElement, text);
--- a/packages/excalidraw/element/resizeElements.ts
+++ b/packages/excalidraw/element/resizeElements.ts
@ -126,6 +126,7 @@ export const transformElements = (
      rotateMultipleElements(
        originalElements,
        selectedElements,
+        elementsMap,
        pointerX,
        pointerY,
        shouldRotateWithDiscreteAngle,
@ -219,7 +220,7 @@ const measureFontSizeFromWidth = (
  if (hasContainer) {
    const container = getContainerElement(element, elementsMap);
    if (container) {
-      width = getBoundTextMaxWidth(container);
+      width = getBoundTextMaxWidth(container, element);
    }
  }
  const nextFontSize = element.fontSize * (nextWidth / width);
@ -394,7 +395,7 @@ export const resizeSingleElement = (
  let scaleY = atStartBoundsHeight / boundsCurrentHeight;

  let boundTextFont: { fontSize?: number; baseline?: number } = {};
-  const boundTextElement = getBoundTextElement(element);
+  const boundTextElement = getBoundTextElement(element, elementsMap);

  if (transformHandleDirection.includes("e")) {
    scaleX = (rotatedPointer[0] - startTopLeft[0]) / boundsCurrentWidth;
@ -458,7 +459,7 @@ export const resizeSingleElement = (
      const nextFont = measureFontSizeFromWidth(
        boundTextElement,
        elementsMap,
-        getBoundTextMaxWidth(updatedElement),
+        getBoundTextMaxWidth(updatedElement, boundTextElement),
        getBoundTextMaxHeight(updatedElement, boundTextElement),
      );
      if (nextFont === null) {
@ -640,6 +641,7 @@ export const resizeSingleElement = (
    }
    handleBindTextResize(
      element,
+      elementsMap,
      transformHandleDirection,
      shouldMaintainAspectRatio,
    );
@ -882,7 +884,7 @@ export const resizeMultipleElements = (
      newSize: { width, height },
    });

-    const boundTextElement = getBoundTextElement(element);
+    const boundTextElement = getBoundTextElement(element, elementsMap);
    if (boundTextElement && boundTextFontSize) {
      mutateElement(
        boundTextElement,
@ -892,7 +894,7 @@ export const resizeMultipleElements = (
        },
        false,
      );
-      handleBindTextResize(element, transformHandleType, true);
+      handleBindTextResize(element, elementsMap, transformHandleType, true);
    }
  }

@ -902,6 +904,7 @@ export const resizeMultipleElements = (
 const rotateMultipleElements = (
  originalElements: PointerDownState["originalElements"],
  elements: readonly NonDeletedExcalidrawElement[],
+  elementsMap: ElementsMap,
  pointerX: number,
  pointerY: number,
  shouldRotateWithDiscreteAngle: boolean,
@ -941,7 +944,7 @@ const rotateMultipleElements = (
      );
      updateBoundElements(element, { simultaneouslyUpdated: elements });

-      const boundText = getBoundTextElement(element);
+      const boundText = getBoundTextElement(element, elementsMap);
      if (boundText && !isArrowElement(element)) {
        mutateElement(
          boundText,
--- a/packages/excalidraw/element/textElement.test.ts
+++ b/packages/excalidraw/element/textElement.test.ts
@ -319,17 +319,17 @@ describe("Test measureText", () => {

    it("should return max width when container is rectangle", () => {
      const container = API.createElement({ type: "rectangle", ...params });
-      expect(getBoundTextMaxWidth(container)).toBe(168);
+      expect(getBoundTextMaxWidth(container, null)).toBe(168);
    });

    it("should return max width when container is ellipse", () => {
      const container = API.createElement({ type: "ellipse", ...params });
-      expect(getBoundTextMaxWidth(container)).toBe(116);
+      expect(getBoundTextMaxWidth(container, null)).toBe(116);
    });

    it("should return max width when container is diamond", () => {
      const container = API.createElement({ type: "diamond", ...params });
-      expect(getBoundTextMaxWidth(container)).toBe(79);
+      expect(getBoundTextMaxWidth(container, null)).toBe(79);
    });
  });

--- a/packages/excalidraw/element/textElement.ts
+++ b/packages/excalidraw/element/textElement.ts
@ -23,7 +23,6 @@ import {
  VERTICAL_ALIGN,
 } from "../constants";
 import { MaybeTransformHandleType } from "./transformHandles";
-import Scene from "../scene/Scene";
 import { isTextElement } from ".";
 import { isBoundToContainer, isArrowElement } from "./typeChecks";
 import { LinearElementEditor } from "./linearElementEditor";
@ -89,7 +88,7 @@ export const redrawTextBoundingBox = (
      container,
      textElement as ExcalidrawTextElementWithContainer,
    );
-    const maxContainerWidth = getBoundTextMaxWidth(container);
+    const maxContainerWidth = getBoundTextMaxWidth(container, textElement);

    if (!isArrowElement(container) && metrics.height > maxContainerHeight) {
      const nextHeight = computeContainerDimensionForBoundText(
@ -162,6 +161,7 @@ export const bindTextToShapeAfterDuplication = (

 export const handleBindTextResize = (
  container: NonDeletedExcalidrawElement,
+  elementsMap: ElementsMap,
  transformHandleType: MaybeTransformHandleType,
  shouldMaintainAspectRatio = false,
 ) => {
@ -170,25 +170,17 @@ export const handleBindTextResize = (
    return;
  }
  resetOriginalContainerCache(container.id);
-  let textElement = Scene.getScene(container)!.getElement(
-    boundTextElementId,
-  ) as ExcalidrawTextElement;
+  const textElement = getBoundTextElement(container, elementsMap);
  if (textElement && textElement.text) {
    if (!container) {
      return;
    }

-    textElement = Scene.getScene(container)!.getElement(
-      boundTextElementId,
-    ) as ExcalidrawTextElement;
    let text = textElement.text;
    let nextHeight = textElement.height;
    let nextWidth = textElement.width;
-    const maxWidth = getBoundTextMaxWidth(container);
-    const maxHeight = getBoundTextMaxHeight(
-      container,
-      textElement as ExcalidrawTextElementWithContainer,
-    );
+    const maxWidth = getBoundTextMaxWidth(container, textElement);
+    const maxHeight = getBoundTextMaxHeight(container, textElement);
    let containerHeight = container.height;
    let nextBaseLine = textElement.baseline;
    if (
@ -243,10 +235,7 @@ export const handleBindTextResize = (
    if (!isArrowElement(container)) {
      mutateElement(
        textElement,
-        computeBoundTextPosition(
-          container,
-          textElement as ExcalidrawTextElementWithContainer,
-        ),
+        computeBoundTextPosition(container, textElement),
      );
    }
  }
@ -264,7 +253,7 @@ export const computeBoundTextPosition = (
  }
  const containerCoords = getContainerCoords(container);
  const maxContainerHeight = getBoundTextMaxHeight(container, boundTextElement);
-  const maxContainerWidth = getBoundTextMaxWidth(container);
+  const maxContainerWidth = getBoundTextMaxWidth(container, boundTextElement);

  let x;
  let y;
@ -667,17 +656,18 @@ export const getBoundTextElementId = (container: ExcalidrawElement | null) => {
    : null;
 };

-export const getBoundTextElement = (element: ExcalidrawElement | null) => {
+export const getBoundTextElement = (
+  element: ExcalidrawElement | null,
+  elementsMap: ElementsMap,
+) => {
  if (!element) {
    return null;
  }
  const boundTextElementId = getBoundTextElementId(element);
+
  if (boundTextElementId) {
-    return (
-      (Scene.getScene(element)?.getElement(
-        boundTextElementId,
-      ) as ExcalidrawTextElementWithContainer) || null
-    );
+    return (elementsMap.get(boundTextElementId) ||
+      null) as ExcalidrawTextElementWithContainer | null;
  }
  return null;
 };
@ -699,6 +689,7 @@ export const getContainerElement = (
 export const getContainerCenter = (
  container: ExcalidrawElement,
  appState: AppState,
+  elementsMap: ElementsMap,
 ) => {
  if (!isArrowElement(container)) {
    return {
@ -718,6 +709,7 @@ export const getContainerCenter = (
  const index = container.points.length / 2 - 1;
  let midSegmentMidpoint = LinearElementEditor.getEditorMidPoints(
    container,
+    elementsMap,
    appState,
  )[index];
  if (!midSegmentMidpoint) {
@ -877,9 +869,7 @@ export const computeContainerDimensionForBoundText = (

 export const getBoundTextMaxWidth = (
  container: ExcalidrawElement,
-  boundTextElement: ExcalidrawTextElement | null = getBoundTextElement(
-    container,
-  ),
+  boundTextElement: ExcalidrawTextElement | null,
 ) => {
  const { width } = container;
  if (isArrowElement(container)) {
--- a/packages/excalidraw/element/textWysiwyg.tsx
+++ b/packages/excalidraw/element/textWysiwyg.tsx
@ -34,6 +34,7 @@ import {
  computeContainerDimensionForBoundText,
  detectLineHeight,
  computeBoundTextPosition,
+  getBoundTextElement,
 } from "./textElement";
 import {
  actionDecreaseFontSize,
@ -196,7 +197,8 @@ export const textWysiwyg = ({
          }
        }

-        maxWidth = getBoundTextMaxWidth(container);
+        maxWidth = getBoundTextMaxWidth(container, updatedTextElement);
+
        maxHeight = getBoundTextMaxHeight(
          container,
          updatedTextElement as ExcalidrawTextElementWithContainer,
@ -361,10 +363,14 @@ export const textWysiwyg = ({
        fontFamily: app.state.currentItemFontFamily,
      });
      if (container) {
+        const boundTextElement = getBoundTextElement(
+          container,
+          app.scene.getNonDeletedElementsMap(),
+        );
        const wrappedText = wrapText(
          `${editable.value}${data}`,
          font,
-          getBoundTextMaxWidth(container),
+          getBoundTextMaxWidth(container, boundTextElement),
        );
        const width = getTextWidth(wrappedText, font);
        editable.style.width = `${width}px`;
--- a/packages/excalidraw/element/types.ts
+++ b/packages/excalidraw/element/types.ts
@ -279,6 +279,16 @@ export type NonDeletedElementsMap = Map<
 export type SceneElementsMap = Map<ExcalidrawElement["id"], ExcalidrawElement> &
  MakeBrand<"SceneElementsMap">;

+/**
+ * Map of all non-deleted Scene elements.
+ * Not a subset. Use this type when you need access to current Scene elements.
+ */
+export type NonDeletedSceneElementsMap = Map<
+  ExcalidrawElement["id"],
+  NonDeletedExcalidrawElement
+> &
+  MakeBrand<"NonDeletedSceneElementsMap">;
+
 export type ElementsMapOrArray =
  | readonly ExcalidrawElement[]
  | Readonly<ElementsMap>;