fix: improve text wrapping in ellipse and alignment (#6172)

* fix: improve text wrapping in ellipse

* compute height when font properties updated

* fix alignment

* fix alignment when resizing

* fix

* ad padding

* always compute height when redrawing bounding box and refactor

* lint

* fix specs

* fix

* redraw text bounding box when pasted or refreshed

* fix

* Add specs

* fix

* restore on font load

* add comments

src/components/App.tsx

@@ -108,6 +108,7 @@ import {
   textWysiwyg,
   transformElements,
   updateTextElement,
+  redrawTextBoundingBox,
 } from "../element";
 import {
   bindOrUnbindLinearElement,
@@ -264,6 +265,7 @@ import {
   getBoundTextElement,
   getContainerCenter,
   getContainerDims,
+  getContainerElement,
   getTextBindableContainerAtPosition,
   isValidTextContainer,
 } from "../element/textElement";
@@ -1637,6 +1639,14 @@ 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);
+      }
+    });
+
     this.history.resumeRecording();
 
     this.setState(

src/element/newElement.ts

@@ -290,6 +290,11 @@ export const getMaxContainerWidth = (container: ExcalidrawElement) => {
       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;
 };
@@ -306,6 +311,11 @@ export const getMaxContainerHeight = (container: ExcalidrawElement) => {
       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;
 };

src/element/textElement.test.ts

@@ -1,5 +1,11 @@
 import { BOUND_TEXT_PADDING } from "../constants";
-import { measureText, wrapText } from "./textElement";
+import { API } from "../tests/helpers/api";
+import {
+  computeContainerHeightForBoundText,
+  getContainerCoords,
+  measureText,
+  wrapText,
+} from "./textElement";
 import { FontString } from "./types";
 
 describe("Test wrapText", () => {
@@ -193,4 +199,49 @@ describe("Test measureText", () => {
       </div>
     `);
   });
+
+  describe("Test getContainerCoords", () => {
+    const params = { width: 200, height: 100, x: 10, y: 20 };
+    it("should compute coords correctly when ellipse", () => {
+      const ellipse = API.createElement({
+        type: "ellipse",
+        ...params,
+      });
+      expect(getContainerCoords(ellipse)).toEqual({
+        x: 44.2893218813452455,
+        y: 39.64466094067262,
+      });
+    });
+    it("should compute coords correctly when rectangle", () => {
+      const rectangle = API.createElement({
+        type: "rectangle",
+        ...params,
+      });
+      expect(getContainerCoords(rectangle)).toEqual({
+        x: 10,
+        y: 20,
+      });
+    });
+  });
+
+  describe("Test computeContainerHeightForBoundText", () => {
+    const params = {
+      width: 178,
+      height: 194,
+    };
+    it("should compute container height correctly for rectangle", () => {
+      const element = API.createElement({
+        type: "rectangle",
+        ...params,
+      });
+      expect(computeContainerHeightForBoundText(element, 150)).toEqual(160);
+    });
+    it("should compute container height correctly for ellipse", () => {
+      const element = API.createElement({
+        type: "ellipse",
+        ...params,
+      });
+      expect(computeContainerHeightForBoundText(element, 150)).toEqual(212);
+    });
+  });
 });

src/element/textElement.ts

@@ -44,68 +44,69 @@ export const redrawTextBoundingBox = (
   container: ExcalidrawElement | null,
 ) => {
   let maxWidth = undefined;
-  let text = textElement.text;
+
+  const boundTextUpdates = {
+    x: textElement.x,
+    y: textElement.y,
+    text: textElement.text,
+    width: textElement.width,
+    height: textElement.height,
+    baseline: textElement.baseline,
+  };
+
+  boundTextUpdates.text = textElement.text;
+
   if (container) {
     maxWidth = getMaxContainerWidth(container);
-    text = wrapText(
+    boundTextUpdates.text = wrapText(
       textElement.originalText,
       getFontString(textElement),
       maxWidth,
     );
   }
-  const metrics = measureText(text, getFontString(textElement), maxWidth);
-  let coordY = textElement.y;
-  let coordX = textElement.x;
-  // Resize container and vertically center align the text
+  const metrics = measureText(
+    boundTextUpdates.text,
+    getFontString(textElement),
+    maxWidth,
+  );
+
+  boundTextUpdates.width = metrics.width;
+  boundTextUpdates.height = metrics.height;
+  boundTextUpdates.baseline = metrics.baseline;
+
   if (container) {
-    if (!isArrowElement(container)) {
-      const containerDims = getContainerDims(container);
-      let nextHeight = containerDims.height;
-      if (textElement.verticalAlign === VERTICAL_ALIGN.TOP) {
-        coordY = container.y;
-      } else if (textElement.verticalAlign === VERTICAL_ALIGN.BOTTOM) {
-        coordY =
-          container.y +
-          containerDims.height -
-          metrics.height -
-          BOUND_TEXT_PADDING;
-      } else {
-        coordY = container.y + containerDims.height / 2 - metrics.height / 2;
-        if (metrics.height > getMaxContainerHeight(container)) {
-          nextHeight = metrics.height + BOUND_TEXT_PADDING * 2;
-          coordY = container.y + nextHeight / 2 - metrics.height / 2;
-        }
-      }
-      if (textElement.textAlign === TEXT_ALIGN.LEFT) {
-        coordX = container.x + BOUND_TEXT_PADDING;
-      } else if (textElement.textAlign === TEXT_ALIGN.RIGHT) {
-        coordX =
-          container.x +
-          containerDims.width -
-          metrics.width -
-          BOUND_TEXT_PADDING;
-      } else {
-        coordX = container.x + containerDims.width / 2 - metrics.width / 2;
-      }
-      updateOriginalContainerCache(container.id, nextHeight);
-      mutateElement(container, { height: nextHeight });
-    } else {
+    if (isArrowElement(container)) {
       const centerX = textElement.x + textElement.width / 2;
       const centerY = textElement.y + textElement.height / 2;
       const diffWidth = metrics.width - textElement.width;
       const diffHeight = metrics.height - textElement.height;
-      coordY = centerY - (textElement.height + diffHeight) / 2;
-      coordX = centerX - (textElement.width + diffWidth) / 2;
+      boundTextUpdates.x = centerY - (textElement.height + diffHeight) / 2;
+      boundTextUpdates.y = centerX - (textElement.width + diffWidth) / 2;
+    } else {
+      const containerDims = getContainerDims(container);
+      let maxContainerHeight = getMaxContainerHeight(container);
+
+      let nextHeight = containerDims.height;
+      if (metrics.height > maxContainerHeight) {
+        nextHeight = computeContainerHeightForBoundText(
+          container,
+          metrics.height,
+        );
+        mutateElement(container, { height: nextHeight });
+        maxContainerHeight = getMaxContainerHeight(container);
+        updateOriginalContainerCache(container.id, nextHeight);
+      }
+      const updatedTextElement = {
+        ...textElement,
+        ...boundTextUpdates,
+      } as ExcalidrawTextElementWithContainer;
+      const { x, y } = computeBoundTextPosition(container, updatedTextElement);
+      boundTextUpdates.x = x;
+      boundTextUpdates.y = y;
     }
   }
-  mutateElement(textElement, {
-    width: metrics.width,
-    height: metrics.height,
-    baseline: metrics.baseline,
-    y: coordY,
-    x: coordX,
-    text,
-  });
+
+  mutateElement(textElement, boundTextUpdates);
 };
 
 export const bindTextToShapeAfterDuplication = (
@@ -197,7 +198,11 @@ export const handleBindTextResize = (
     }
     // increase height in case text element height exceeds
     if (nextHeight > maxHeight) {
-      containerHeight = nextHeight + getBoundTextElementOffset(textElement) * 2;
+      containerHeight = computeContainerHeightForBoundText(
+        container,
+        nextHeight,
+      );
+
       const diff = containerHeight - containerDims.height;
       // fix the y coord when resizing from ne/nw/n
       const updatedY =
@@ -217,48 +222,57 @@ export const handleBindTextResize = (
       text,
       width: nextWidth,
       height: nextHeight,
-
       baseline: nextBaseLine,
     });
+
     if (!isArrowElement(container)) {
-      updateBoundTextPosition(
-        container,
-        textElement as ExcalidrawTextElementWithContainer,
+      mutateElement(
+        textElement,
+        computeBoundTextPosition(
+          container,
+          textElement as ExcalidrawTextElementWithContainer,
+        ),
       );
     }
   }
 };
 
-const updateBoundTextPosition = (
+const computeBoundTextPosition = (
   container: ExcalidrawElement,
   boundTextElement: ExcalidrawTextElementWithContainer,
 ) => {
-  const containerDims = getContainerDims(container);
-  const boundTextElementPadding = getBoundTextElementOffset(boundTextElement);
+  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 = container.y + boundTextElementPadding;
+    y = containerCoords.y + padding;
   } else if (boundTextElement.verticalAlign === VERTICAL_ALIGN.BOTTOM) {
     y =
-      container.y +
-      containerDims.height -
-      boundTextElement.height -
-      boundTextElementPadding;
+      containerCoords.y +
+      (maxContainerHeight - boundTextElement.height + padding);
   } else {
-    y = container.y + containerDims.height / 2 - boundTextElement.height / 2;
-  }
-  const x =
-    boundTextElement.textAlign === TEXT_ALIGN.LEFT
-      ? container.x + boundTextElementPadding
-      : boundTextElement.textAlign === TEXT_ALIGN.RIGHT
-      ? container.x +
-        containerDims.width -
-        boundTextElement.width -
-        boundTextElementPadding
-      : container.x + containerDims.width / 2 - boundTextElement.width / 2;
-
-  mutateElement(boundTextElement, { x, y });
+    y =
+      containerCoords.y +
+      (maxContainerHeight / 2 - boundTextElement.height / 2 + padding);
+  }
+  if (boundTextElement.textAlign === TEXT_ALIGN.LEFT) {
+    x = containerCoords.x + padding;
+  } else if (boundTextElement.textAlign === TEXT_ALIGN.RIGHT) {
+    x =
+      containerCoords.x +
+      (maxContainerWidth - boundTextElement.width + padding);
+  } else {
+    x =
+      containerCoords.x +
+      (maxContainerWidth / 2 - boundTextElement.width / 2 + padding);
+  }
+  return { x, y };
 };
+
 // https://github.com/grassator/canvas-text-editor/blob/master/lib/FontMetrics.js
 export const measureText = (
   text: string,
@@ -621,6 +635,24 @@ export const getContainerCenter = (
   return { x: midSegmentMidpoint[0], y: midSegmentMidpoint[1] };
 };
 
+export const getContainerCoords = (container: NonDeletedExcalidrawElement) => {
+  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,
+    };
+  }
+  return {
+    x: container.x,
+    y: container.y,
+  };
+};
+
 export const getTextElementAngle = (textElement: ExcalidrawTextElement) => {
   const container = getContainerElement(textElement);
   if (!container || isArrowElement(container)) {
@@ -633,12 +665,13 @@ export const getBoundTextElementOffset = (
   boundTextElement: ExcalidrawTextElement | null,
 ) => {
   const container = getContainerElement(boundTextElement);
-  if (!container) {
+  if (!container || !boundTextElement) {
     return 0;
   }
   if (isArrowElement(container)) {
     return BOUND_TEXT_PADDING * 8;
   }
+
   return BOUND_TEXT_PADDING;
 };
 
@@ -723,3 +756,16 @@ export const isValidTextContainer = (element: ExcalidrawElement) => {
     isArrowElement(element)
   );
 };
+
+export const computeContainerHeightForBoundText = (
+  container: NonDeletedExcalidrawElement,
+  boundTextElementHeight: number,
+) => {
+  if (container.type === "ellipse") {
+    return Math.round((boundTextElementHeight / Math.sqrt(2)) * 2);
+  }
+  if (isArrowElement(container)) {
+    return boundTextElementHeight + BOUND_TEXT_PADDING * 8 * 2;
+  }
+  return boundTextElementHeight + BOUND_TEXT_PADDING * 2;
+};

src/element/textWysiwyg.test.tsx

@@ -791,9 +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(
-        rectangle.y + rectangle.height / 2 - (APPROX_LINE_HEIGHT * 2) / 2,
-      );
+      expect(text.y).toBe(27.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);
@@ -827,9 +825,7 @@ describe("textWysiwyg", () => {
 
       expect(text.text).toBe("Hello");
       expect(text.originalText).toBe("Hello");
-      expect(text.y).toBe(
-        rectangle.y + rectangle.height / 2 - APPROX_LINE_HEIGHT / 2,
-      );
+      expect(text.y).toBe(40);
       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);
@@ -1248,7 +1244,7 @@ describe("textWysiwyg", () => {
         expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
           Array [
             15,
-            20,
+            25,
           ]
         `);
       });
@@ -1259,7 +1255,7 @@ describe("textWysiwyg", () => {
         expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
           Array [
             94.5,
-            20,
+            25,
           ]
         `);
       });
@@ -1269,22 +1265,22 @@ describe("textWysiwyg", () => {
         fireEvent.click(screen.getByTitle("Align top"));
 
         expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
-            Array [
-              174,
-              20,
-            ]
-          `);
+          Array [
+            174,
+            25,
+          ]
+        `);
       });
 
       it("when center left", async () => {
         fireEvent.click(screen.getByTitle("Center vertically"));
         fireEvent.click(screen.getByTitle("Left"));
         expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
-            Array [
-              15,
-              25,
-            ]
-          `);
+          Array [
+            15,
+            20,
+          ]
+        `);
       });
 
       it("when center center", async () => {
@@ -1292,11 +1288,11 @@ describe("textWysiwyg", () => {
         fireEvent.click(screen.getByTitle("Center vertically"));
 
         expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
-            Array [
-              -25,
-              25,
-            ]
-          `);
+          Array [
+            -25,
+            20,
+          ]
+        `);
       });
 
       it("when center right", async () => {
@@ -1304,11 +1300,11 @@ describe("textWysiwyg", () => {
         fireEvent.click(screen.getByTitle("Center vertically"));
 
         expect([h.elements[1].x, h.elements[1].y]).toMatchInlineSnapshot(`
-            Array [
-              174,
-              25,
-            ]
-          `);
+          Array [
+            174,
+            20,
+          ]
+        `);
       });
 
       it("when bottom left", async () => {

src/element/textWysiwyg.tsx

@@ -25,6 +25,7 @@ import {
   getApproxLineHeight,
   getBoundTextElementId,
   getBoundTextElementOffset,
+  getContainerCoords,
   getContainerDims,
   getContainerElement,
   getTextElementAngle,
@@ -230,19 +231,22 @@ 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
           if (verticalAlign === VERTICAL_ALIGN.MIDDLE) {
             if (!isArrowElement(container)) {
               coordY =
-                container.y + containerDims.height / 2 - textElementHeight / 2;
+                containerCoords.y + maxHeight / 2 - textElementHeight / 2;
             }
           }
           if (verticalAlign === VERTICAL_ALIGN.BOTTOM) {
             coordY =
-              container.y +
-              containerDims.height -
-              textElementHeight -
-              getBoundTextElementOffset(updatedTextElement);
+              containerCoords.y + (maxHeight - textElementHeight + padding);
           }
         }
       }