fix: restore text dimensions (#5432)

* fix: restore text dimensions

* fix tests

* update readme & changelog

* reduce API surface area by always refreshing dimensions for full `restore()`

src/data/restore.ts

@@ -14,6 +14,7 @@ import {
   getNonDeletedElements,
   getNormalizedDimensions,
   isInvisiblySmallElement,
+  refreshTextDimensions,
 } from "../element";
 import { isLinearElementType } from "../element/typeChecks";
 import { randomId } from "../random";
@@ -138,6 +139,7 @@ const restoreElementWithProperties = <
 
 const restoreElement = (
   element: Exclude<ExcalidrawElement, ExcalidrawSelectionElement>,
+  refreshDimensions = true,
 ): typeof element | null => {
   switch (element.type) {
     case "text":
@@ -150,7 +152,7 @@ const restoreElement = (
         fontSize = parseInt(fontPx, 10);
         fontFamily = getFontFamilyByName(_fontFamily);
       }
-      return restoreElementWithProperties(element, {
+      element = restoreElementWithProperties(element, {
         fontSize,
         fontFamily,
         text: element.text ?? "",
@@ -160,6 +162,11 @@ const restoreElement = (
         containerId: element.containerId ?? null,
         originalText: element.originalText || element.text,
       });
+
+      if (refreshDimensions) {
+        element = { ...element, ...refreshTextDimensions(element) };
+      }
+      return element;
     case "freedraw": {
       return restoreElementWithProperties(element, {
         points: element.points,
@@ -232,13 +239,17 @@ export const restoreElements = (
   elements: ImportedDataState["elements"],
   /** NOTE doesn't serve for reconciliation */
   localElements: readonly ExcalidrawElement[] | null | undefined,
+  refreshDimensions = true,
 ): ExcalidrawElement[] => {
   const localElementsMap = localElements ? arrayToMap(localElements) : null;
   return (elements || []).reduce((elements, element) => {
     // filtering out selection, which is legacy, no longer kept in elements,
     // and causing issues if retained
     if (element.type !== "selection" && !isInvisiblySmallElement(element)) {
-      let migratedElement: ExcalidrawElement | null = restoreElement(element);
+      let migratedElement: ExcalidrawElement | null = restoreElement(
+        element,
+        refreshDimensions,
+      );
       if (migratedElement) {
         const localElement = localElementsMap?.get(element.id);
         if (localElement && localElement.version > migratedElement.version) {
@@ -376,7 +387,7 @@ export const restore = (
   localElements: readonly ExcalidrawElement[] | null | undefined,
 ): RestoredDataState => {
   return {
-    elements: restoreElements(data?.elements, localElements),
+    elements: restoreElements(data?.elements, localElements, true),
     appState: restoreAppState(data?.appState, localAppState || null),
     files: data?.files || {},
   };

src/element/index.ts

@@ -10,6 +10,7 @@ export {
   newElement,
   newTextElement,
   updateTextElement,
+  refreshTextDimensions,
   newLinearElement,
   newImageElement,
   duplicateElement,

src/element/newElement.ts

@@ -252,6 +252,23 @@ const getAdjustedDimensions = (
   };
 };
 
+export const refreshTextDimensions = (
+  textElement: ExcalidrawTextElement,
+  text = textElement.text,
+) => {
+  const container = getContainerElement(textElement);
+  if (container) {
+    // text = wrapText(text, getFontString(textElement), container.width);
+    text = wrapText(
+      text,
+      getFontString(textElement),
+      getMaxContainerWidth(container),
+    );
+  }
+  const dimensions = getAdjustedDimensions(textElement, text);
+  return { text, ...dimensions };
+};
+
 export const getMaxContainerWidth = (container: ExcalidrawElement) => {
   return getContainerDims(container).width - BOUND_TEXT_PADDING * 2;
 };
@@ -272,20 +289,10 @@ export const updateTextElement = (
     originalText: string;
   },
 ): ExcalidrawTextElement => {
-  const container = getContainerElement(textElement);
-  if (container) {
-    text = wrapText(
-      originalText,
-      getFontString(textElement),
-      getMaxContainerWidth(container),
-    );
-  }
-  const dimensions = getAdjustedDimensions(textElement, text);
   return newElementWith(textElement, {
-    text,
     originalText,
     isDeleted: isDeleted ?? textElement.isDeleted,
-    ...dimensions,
+    ...refreshTextDimensions(textElement, originalText),
   });
 };
 

src/excalidraw-app/collab/Collab.tsx

@@ -583,7 +583,7 @@ class Collab extends PureComponent<Props, CollabState> {
     const localElements = this.getSceneElementsIncludingDeleted();
     const appState = this.excalidrawAPI.getAppState();
 
-    remoteElements = restoreElements(remoteElements, null);
+    remoteElements = restoreElements(remoteElements, null, false);
 
     const reconciledElements = _reconcileElements(
       localElements,

src/packages/excalidraw/CHANGELOG.md

@@ -17,6 +17,7 @@ Please add the latest change on the top under the correct section.
 
 #### Features
 
+- `restoreElements()` now takes an optional parameter to indicate whether we should also recalculate text element dimensions. Defaults to `true`, but since this is a potentially costly operation, you may want to disable it if you restore elements in tight loops, such as during collaboration [#5432](https://github.com/excalidraw/excalidraw/pull/5432).
 - Support rendering custom sidebar using [`renderSidebar`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#renderSidebar) prop ([#5663](https://github.com/excalidraw/excalidraw/pull/5663)).
 - Add [`toggleMenu`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#toggleMenu) prop to toggle specific menu open/close state ([#5663](https://github.com/excalidraw/excalidraw/pull/5663)).
 - Support [theme](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#theme) to be semi-controlled [#5660](https://github.com/excalidraw/excalidraw/pull/5660).

src/packages/excalidraw/README.md

@@ -915,7 +915,11 @@ When `localAppState` is supplied, it's used in place of values that are missing
 **_Signature_**
 
 <pre>
-restoreElements(elements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ImportedDataState["elements"]</a>, localElements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ExcalidrawElement[]</a> | null | undefined): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/element/types.ts#L106">ExcalidrawElement[]</a>
+restoreElements(
+  elements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ImportedDataState["elements"]</a>,
+  localElements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ExcalidrawElement[]</a> | null | undefined): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/element/types.ts#L106">ExcalidrawElement[]</a>,
+  refreshDimensions: boolean
+)
 </pre>
 
 **_How to use_**
@@ -928,12 +932,18 @@ This function will make sure all properties of element is correctly set and if a
 
 When `localElements` are supplied, they are used to ensure that existing restored elements reuse `version` (and increment it), and regenerate `versionNonce`. Use this when you import elements which may already be present in the scene to ensure that you do not disregard the newly imported elements if you're using element version to detect the updates.
 
+Parameter `refreshDimensions` indicates whether we should also recalculate text element dimensions. Defaults to `true`, but since this is a potentially costly operation, you may want to disable it if you restore elements in tight loops, such as during collaboration.
+
 #### `restore`
 
 **_Signature_**
 
 <pre>
-restoreElements(data: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L12">ImportedDataState</a>, localAppState: Partial<<a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L79">AppState</a>> | null | undefined, localElements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ExcalidrawElement[]</a> | null | undefined): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L4">DataState</a>
+restoreElements(
+  data: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L12">ImportedDataState</a>,
+  localAppState: Partial<<a href="https://github.com/excalidraw/excalidraw/blob/master/src/types.ts#L79">AppState</a>> | null | undefined,
+  localElements: <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L16">ExcalidrawElement[]</a> | null | undefined): <a href="https://github.com/excalidraw/excalidraw/blob/master/src/data/types.ts#L4">DataState</a>
+)
 </pre>
 
 See [`restoreAppState()`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#restoreAppState) about `localAppState`, and [`restoreElements()`](https://github.com/excalidraw/excalidraw/blob/master/src/packages/excalidraw/README.md#restoreElements) about `localElements`.

src/tests/data/__snapshots__/restore.test.ts.snap

@@ -275,7 +275,7 @@ Object {
   "fontFamily": 1,
   "fontSize": 14,
   "groupIds": Array [],
-  "height": 100,
+  "height": 0,
   "id": "id-text01",
   "isDeleted": false,
   "link": null,
@@ -295,7 +295,7 @@ Object {
   "version": 1,
   "versionNonce": 0,
   "verticalAlign": "middle",
-  "width": 100,
+  "width": 1,
   "x": -0.5,
   "y": 0,
 }
@@ -312,7 +312,7 @@ Object {
   "fontFamily": 1,
   "fontSize": 10,
   "groupIds": Array [],
-  "height": 100,
+  "height": 0,
   "id": "id-text01",
   "isDeleted": false,
   "link": null,
@@ -332,7 +332,7 @@ Object {
   "version": 1,
   "versionNonce": 0,
   "verticalAlign": "top",
-  "width": 100,
+  "width": 1,
   "x": 0,
   "y": 0,
 }