Merge remote-tracking branch 'origin/release' into danieljgeiger-mathjax

src/clients.ts

@@ -21,7 +21,7 @@ export const getClientColors = (clientId: string, appState: AppState) => {
 };
 
 export const getClientInitials = (userName?: string | null) => {
-  if (!userName) {
+  if (!userName?.trim()) {
     return "?";
   }
   return userName.trim()[0].toUpperCase();

src/components/App.tsx

@@ -227,6 +227,7 @@ import {
   setEraserCursor,
   updateActiveTool,
   getShortcutKey,
+  isTransparent,
 } from "../utils";
 import {
   ContextMenu,
@@ -884,7 +885,7 @@ class App extends React.Component<AppProps, AppState> {
         },
       };
     }
-    const scene = restore(initialData, null, null);
+    const scene = restore(initialData, null, null, { repairBindings: true });
     scene.appState = {
       ...scene.appState,
       theme: this.props.theme || scene.appState.theme,
@@ -2827,7 +2828,15 @@ class App extends React.Component<AppProps, AppState> {
         sceneY,
       );
       if (container) {
-        if (isArrowElement(container) || hasBoundTextElement(container)) {
+        if (
+          isArrowElement(container) ||
+          hasBoundTextElement(container) ||
+          !isTransparent(container.backgroundColor) ||
+          isHittingElementNotConsideringBoundingBox(container, this.state, [
+            sceneX,
+            sceneY,
+          ])
+        ) {
           const midPoint = getContainerCenter(container, this.state);
 
           sceneX = midPoint.x;

src/data/blob.ts

@@ -156,6 +156,7 @@ export const loadSceneOrLibraryFromBlob = async (
           },
           localAppState,
           localElements,
+          { repairBindings: true },
         ),
       };
     } else if (isValidLibrary(data)) {

src/data/restore.ts

@@ -344,7 +344,7 @@ export const restoreElements = (
   elements: ImportedDataState["elements"],
   /** NOTE doesn't serve for reconciliation */
   localElements: readonly ExcalidrawElement[] | null | undefined,
-  refreshDimensions = false,
+  opts?: { refreshDimensions?: boolean; repairBindings?: boolean } | undefined,
 ): ExcalidrawElement[] => {
   const localElementsMap = localElements ? arrayToMap(localElements) : null;
   const restoredElements = (elements || []).reduce((elements, element) => {
@@ -353,7 +353,7 @@ export const restoreElements = (
     if (element.type !== "selection" && !isInvisiblySmallElement(element)) {
       let migratedElement: ExcalidrawElement | null = restoreElement(
         element,
-        refreshDimensions,
+        opts?.refreshDimensions,
       );
       if (migratedElement) {
         const localElement = localElementsMap?.get(element.id);
@@ -366,6 +366,10 @@ export const restoreElements = (
     return elements;
   }, [] as ExcalidrawElement[]);
 
+  if (!opts?.repairBindings) {
+    return restoredElements;
+  }
+
   // repair binding. Mutates elements.
   const restoredElementsMap = arrayToMap(restoredElements);
   for (const element of restoredElements) {
@@ -508,9 +512,10 @@ export const restore = (
    */
   localAppState: Partial<AppState> | null | undefined,
   localElements: readonly ExcalidrawElement[] | null | undefined,
+  elementsConfig?: { refreshDimensions?: boolean; repairBindings?: boolean },
 ): RestoredDataState => {
   return {
-    elements: restoreElements(data?.elements, localElements),
+    elements: restoreElements(data?.elements, localElements, elementsConfig),
     appState: restoreAppState(data?.appState, localAppState || null),
     files: data?.files || {},
   };

src/element/textWysiwyg.test.tsx

@@ -465,14 +465,21 @@ describe("textWysiwyg", () => {
       });
     });
 
-    it("should bind text to container when double clicked on center of filled container", async () => {
+    it("should bind text to container when double clicked inside filled container", async () => {
+      const rectangle = API.createElement({
+        type: "rectangle",
+        x: 10,
+        y: 20,
+        width: 90,
+        height: 75,
+        backgroundColor: "red",
+      });
+      h.elements = [rectangle];
+
       expect(h.elements.length).toBe(1);
       expect(h.elements[0].id).toBe(rectangle.id);
 
-      mouse.doubleClickAt(
-        rectangle.x + rectangle.width / 2,
-        rectangle.y + rectangle.height / 2,
-      );
+      mouse.doubleClickAt(rectangle.x + 10, rectangle.y + 10);
       expect(h.elements.length).toBe(2);
 
       const text = h.elements[1] as ExcalidrawTextElementWithContainer;
@@ -506,24 +513,37 @@ describe("textWysiwyg", () => {
       });
       h.elements = [rectangle];
 
+      mouse.doubleClickAt(rectangle.x + 10, rectangle.y + 10);
+      expect(h.elements.length).toBe(2);
+      let text = h.elements[1] as ExcalidrawTextElementWithContainer;
+      expect(text.type).toBe("text");
+      expect(text.containerId).toBe(null);
+      mouse.down();
+      let editor = document.querySelector(
+        ".excalidraw-textEditorContainer > textarea",
+      ) as HTMLTextAreaElement;
+      await new Promise((r) => setTimeout(r, 0));
+      editor.blur();
+
       mouse.doubleClickAt(
         rectangle.x + rectangle.width / 2,
         rectangle.y + rectangle.height / 2,
       );
-      expect(h.elements.length).toBe(2);
+      expect(h.elements.length).toBe(3);
 
-      const text = h.elements[1] as ExcalidrawTextElementWithContainer;
+      text = h.elements[1] as ExcalidrawTextElementWithContainer;
       expect(text.type).toBe("text");
       expect(text.containerId).toBe(rectangle.id);
+
       mouse.down();
-      const editor = document.querySelector(
+      editor = document.querySelector(
         ".excalidraw-textEditorContainer > textarea",
       ) as HTMLTextAreaElement;
 
       fireEvent.change(editor, { target: { value: "Hello World!" } });
-
       await new Promise((r) => setTimeout(r, 0));
       editor.blur();
+
       expect(rectangle.boundElements).toStrictEqual([
         { id: text.id, type: "text" },
       ]);
@@ -553,6 +573,43 @@ describe("textWysiwyg", () => {
       ]);
     });
 
+    it("should bind text to container when double clicked on container stroke", async () => {
+      const rectangle = API.createElement({
+        type: "rectangle",
+        x: 10,
+        y: 20,
+        width: 90,
+        height: 75,
+        strokeWidth: 4,
+      });
+      h.elements = [rectangle];
+
+      expect(h.elements.length).toBe(1);
+      expect(h.elements[0].id).toBe(rectangle.id);
+
+      mouse.doubleClickAt(rectangle.x + 2, rectangle.y + 2);
+      expect(h.elements.length).toBe(2);
+
+      const text = h.elements[1] as ExcalidrawTextElementWithContainer;
+      expect(text.type).toBe("text");
+      expect(text.containerId).toBe(rectangle.id);
+      expect(rectangle.boundElements).toStrictEqual([
+        { id: text.id, type: "text" },
+      ]);
+      mouse.down();
+      const editor = document.querySelector(
+        ".excalidraw-textEditorContainer > textarea",
+      ) as HTMLTextAreaElement;
+
+      fireEvent.change(editor, { target: { value: "Hello World!" } });
+
+      await new Promise((r) => setTimeout(r, 0));
+      editor.blur();
+      expect(rectangle.boundElements).toStrictEqual([
+        { id: text.id, type: "text" },
+      ]);
+    });
+
     it("shouldn't bind to non-text-bindable containers", async () => {
       const freedraw = API.createElement({
         type: "freedraw",

src/element/typeChecks.ts

@@ -137,7 +137,7 @@ export const isExcalidrawElement = (element: any): boolean => {
 
 export const hasBoundTextElement = (
   element: ExcalidrawElement | null,
-): element is ExcalidrawBindableElement => {
+): element is MarkNonNullable<ExcalidrawBindableElement, "boundElements"> => {
   return (
     isBindableElement(element) &&
     !!element.boundElements?.some(({ type }) => type === "text")

src/excalidraw-app/collab/Collab.tsx

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

src/excalidraw-app/collab/RoomDialog.tsx

@@ -144,7 +144,7 @@ const RoomDialog = ({
               <input
                 type="text"
                 id="username"
-                value={username || ""}
+                value={username.trim() || ""}
                 className="RoomDialog-username TextInput"
                 onChange={(event) => onUsernameChange(event.target.value)}
                 onKeyPress={(event) => event.key === "Enter" && handleClose()}

src/excalidraw-app/data/index.ts

@@ -263,9 +263,12 @@ export const loadScene = async (
       await importFromBackend(id, privateKey),
       localDataState?.appState,
       localDataState?.elements,
+      { repairBindings: true },
     );
   } else {
-    data = restore(localDataState || null, null, null);
+    data = restore(localDataState || null, null, null, {
+      repairBindings: true,
+    });
   }
 
   return {

src/excalidraw-app/index.tsx

@@ -365,7 +365,7 @@ const ExcalidrawWrapper = () => {
           if (data.scene) {
             excalidrawAPI.updateScene({
               ...data.scene,
-              ...restore(data.scene, null, null),
+              ...restore(data.scene, null, null, { repairBindings: true }),
               commitToHistory: true,
             });
           }

src/packages/excalidraw/CHANGELOG.md

@@ -11,6 +11,24 @@ The change should be grouped under one of the below section and must contain PR
 Please add the latest change on the top under the correct section.
 -->
 
+## Unreleased
+
+### Features
+
+- [`restoreElements`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/utils/restore#restoreelements) API now takes an optional parameter `opts` which currently supports the below attributes
+
+```js
+{ refreshDimensions?: boolean, repairBindings?: boolean }
+```
+
+The same `opts` param has been added to [`restore`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/utils/restore#restore) API as well.
+
+For more details refer to the [docs](https://docs.excalidraw.com)
+
+#### BREAKING CHANGE
+
+- The optional parameter `refreshDimensions` in [`restoreElements`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/utils/restore#restoreelements) has been removed and can be enabled via `opts`
+
 ## 0.14.2 (2023-02-01)
 
 ### Features

src/tests/clients.test.ts

@@ -36,4 +36,9 @@ describe("getClientInitials", () => {
     result = getClientInitials(null);
     expect(result).toBe("?");
   });
+
+  it('returns "?" when value is blank', () => {
+    const result = getClientInitials(" ");
+    expect(result).toBe("?");
+  });
 });

src/tests/data/restore.test.ts

@@ -534,7 +534,7 @@ describe("restore", () => {
 });
 
 describe("repairing bindings", () => {
-  it("should repair container boundElements", () => {
+  it("should repair container boundElements when repair is true", () => {
     const container = API.createElement({
       type: "rectangle",
       boundElements: [],
@@ -546,11 +546,28 @@ describe("repairing bindings", () => {
 
     expect(container.boundElements).toEqual([]);
 
-    const restoredElements = restore.restoreElements(
+    let restoredElements = restore.restoreElements(
       [container, boundElement],
       null,
     );
 
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: container.id,
+        boundElements: [],
+      }),
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: container.id,
+      }),
+    ]);
+
+    restoredElements = restore.restoreElements(
+      [container, boundElement],
+      null,
+      { repairBindings: true },
+    );
+
     expect(restoredElements).toEqual([
       expect.objectContaining({
         id: container.id,
@@ -563,7 +580,7 @@ describe("repairing bindings", () => {
     ]);
   });
 
-  it("should repair containerId of boundElements", () => {
+  it("should repair containerId of boundElements when repair is true", () => {
     const boundElement = API.createElement({
       type: "text",
       containerId: null,
@@ -573,11 +590,28 @@ describe("repairing bindings", () => {
       boundElements: [{ type: boundElement.type, id: boundElement.id }],
     });
 
-    const restoredElements = restore.restoreElements(
+    let restoredElements = restore.restoreElements(
       [container, boundElement],
       null,
     );
 
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: container.id,
+        boundElements: [{ type: boundElement.type, id: boundElement.id }],
+      }),
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: null,
+      }),
+    ]);
+
+    restoredElements = restore.restoreElements(
+      [container, boundElement],
+      null,
+      { repairBindings: true },
+    );
+
     expect(restoredElements).toEqual([
       expect.objectContaining({
         id: container.id,
@@ -620,7 +654,7 @@ describe("repairing bindings", () => {
     ]);
   });
 
-  it("should remove bindings of deleted elements from boundElements", () => {
+  it("should remove bindings of deleted elements from boundElements when repair is true", () => {
     const container = API.createElement({
       type: "rectangle",
       boundElements: [],
@@ -642,6 +676,8 @@ describe("repairing bindings", () => {
       type: invisibleBoundElement.type,
       id: invisibleBoundElement.id,
     };
+    expect(container.boundElements).toEqual([]);
+
     const nonExistentBinding = { type: "text", id: "non-existent" };
     // @ts-ignore
     container.boundElements = [
@@ -650,17 +686,28 @@ describe("repairing bindings", () => {
       nonExistentBinding,
     ];
 
-    expect(container.boundElements).toEqual([
-      obsoleteBinding,
-      invisibleBinding,
-      nonExistentBinding,
-    ]);
-
-    const restoredElements = restore.restoreElements(
+    let restoredElements = restore.restoreElements(
       [container, invisibleBoundElement, boundElement],
       null,
     );
 
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: container.id,
+        boundElements: [obsoleteBinding, invisibleBinding, nonExistentBinding],
+      }),
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: container.id,
+      }),
+    ]);
+
+    restoredElements = restore.restoreElements(
+      [container, invisibleBoundElement, boundElement],
+      null,
+      { repairBindings: true },
+    );
+
     expect(restoredElements).toEqual([
       expect.objectContaining({
         id: container.id,
@@ -673,7 +720,7 @@ describe("repairing bindings", () => {
     ]);
   });
 
-  it("should remove containerId if container not exists", () => {
+  it("should remove containerId if container not exists when repair is true", () => {
     const boundElement = API.createElement({
       type: "text",
       containerId: "non-existent",
@@ -684,11 +731,28 @@ describe("repairing bindings", () => {
       isDeleted: true,
     });
 
-    const restoredElements = restore.restoreElements(
+    let restoredElements = restore.restoreElements(
       [boundElement, boundElementDeleted],
       null,
     );
 
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: "non-existent",
+      }),
+      expect.objectContaining({
+        id: boundElementDeleted.id,
+        containerId: "non-existent",
+      }),
+    ]);
+
+    restoredElements = restore.restoreElements(
+      [boundElement, boundElementDeleted],
+      null,
+      { repairBindings: true },
+    );
+
     expect(restoredElements).toEqual([
       expect.objectContaining({
         id: boundElement.id,