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

src/element/resizeElements.ts

@@ -693,7 +693,9 @@ const resizeMultipleElements = (
       };
       const fontSize = measureFontSizeFromWidth(
         boundTextElement ?? (element.orig as ExcalidrawTextElement),
-        getMaxContainerWidth(updatedElement),
+        boundTextElement
+          ? getMaxContainerWidth(updatedElement)
+          : updatedElement.width,
       );
 
       if (!fontSize) {

src/element/textElement.test.ts

@@ -1,7 +1,7 @@
 import { BOUND_TEXT_PADDING } from "../constants";
 import { API } from "../tests/helpers/api";
 import {
-  computeContainerHeightForBoundText,
+  computeContainerDimensionForBoundText,
   getContainerCoords,
   getMaxContainerWidth,
   getMaxContainerHeight,
@@ -35,10 +35,11 @@ describe("Test wrapText", () => {
 
   describe("When text doesn't contain new lines", () => {
     const text = "Hello whats up";
+
     [
       {
         desc: "break all words when width of each word is less than container width",
-        width: 90,
+        width: 80,
         res: `Hello 
 whats 
 up`,
@@ -62,7 +63,7 @@ p`,
       {
         desc: "break words as per the width",
 
-        width: 150,
+        width: 140,
         res: `Hello whats 
 up`,
       },
@@ -93,7 +94,7 @@ whats up`;
     [
       {
         desc: "break all words when width of each word is less than container width",
-        width: 90,
+        width: 80,
         res: `Hello
 whats 
 up`,
@@ -214,7 +215,7 @@ describe("Test measureText", () => {
     });
   });
 
-  describe("Test computeContainerHeightForBoundText", () => {
+  describe("Test computeContainerDimensionForBoundText", () => {
     const params = {
       width: 178,
       height: 194,
@@ -225,7 +226,9 @@ describe("Test measureText", () => {
         type: "rectangle",
         ...params,
       });
-      expect(computeContainerHeightForBoundText(element, 150)).toEqual(160);
+      expect(computeContainerDimensionForBoundText(150, element.type)).toEqual(
+        160,
+      );
     });
 
     it("should compute container height correctly for ellipse", () => {
@@ -233,7 +236,9 @@ describe("Test measureText", () => {
         type: "ellipse",
         ...params,
       });
-      expect(computeContainerHeightForBoundText(element, 150)).toEqual(226);
+      expect(computeContainerDimensionForBoundText(150, element.type)).toEqual(
+        226,
+      );
     });
 
     it("should compute container height correctly for diamond", () => {
@@ -241,7 +246,9 @@ describe("Test measureText", () => {
         type: "diamond",
         ...params,
       });
-      expect(computeContainerHeightForBoundText(element, 150)).toEqual(320);
+      expect(computeContainerDimensionForBoundText(150, element.type)).toEqual(
+        320,
+      );
     });
   });
 

src/element/textElement.ts

@@ -13,11 +13,7 @@ import { BOUND_TEXT_PADDING, TEXT_ALIGN, VERTICAL_ALIGN } from "../constants";
 import { MaybeTransformHandleType } from "./transformHandles";
 import Scene from "../scene/Scene";
 import { isTextElement } from ".";
-import {
-  isBoundToContainer,
-  isImageElement,
-  isArrowElement,
-} from "./typeChecks";
+import { isBoundToContainer, isArrowElement } from "./typeChecks";
 import { LinearElementEditor } from "./linearElementEditor";
 import { AppState } from "../types";
 import { isTextBindableContainer } from "./typeChecks";
@@ -112,9 +108,9 @@ export const redrawTextBoundingBox = (
 
       let nextHeight = containerDims.height;
       if (metrics.height > maxContainerHeight) {
-        nextHeight = computeContainerHeightForBoundText(
-          container,
+        nextHeight = computeContainerDimensionForBoundText(
           metrics.height,
+          container.type,
         );
         mutateElement(container, { height: nextHeight });
         maxContainerHeight = getMaxContainerHeight(container);
@@ -212,9 +208,9 @@ export const handleBindTextResize = (
     }
     // increase height in case text element height exceeds
     if (nextHeight > maxHeight) {
-      containerHeight = computeContainerHeightForBoundText(
-        container,
+      containerHeight = computeContainerDimensionForBoundText(
         nextHeight,
+        container.type,
       );
 
       const diff = containerHeight - containerDims.height;
@@ -348,7 +344,6 @@ export const wrapText = (text: string, font: FontString, maxWidth: number) => {
   const lines: Array<string> = [];
   const originalLines = text.split("\n");
   const spaceWidth = getLineWidth(" ", font);
-
   const push = (str: string) => {
     if (str.trim()) {
       lines.push(str);
@@ -422,7 +417,7 @@ export const wrapText = (text: string, font: FontString, maxWidth: number) => {
           const word = words[index];
           currentLineWidthTillNow = getLineWidth(currentLine + word, font);
 
-          if (currentLineWidthTillNow >= maxWidth) {
+          if (currentLineWidthTillNow > maxWidth) {
             push(currentLine);
             currentLineWidthTillNow = 0;
             currentLine = "";
@@ -738,32 +733,34 @@ export const getTextBindableContainerAtPosition = (
   return isTextBindableContainer(hitElement, false) ? hitElement : null;
 };
 
-export const isValidTextContainer = (element: ExcalidrawElement) => {
-  return (
-    element.type === "rectangle" ||
-    element.type === "ellipse" ||
-    element.type === "diamond" ||
-    isImageElement(element) ||
-    isArrowElement(element)
-  );
-};
+const VALID_CONTAINER_TYPES = new Set([
+  "rectangle",
+  "ellipse",
+  "diamond",
+  "image",
+  "arrow",
+]);
 
-export const computeContainerHeightForBoundText = (
-  container: NonDeletedExcalidrawElement,
-  boundTextElementHeight: number,
+export const isValidTextContainer = (element: ExcalidrawElement) =>
+  VALID_CONTAINER_TYPES.has(element.type);
+
+export const computeContainerDimensionForBoundText = (
+  dimension: number,
+  containerType: ExtractSetType<typeof VALID_CONTAINER_TYPES>,
 ) => {
-  if (container.type === "ellipse") {
-    return Math.round(
-      ((boundTextElementHeight + BOUND_TEXT_PADDING * 2) / Math.sqrt(2)) * 2,
-    );
+  dimension = Math.ceil(dimension);
+  const padding = BOUND_TEXT_PADDING * 2;
+
+  if (containerType === "ellipse") {
+    return Math.round(((dimension + padding) / Math.sqrt(2)) * 2);
   }
-  if (isArrowElement(container)) {
-    return boundTextElementHeight + BOUND_TEXT_PADDING * 8 * 2;
+  if (containerType === "arrow") {
+    return dimension + padding * 8;
   }
-  if (container.type === "diamond") {
-    return 2 * (boundTextElementHeight + BOUND_TEXT_PADDING * 2);
+  if (containerType === "diamond") {
+    return 2 * (dimension + padding);
   }
-  return boundTextElementHeight + BOUND_TEXT_PADDING * 2;
+  return dimension + padding;
 };
 
 export const getMaxContainerWidth = (container: ExcalidrawElement) => {

src/element/textWysiwyg.test.tsx

@@ -10,7 +10,7 @@ import {
 } from "../tests/test-utils";
 import { queryByText } from "@testing-library/react";
 
-import { FONT_FAMILY } from "../constants";
+import { FONT_FAMILY, TEXT_ALIGN, VERTICAL_ALIGN } from "../constants";
 import {
   ExcalidrawTextElement,
   ExcalidrawTextElementWithContainer,
@@ -19,6 +19,7 @@ import { API } from "../tests/helpers/api";
 import { mutateElement } from "./mutateElement";
 import { resize } from "../tests/utils";
 import { getOriginalContainerHeightFromCache } from "./textWysiwyg";
+
 // Unmount ReactDOM from root
 ReactDOM.unmountComponentAtNode(document.getElementById("root")!);
 
@@ -1307,5 +1308,91 @@ describe("textWysiwyg", () => {
         `);
       });
     });
+
+    it("should wrap text in a container when wrap text in container triggered from context menu", async () => {
+      UI.clickTool("text");
+      mouse.clickAt(20, 30);
+      const editor = document.querySelector(
+        ".excalidraw-textEditorContainer > textarea",
+      ) as HTMLTextAreaElement;
+
+      fireEvent.change(editor, {
+        target: {
+          value: "Excalidraw is an opensource virtual collaborative whiteboard",
+        },
+      });
+
+      editor.dispatchEvent(new Event("input"));
+      await new Promise((cb) => setTimeout(cb, 0));
+
+      editor.select();
+      fireEvent.click(screen.getByTitle("Left"));
+      await new Promise((r) => setTimeout(r, 0));
+
+      editor.blur();
+
+      const textElement = h.elements[1] as ExcalidrawTextElement;
+      expect(textElement.width).toBe(600);
+      expect(textElement.height).toBe(24);
+      expect(textElement.textAlign).toBe(TEXT_ALIGN.LEFT);
+      expect((textElement as ExcalidrawTextElement).text).toBe(
+        "Excalidraw is an opensource virtual collaborative whiteboard",
+      );
+
+      API.setSelectedElements([textElement]);
+
+      fireEvent.contextMenu(GlobalTestState.canvas, {
+        button: 2,
+        clientX: 20,
+        clientY: 30,
+      });
+
+      const contextMenu = document.querySelector(".context-menu");
+      fireEvent.click(
+        queryByText(contextMenu as HTMLElement, "Wrap text in a container")!,
+      );
+      expect(h.elements.length).toBe(3);
+
+      expect(h.elements[1]).toEqual(
+        expect.objectContaining({
+          angle: 0,
+          backgroundColor: "transparent",
+          boundElements: [
+            {
+              id: h.elements[2].id,
+              type: "text",
+            },
+          ],
+          fillStyle: "hachure",
+          groupIds: [],
+          height: 34,
+          isDeleted: false,
+          link: null,
+          locked: false,
+          opacity: 100,
+          roughness: 1,
+          roundness: {
+            type: 3,
+          },
+          strokeColor: "#000000",
+          strokeStyle: "solid",
+          strokeWidth: 1,
+          type: "rectangle",
+          updated: 1,
+          version: 1,
+          width: 610,
+          x: 15,
+          y: 25,
+        }),
+      );
+      expect(h.elements[2] as ExcalidrawTextElement).toEqual(
+        expect.objectContaining({
+          text: "Excalidraw is an opensource virtual collaborative whiteboard",
+          verticalAlign: VERTICAL_ALIGN.MIDDLE,
+          textAlign: TEXT_ALIGN.LEFT,
+          boundElements: null,
+        }),
+      );
+    });
   });
 });

src/element/typeChecks.test.ts

@@ -0,0 +1,66 @@
+import { API } from "../tests/helpers/api";
+import { hasBoundTextElement } from "./typeChecks";
+
+describe("Test TypeChecks", () => {
+  describe("Test hasBoundTextElement", () => {
+    it("should return true for text bindable containers with bound text", () => {
+      expect(
+        hasBoundTextElement(
+          API.createElement({
+            type: "rectangle",
+            boundElements: [{ type: "text", id: "text-id" }],
+          }),
+        ),
+      ).toBeTruthy();
+
+      expect(
+        hasBoundTextElement(
+          API.createElement({
+            type: "ellipse",
+            boundElements: [{ type: "text", id: "text-id" }],
+          }),
+        ),
+      ).toBeTruthy();
+
+      expect(
+        hasBoundTextElement(
+          API.createElement({
+            type: "arrow",
+            boundElements: [{ type: "text", id: "text-id" }],
+          }),
+        ),
+      ).toBeTruthy();
+
+      expect(
+        hasBoundTextElement(
+          API.createElement({
+            type: "image",
+            boundElements: [{ type: "text", id: "text-id" }],
+          }),
+        ),
+      ).toBeTruthy();
+    });
+
+    it("should return false for text bindable containers without bound text", () => {
+      expect(
+        hasBoundTextElement(
+          API.createElement({
+            type: "freedraw",
+            boundElements: [{ type: "arrow", id: "arrow-id" }],
+          }),
+        ),
+      ).toBeFalsy();
+    });
+
+    it("should return false for non text bindable containers", () => {
+      expect(
+        hasBoundTextElement(
+          API.createElement({
+            type: "freedraw",
+            boundElements: [{ type: "text", id: "text-id" }],
+          }),
+        ),
+      ).toBeFalsy();
+    });
+  });
+});

src/element/typeChecks.ts

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