fix: improve text wrapping inside rhombus and more fixes (#6265)

* fix: improve text wrapping inside rhombus

* Add comments

* specs

* fix: shift resize and multiple element regression for ellipse and rhombus

* use container width for scaling font size

* fix

* fix multiple resize

* lint

* redraw on submit

* redraw only newly pasted elements

* no padding when center

* fix tests

* fix

* dont add padding in rhombus when aligning

* refactor

* fix

* move getMaxContainerHeight and getMaxContainerWidth to textElement.ts

* Add specs

src/components/App.tsx

@@ -1627,6 +1627,7 @@ class App extends React.Component<AppProps, AppState> {
       oldIdToDuplicatedId.set(element.id, newElement.id);
       return newElement;
     });
+
     bindTextToShapeAfterDuplication(newElements, elements, oldIdToDuplicatedId);
     const nextElements = [
       ...this.scene.getElementsIncludingDeleted(),
@@ -1640,10 +1641,10 @@ class App extends React.Component<AppProps, AppState> {
 
     this.scene.replaceAllElements(nextElements);
 
-    nextElements.forEach((nextElement) => {
-      if (isTextElement(nextElement) && isBoundToContainer(nextElement)) {
-        const container = getContainerElement(nextElement);
-        redrawTextBoundingBox(nextElement, container);
+    newElements.forEach((newElement) => {
+      if (isTextElement(newElement) && isBoundToContainer(newElement)) {
+        const container = getContainerElement(newElement);
+        redrawTextBoundingBox(newElement, container);
       }
     });
 

src/element/newElement.ts

@@ -22,15 +22,15 @@ import { getElementAbsoluteCoords } from ".";
 import { adjustXYWithRotation } from "../math";
 import { getResizedElementAbsoluteCoords } from "./bounds";
 import {
-  getBoundTextElement,
   getBoundTextElementOffset,
   getContainerDims,
   getContainerElement,
   measureText,
   normalizeText,
   wrapText,
+  getMaxContainerWidth,
 } from "./textElement";
-import { BOUND_TEXT_PADDING, VERTICAL_ALIGN } from "../constants";
+import { VERTICAL_ALIGN } from "../constants";
 import { isArrowElement } from "./typeChecks";
 
 type ElementConstructorOpts = MarkOptional<
@@ -278,48 +278,6 @@ export const refreshTextDimensions = (
   return { text, ...dimensions };
 };
 
-export const getMaxContainerWidth = (container: ExcalidrawElement) => {
-  const width = getContainerDims(container).width;
-  if (isArrowElement(container)) {
-    const containerWidth = width - BOUND_TEXT_PADDING * 8 * 2;
-    if (containerWidth <= 0) {
-      const boundText = getBoundTextElement(container);
-      if (boundText) {
-        return boundText.width;
-      }
-      return BOUND_TEXT_PADDING * 8 * 2;
-    }
-    return containerWidth;
-  } else if (container.type === "ellipse") {
-    // The width of the largest rectangle inscribed inside an ellipse is
-    // Math.round((ellipse.width / 2) * Math.sqrt(2)) which is derived from
-    // equation of an ellipse -https://github.com/excalidraw/excalidraw/pull/6172
-    return Math.round((width / 2) * Math.sqrt(2)) - BOUND_TEXT_PADDING * 2;
-  }
-  return width - BOUND_TEXT_PADDING * 2;
-};
-
-export const getMaxContainerHeight = (container: ExcalidrawElement) => {
-  const height = getContainerDims(container).height;
-  if (isArrowElement(container)) {
-    const containerHeight = height - BOUND_TEXT_PADDING * 8 * 2;
-    if (containerHeight <= 0) {
-      const boundText = getBoundTextElement(container);
-      if (boundText) {
-        return boundText.height;
-      }
-      return BOUND_TEXT_PADDING * 8 * 2;
-    }
-    return height;
-  } else if (container.type === "ellipse") {
-    // The height of the largest rectangle inscribed inside an ellipse is
-    // Math.round((ellipse.height / 2) * Math.sqrt(2)) which is derived from
-    // equation of an ellipse - https://github.com/excalidraw/excalidraw/pull/6172
-    return Math.round((height / 2) * Math.sqrt(2)) - BOUND_TEXT_PADDING * 2;
-  }
-  return height - BOUND_TEXT_PADDING * 2;
-};
-
 export const updateTextElement = (
   textElement: ExcalidrawTextElement,
   {

src/element/resizeElements.ts

@@ -43,12 +43,12 @@ import {
   getApproxMinLineWidth,
   getBoundTextElement,
   getBoundTextElementId,
-  getBoundTextElementOffset,
   getContainerElement,
   handleBindTextResize,
   measureText,
+  getMaxContainerHeight,
+  getMaxContainerWidth,
 } from "./textElement";
-import { getMaxContainerWidth } from "./newElement";
 
 export const normalizeAngle = (angle: number): number => {
   if (angle >= 2 * Math.PI) {
@@ -427,12 +427,16 @@ export const resizeSingleElement = (
       };
     }
     if (shouldMaintainAspectRatio) {
-      const boundTextElementPadding =
-        getBoundTextElementOffset(boundTextElement);
+      const updatedElement = {
+        ...element,
+        width: eleNewWidth,
+        height: eleNewHeight,
+      };
+
       const nextFont = measureFontSizeFromWH(
         boundTextElement,
-        eleNewWidth - boundTextElementPadding * 2,
-        eleNewHeight - boundTextElementPadding * 2,
+        getMaxContainerWidth(updatedElement),
+        getMaxContainerHeight(updatedElement),
       );
       if (nextFont === null) {
         return;
@@ -697,11 +701,15 @@ const resizeMultipleElements = (
     const boundTextElement = getBoundTextElement(element.latest);
 
     if (boundTextElement || isTextElement(element.orig)) {
-      const optionalPadding = getBoundTextElementOffset(boundTextElement) * 2;
+      const updatedElement = {
+        ...element.latest,
+        width,
+        height,
+      };
       const textMeasurements = measureFontSizeFromWH(
         boundTextElement ?? (element.orig as ExcalidrawTextElement),
-        width - optionalPadding,
-        height - optionalPadding,
+        getMaxContainerWidth(updatedElement),
+        getMaxContainerHeight(updatedElement),
       );
 
       if (!textMeasurements) {

src/element/textElement.test.ts

@@ -3,6 +3,8 @@ import { API } from "../tests/helpers/api";
 import {
   computeContainerHeightForBoundText,
   getContainerCoords,
+  getMaxContainerWidth,
+  getMaxContainerHeight,
   measureText,
   wrapText,
 } from "./textElement";
@@ -202,24 +204,37 @@ describe("Test measureText", () => {
 
   describe("Test getContainerCoords", () => {
     const params = { width: 200, height: 100, x: 10, y: 20 };
+
     it("should compute coords correctly when ellipse", () => {
-      const ellipse = API.createElement({
+      const element = API.createElement({
         type: "ellipse",
         ...params,
       });
-      expect(getContainerCoords(ellipse)).toEqual({
+      expect(getContainerCoords(element)).toEqual({
         x: 44.2893218813452455,
         y: 39.64466094067262,
       });
     });
+
     it("should compute coords correctly when rectangle", () => {
-      const rectangle = API.createElement({
+      const element = API.createElement({
         type: "rectangle",
         ...params,
       });
-      expect(getContainerCoords(rectangle)).toEqual({
-        x: 10,
-        y: 20,
+      expect(getContainerCoords(element)).toEqual({
+        x: 15,
+        y: 25,
+      });
+    });
+
+    it("should compute coords correctly when diamond", () => {
+      const element = API.createElement({
+        type: "diamond",
+        ...params,
+      });
+      expect(getContainerCoords(element)).toEqual({
+        x: 65,
+        y: 50,
       });
     });
   });
@@ -229,6 +244,7 @@ describe("Test measureText", () => {
       width: 178,
       height: 194,
     };
+
     it("should compute container height correctly for rectangle", () => {
       const element = API.createElement({
         type: "rectangle",
@@ -236,6 +252,7 @@ describe("Test measureText", () => {
       });
       expect(computeContainerHeightForBoundText(element, 150)).toEqual(160);
     });
+
     it("should compute container height correctly for ellipse", () => {
       const element = API.createElement({
         type: "ellipse",
@@ -243,5 +260,57 @@ describe("Test measureText", () => {
       });
       expect(computeContainerHeightForBoundText(element, 150)).toEqual(212);
     });
+
+    it("should compute container height correctly for diamond", () => {
+      const element = API.createElement({
+        type: "diamond",
+        ...params,
+      });
+      expect(computeContainerHeightForBoundText(element, 150)).toEqual(300);
+    });
+  });
+
+  describe("Test getMaxContainerWidth", () => {
+    const params = {
+      width: 178,
+      height: 194,
+    };
+
+    it("should return max width when container is rectangle", () => {
+      const container = API.createElement({ type: "rectangle", ...params });
+      expect(getMaxContainerWidth(container)).toBe(168);
+    });
+
+    it("should return max width when container is ellipse", () => {
+      const container = API.createElement({ type: "ellipse", ...params });
+      expect(getMaxContainerWidth(container)).toBe(116);
+    });
+
+    it("should return max width when container is diamond", () => {
+      const container = API.createElement({ type: "diamond", ...params });
+      expect(getMaxContainerWidth(container)).toBe(79);
+    });
+  });
+
+  describe("Test getMaxContainerHeight", () => {
+    const params = {
+      width: 178,
+      height: 194,
+    };
+
+    it("should return max height when container is rectangle", () => {
+      const container = API.createElement({ type: "rectangle", ...params });
+      expect(getMaxContainerHeight(container)).toBe(184);
+    });
+
+    it("should return max height when container is ellipse", () => {
+      const container = API.createElement({ type: "ellipse", ...params });
+      expect(getMaxContainerHeight(container)).toBe(127);
+    });
+
+    it("should return max height when container is diamond", () => {
+      const container = API.createElement({ type: "diamond", ...params });
+      expect(getMaxContainerHeight(container)).toBe(87);
+    });
   });
 });

src/element/textElement.ts

@@ -12,7 +12,6 @@ import { BOUND_TEXT_PADDING, TEXT_ALIGN, VERTICAL_ALIGN } from "../constants";
 import { MaybeTransformHandleType } from "./transformHandles";
 import Scene from "../scene/Scene";
 import { isTextElement } from ".";
-import { getMaxContainerHeight, getMaxContainerWidth } from "./newElement";
 import {
   isBoundToContainer,
   isImageElement,
@@ -244,31 +243,25 @@ const computeBoundTextPosition = (
   const containerCoords = getContainerCoords(container);
   const maxContainerHeight = getMaxContainerHeight(container);
   const maxContainerWidth = getMaxContainerWidth(container);
-  const padding = container.type === "ellipse" ? 0 : BOUND_TEXT_PADDING;
 
   let x;
   let y;
   if (boundTextElement.verticalAlign === VERTICAL_ALIGN.TOP) {
-    y = containerCoords.y + padding;
+    y = containerCoords.y;
   } else if (boundTextElement.verticalAlign === VERTICAL_ALIGN.BOTTOM) {
-    y =
-      containerCoords.y +
-      (maxContainerHeight - boundTextElement.height + padding);
+    y = containerCoords.y + (maxContainerHeight - boundTextElement.height);
   } else {
     y =
       containerCoords.y +
-      (maxContainerHeight / 2 - boundTextElement.height / 2 + padding);
+      (maxContainerHeight / 2 - boundTextElement.height / 2);
   }
   if (boundTextElement.textAlign === TEXT_ALIGN.LEFT) {
-    x = containerCoords.x + padding;
+    x = containerCoords.x;
   } else if (boundTextElement.textAlign === TEXT_ALIGN.RIGHT) {
-    x =
-      containerCoords.x +
-      (maxContainerWidth - boundTextElement.width + padding);
+    x = containerCoords.x + (maxContainerWidth - boundTextElement.width);
   } else {
     x =
-      containerCoords.x +
-      (maxContainerWidth / 2 - boundTextElement.width / 2 + padding);
+      containerCoords.x + (maxContainerWidth / 2 - boundTextElement.width / 2);
   }
   return { x, y };
 };
@@ -636,20 +629,22 @@ export const getContainerCenter = (
 };
 
 export const getContainerCoords = (container: NonDeletedExcalidrawElement) => {
+  let offsetX = BOUND_TEXT_PADDING;
+  let offsetY = BOUND_TEXT_PADDING;
+
   if (container.type === "ellipse") {
     // The derivation of coordinates is explained in https://github.com/excalidraw/excalidraw/pull/6172
-    const offsetX =
-      (container.width / 2) * (1 - Math.sqrt(2) / 2) + BOUND_TEXT_PADDING;
-    const offsetY =
-      (container.height / 2) * (1 - Math.sqrt(2) / 2) + BOUND_TEXT_PADDING;
-    return {
-      x: container.x + offsetX,
-      y: container.y + offsetY,
-    };
+    offsetX += (container.width / 2) * (1 - Math.sqrt(2) / 2);
+    offsetY += (container.height / 2) * (1 - Math.sqrt(2) / 2);
+  }
+  // The derivation of coordinates is explained in https://github.com/excalidraw/excalidraw/pull/6265
+  if (container.type === "diamond") {
+    offsetX += container.width / 4;
+    offsetY += container.height / 4;
   }
   return {
-    x: container.x,
-    y: container.y,
+    x: container.x + offsetX,
+    y: container.y + offsetY,
   };
 };
 
@@ -767,5 +762,63 @@ export const computeContainerHeightForBoundText = (
   if (isArrowElement(container)) {
     return boundTextElementHeight + BOUND_TEXT_PADDING * 8 * 2;
   }
+  if (container.type === "diamond") {
+    return 2 * boundTextElementHeight;
+  }
   return boundTextElementHeight + BOUND_TEXT_PADDING * 2;
 };
+
+export const getMaxContainerWidth = (container: ExcalidrawElement) => {
+  const width = getContainerDims(container).width;
+  if (isArrowElement(container)) {
+    const containerWidth = width - BOUND_TEXT_PADDING * 8 * 2;
+    if (containerWidth <= 0) {
+      const boundText = getBoundTextElement(container);
+      if (boundText) {
+        return boundText.width;
+      }
+      return BOUND_TEXT_PADDING * 8 * 2;
+    }
+    return containerWidth;
+  }
+
+  if (container.type === "ellipse") {
+    // The width of the largest rectangle inscribed inside an ellipse is
+    // Math.round((ellipse.width / 2) * Math.sqrt(2)) which is derived from
+    // equation of an ellipse -https://github.com/excalidraw/excalidraw/pull/6172
+    return Math.round((width / 2) * Math.sqrt(2)) - BOUND_TEXT_PADDING * 2;
+  }
+  if (container.type === "diamond") {
+    // The width of the largest rectangle inscribed inside a rhombus is
+    // Math.round(width / 2) - https://github.com/excalidraw/excalidraw/pull/6265
+    return Math.round(width / 2) - BOUND_TEXT_PADDING * 2;
+  }
+  return width - BOUND_TEXT_PADDING * 2;
+};
+
+export const getMaxContainerHeight = (container: ExcalidrawElement) => {
+  const height = getContainerDims(container).height;
+  if (isArrowElement(container)) {
+    const containerHeight = height - BOUND_TEXT_PADDING * 8 * 2;
+    if (containerHeight <= 0) {
+      const boundText = getBoundTextElement(container);
+      if (boundText) {
+        return boundText.height;
+      }
+      return BOUND_TEXT_PADDING * 8 * 2;
+    }
+    return height;
+  }
+  if (container.type === "ellipse") {
+    // The height of the largest rectangle inscribed inside an ellipse is
+    // Math.round((ellipse.height / 2) * Math.sqrt(2)) which is derived from
+    // equation of an ellipse - https://github.com/excalidraw/excalidraw/pull/6172
+    return Math.round((height / 2) * Math.sqrt(2)) - BOUND_TEXT_PADDING * 2;
+  }
+  if (container.type === "diamond") {
+    // The height of the largest rectangle inscribed inside a rhombus is
+    // Math.round(height / 2) - https://github.com/excalidraw/excalidraw/pull/6265
+    return Math.round(height / 2) - BOUND_TEXT_PADDING * 2;
+  }
+  return height - BOUND_TEXT_PADDING * 2;
+};

src/element/textWysiwyg.test.tsx

@@ -791,7 +791,7 @@ describe("textWysiwyg", () => {
       text = h.elements[1] as ExcalidrawTextElementWithContainer;
       expect(text.text).toBe("Hello \nWorld!");
       expect(text.originalText).toBe("Hello World!");
-      expect(text.y).toBe(27.5);
+      expect(text.y).toBe(57.5);
       expect(text.x).toBe(rectangle.x + BOUND_TEXT_PADDING);
       expect(text.height).toBe(APPROX_LINE_HEIGHT * 2);
       expect(text.width).toBe(rectangle.width - BOUND_TEXT_PADDING * 2);
@@ -825,7 +825,7 @@ describe("textWysiwyg", () => {
 
       expect(text.text).toBe("Hello");
       expect(text.originalText).toBe("Hello");
-      expect(text.y).toBe(40);
+      expect(text.y).toBe(57.5);
       expect(text.x).toBe(rectangle.x + BOUND_TEXT_PADDING);
       expect(text.height).toBe(APPROX_LINE_HEIGHT);
       expect(text.width).toBe(rectangle.width - BOUND_TEXT_PADDING * 2);
@@ -930,6 +930,8 @@ describe("textWysiwyg", () => {
       editor.select();
 
       fireEvent.click(screen.getByTitle("Left"));
+      await new Promise((r) => setTimeout(r, 0));
+
       fireEvent.click(screen.getByTitle("Align bottom"));
       await new Promise((r) => setTimeout(r, 0));
 
@@ -1278,7 +1280,7 @@ describe("textWysiwyg", () => {
         expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
           Array [
             15,
-            20,
+            25,
           ]
         `);
       });
@@ -1290,7 +1292,7 @@ describe("textWysiwyg", () => {
         expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
           Array [
             -25,
-            20,
+            25,
           ]
         `);
       });
@@ -1302,7 +1304,7 @@ describe("textWysiwyg", () => {
         expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
           Array [
             174,
-            20,
+            25,
           ]
         `);
       });

src/element/textWysiwyg.tsx

@@ -24,14 +24,16 @@ import { mutateElement } from "./mutateElement";
 import {
   getApproxLineHeight,
   getBoundTextElementId,
-  getBoundTextElementOffset,
   getContainerCoords,
   getContainerDims,
   getContainerElement,
   getTextElementAngle,
   getTextWidth,
   normalizeText,
+  redrawTextBoundingBox,
   wrapText,
+  getMaxContainerHeight,
+  getMaxContainerWidth,
 } from "./textElement";
 import {
   actionDecreaseFontSize,
@@ -39,7 +41,6 @@ import {
 } from "../actions/actionProperties";
 import { actionZoomIn, actionZoomOut } from "../actions/actionCanvas";
 import App from "../components/App";
-import { getMaxContainerHeight, getMaxContainerWidth } from "./newElement";
 import { LinearElementEditor } from "./linearElementEditor";
 import { parseClipboard } from "../clipboard";
 
@@ -231,10 +232,6 @@ export const textWysiwyg = ({
         // Start pushing text upward until a diff of 30px (padding)
         // is reached
         else {
-          const padding =
-            container.type === "ellipse"
-              ? 0
-              : getBoundTextElementOffset(updatedTextElement);
           const containerCoords = getContainerCoords(container);
 
           // vertically center align the text
@@ -245,8 +242,7 @@ export const textWysiwyg = ({
             }
           }
           if (verticalAlign === VERTICAL_ALIGN.BOTTOM) {
-            coordY =
-              containerCoords.y + (maxHeight - textElementHeight + padding);
+            coordY = containerCoords.y + (maxHeight - textElementHeight);
           }
         }
       }
@@ -616,6 +612,7 @@ export const textWysiwyg = ({
           ),
         });
       }
+      redrawTextBoundingBox(updateElement, container);
     }
 
     onSubmit({

src/tests/linearElementEditor.test.tsx

@@ -17,8 +17,11 @@ import { KEYS } from "../keys";
 import { LinearElementEditor } from "../element/linearElementEditor";
 import { queryByTestId, queryByText } from "@testing-library/react";
 import { resize, rotate } from "./utils";
-import { getBoundTextElementPosition, wrapText } from "../element/textElement";
-import { getMaxContainerWidth } from "../element/newElement";
+import {
+  getBoundTextElementPosition,
+  wrapText,
+  getMaxContainerWidth,
+} from "../element/textElement";
 import * as textElementUtils from "../element/textElement";
 import { ROUNDNESS } from "../constants";