fix: set the dimensions of bound text correctly (#5710)

* fix: set the dimensions of bound text correctly * use original Text when wrapping * fix text align * fix specs * fix * newline
2 years ago · 4cb6f09559
parent 8636ef1017
commit 4cb6f09559
8 changed files with 79 additions and 57 deletions
--- a/.gitignore
+++ b/.gitignore
@ -25,4 +25,3 @@ src/packages/excalidraw/types
 src/packages/excalidraw/example/public/bundle.js
 src/packages/excalidraw/example/public/excalidraw-assets-dev
 src/packages/excalidraw/example/public/excalidraw.development.js
-
--- a/src/constants.ts
+++ b/src/constants.ts
@ -201,6 +201,12 @@ export const VERTICAL_ALIGN = {
  BOTTOM: "bottom",
 };

+export const TEXT_ALIGN = {
+  LEFT: "left",
+  CENTER: "center",
+  RIGHT: "right",
+};
+
 export const ELEMENT_READY_TO_ERASE_OPACITY = 20;

 export const COOKIES = {
--- a/src/element/newElement.ts
+++ b/src/element/newElement.ts
@ -252,8 +252,16 @@ const getAdjustedDimensions = (
  };
 };

+export const getMaxContainerWidth = (container: ExcalidrawElement) => {
+  return getContainerDims(container).width - BOUND_TEXT_PADDING * 2;
+};
+
+export const getMaxContainerHeight = (container: ExcalidrawElement) => {
+  return getContainerDims(container).height - BOUND_TEXT_PADDING * 2;
+};
+
 export const updateTextElement = (
-  element: ExcalidrawTextElement,
+  textElement: ExcalidrawTextElement,
  {
    text,
    isDeleted,
@ -264,16 +272,19 @@ export const updateTextElement = (
    originalText: string;
  },
 ): ExcalidrawTextElement => {
-  const container = getContainerElement(element);
+  const container = getContainerElement(textElement);
  if (container) {
-    const containerDims = getContainerDims(container);
-    text = wrapText(text, getFontString(element), containerDims.width);
+    text = wrapText(
+      originalText,
+      getFontString(textElement),
+      getMaxContainerWidth(container),
+    );
  }
-  const dimensions = getAdjustedDimensions(element, text);
-  return newElementWith(element, {
+  const dimensions = getAdjustedDimensions(textElement, text);
+  return newElementWith(textElement, {
    text,
    originalText,
-    isDeleted: isDeleted ?? element.isDeleted,
+    isDeleted: isDeleted ?? textElement.isDeleted,
    ...dimensions,
  });
 };
--- a/src/element/textElement.test.ts
+++ b/src/element/textElement.test.ts
@ -1,3 +1,4 @@
+import { BOUND_TEXT_PADDING } from "../constants";
 import { wrapText } from "./textElement";
 import { FontString } from "./types";

@ -45,7 +46,7 @@ up`,
      },
    ].forEach((data) => {
      it(`should ${data.desc}`, () => {
-        const res = wrapText(text, font, data.width);
+        const res = wrapText(text, font, data.width - BOUND_TEXT_PADDING * 2);
        expect(res).toEqual(data.res);
      });
    });
@ -93,7 +94,7 @@ whats up`,
      },
    ].forEach((data) => {
      it(`should respect new lines and ${data.desc}`, () => {
-        const res = wrapText(text, font, data.width);
+        const res = wrapText(text, font, data.width - BOUND_TEXT_PADDING * 2);
        expect(res).toEqual(data.res);
      });
    });
@ -132,7 +133,7 @@ break it now`,
      },
    ].forEach((data) => {
      it(`should ${data.desc}`, () => {
-        const res = wrapText(text, font, data.width);
+        const res = wrapText(text, font, data.width - BOUND_TEXT_PADDING * 2);
        expect(res).toEqual(data.res);
      });
    });
--- a/src/element/textElement.ts
+++ b/src/element/textElement.ts
@ -7,42 +7,41 @@ import {
  NonDeletedExcalidrawElement,
 } from "./types";
 import { mutateElement } from "./mutateElement";
-import { BOUND_TEXT_PADDING, VERTICAL_ALIGN } from "../constants";
+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";

 export const redrawTextBoundingBox = (
-  element: ExcalidrawTextElement,
+  textElement: ExcalidrawTextElement,
  container: ExcalidrawElement | null,
 ) => {
  let maxWidth = undefined;
-  let text = element.text;
+  let text = textElement.text;

  if (container) {
-    const containerDims = getContainerDims(container);
-    maxWidth = containerDims.width - BOUND_TEXT_PADDING * 2;
+    maxWidth = getMaxContainerWidth(container);
    text = wrapText(
-      element.originalText,
-      getFontString(element),
-      containerDims.width,
+      textElement.originalText,
+      getFontString(textElement),
+      getMaxContainerWidth(container),
    );
  }
  const metrics = measureText(
-    element.originalText,
-    getFontString(element),
+    textElement.originalText,
+    getFontString(textElement),
    maxWidth,
  );
-  let coordY = element.y;
-  let coordX = element.x;
+  let coordY = textElement.y;
+  let coordX = textElement.x;
  // Resize container and vertically center align the text
  if (container) {
    const containerDims = getContainerDims(container);
    let nextHeight = containerDims.height;
-    coordX = container.x + BOUND_TEXT_PADDING;
-    if (element.verticalAlign === VERTICAL_ALIGN.TOP) {
+    if (textElement.verticalAlign === VERTICAL_ALIGN.TOP) {
      coordY = container.y + BOUND_TEXT_PADDING;
-    } else if (element.verticalAlign === VERTICAL_ALIGN.BOTTOM) {
+    } else if (textElement.verticalAlign === VERTICAL_ALIGN.BOTTOM) {
      coordY =
        container.y +
        containerDims.height -
@ -50,14 +49,25 @@ export const redrawTextBoundingBox = (
        BOUND_TEXT_PADDING;
    } else {
      coordY = container.y + containerDims.height / 2 - metrics.height / 2;
-      if (metrics.height > containerDims.height - BOUND_TEXT_PADDING * 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 + container.width / 2 - metrics.width / 2;
+    }
+
    mutateElement(container, { height: nextHeight });
  }
-  mutateElement(element, {
+
+  mutateElement(textElement, {
    width: metrics.width,
    height: metrics.height,
    baseline: metrics.baseline,
@ -118,6 +128,7 @@ export const handleBindTextResize = (
      }
      let text = textElement.text;
      let nextHeight = textElement.height;
+      let nextWidth = textElement.width;
      let containerHeight = element.height;
      let nextBaseLine = textElement.baseline;
      if (transformHandleType !== "n" && transformHandleType !== "s") {
@ -125,7 +136,7 @@ export const handleBindTextResize = (
          text = wrapText(
            textElement.originalText,
            getFontString(textElement),
-            element.width,
+            getMaxContainerWidth(element),
          );
        }

@ -135,6 +146,7 @@ export const handleBindTextResize = (
          element.width,
        );
        nextHeight = dimensions.height;
+        nextWidth = dimensions.width;
        nextBaseLine = dimensions.baseline;
      }
      // increase height in case text element height exceeds
@ -162,13 +174,12 @@ export const handleBindTextResize = (
      } else {
        updatedY = element.y + element.height / 2 - nextHeight / 2;
      }
-
+      const updatedX = element.x + element.width / 2 - nextWidth / 2;
      mutateElement(textElement, {
        text,
-        // preserve padding and set width correctly
-        width: element.width - BOUND_TEXT_PADDING * 2,
+        width: nextWidth,
        height: nextHeight,
-        x: element.x + BOUND_TEXT_PADDING,
+        x: updatedX,
        y: updatedY,
        baseline: nextBaseLine,
      });
@ -195,7 +206,6 @@ export const measureText = (
  container.style.minHeight = "1em";
  if (maxWidth) {
    const lineHeight = getApproxLineHeight(font);
-    container.style.width = `${String(maxWidth)}px`;
    container.style.maxWidth = `${String(maxWidth)}px`;
    container.style.overflow = "hidden";
    container.style.wordBreak = "break-word";
@ -213,7 +223,8 @@ export const measureText = (
  container.appendChild(span);
  // Baseline is important for positioning text on canvas
  const baseline = span.offsetTop + span.offsetHeight;
-  const width = container.offsetWidth;
+  // Since span adds 1px extra width to the container
+  const width = container.offsetWidth + 1;

  const height = container.offsetHeight;
  document.body.removeChild(container);
@ -251,13 +262,7 @@ const getTextWidth = (text: string, font: FontString) => {
  return metrics.width;
 };

-export const wrapText = (
-  text: string,
-  font: FontString,
-  containerWidth: number,
-) => {
-  const maxWidth = containerWidth - BOUND_TEXT_PADDING * 2;
-
+export const wrapText = (text: string, font: FontString, maxWidth: number) => {
  const lines: Array<string> = [];
  const originalLines = text.split("\n");
  const spaceWidth = getTextWidth(" ", font);
--- a/src/element/textWysiwyg.tsx
+++ b/src/element/textWysiwyg.tsx
@ -28,6 +28,7 @@ import {
 } from "../actions/actionProperties";
 import { actionZoomIn, actionZoomOut } from "../actions/actionCanvas";
 import App from "../components/App";
+import { getMaxContainerWidth } from "./newElement";

 const normalizeText = (text: string) => {
  return (
@ -114,13 +115,13 @@ export const textWysiwyg = ({
      getFontString(updatedTextElement),
    );
    if (updatedTextElement && isTextElement(updatedTextElement)) {
-      let coordX = updatedTextElement.x;
+      const coordX = updatedTextElement.x;
      let coordY = updatedTextElement.y;
      const container = getContainerElement(updatedTextElement);
      let maxWidth = updatedTextElement.width;

      let maxHeight = updatedTextElement.height;
-      let width = updatedTextElement.width;
+      const width = updatedTextElement.width;
      // Set to element height by default since that's
      // what is going to be used for unbounded text
      let height = updatedTextElement.height;
@ -146,10 +147,6 @@ export const textWysiwyg = ({
        }
        maxWidth = containerDims.width - BOUND_TEXT_PADDING * 2;
        maxHeight = containerDims.height - BOUND_TEXT_PADDING * 2;
-        width = maxWidth;
-        // The coordinates of text box set a distance of
-        // 5px to preserve padding
-        coordX = container.x + BOUND_TEXT_PADDING;
        // autogrow container height if text exceeds
        if (height > maxHeight) {
          const diff = Math.min(height - maxHeight, approxLineHeight);
@ -212,7 +209,7 @@ export const textWysiwyg = ({
        font: getFontString(updatedTextElement),
        // must be defined *after* font ¯\_(ツ)_/¯
        lineHeight: `${lineHeight}px`,
-        width: `${width}px`,
+        width: `${Math.min(width, maxWidth)}px`,
        height: `${height}px`,
        left: `${viewportX}px`,
        top: `${viewportY}px`,
@ -229,7 +226,6 @@ export const textWysiwyg = ({
        color: updatedTextElement.strokeColor,
        opacity: updatedTextElement.opacity / 100,
        filter: "var(--theme-filter)",
-        maxWidth: `${maxWidth}px`,
        maxHeight: `${editorMaxHeight}px`,
      });
      // For some reason updating font attribute doesn't set font family
@ -301,13 +297,14 @@ export const textWysiwyg = ({
      // doubles the height as soon as user starts typing
      if (isBoundToContainer(element) && lines > 1) {
        let height = "auto";
-
+        editable.style.height = "0px";
+        let heightSet = false;
        if (lines === 2) {
          const container = getContainerElement(element);
          const actualLineCount = wrapText(
            editable.value,
            font,
-            container!.width,
+            getMaxContainerWidth(container!),
          ).split("\n").length;
          // This is browser behaviour when setting height to "auto"
          // It sets the height needed for 2 lines even if actual
@ -316,10 +313,13 @@ export const textWysiwyg = ({
          // so single line aligns vertically when deleting
          if (actualLineCount === 1) {
            height = `${editable.scrollHeight / 2}px`;
+            editable.style.height = height;
+            heightSet = true;
          }
        }
-        editable.style.height = height;
-        editable.style.height = `${editable.scrollHeight}px`;
+        if (!heightSet) {
+          editable.style.height = `${editable.scrollHeight}px`;
+        }
      }
      onChange(normalizeText(editable.value));
    };
--- a/src/element/types.ts
+++ b/src/element/types.ts
@ -1,5 +1,5 @@
 import { Point } from "../types";
-import { FONT_FAMILY, THEME, VERTICAL_ALIGN } from "../constants";
+import { FONT_FAMILY, TEXT_ALIGN, THEME, VERTICAL_ALIGN } from "../constants";

 export type ChartType = "bar" | "line";
 export type FillStyle = "hachure" | "cross-hatch" | "solid";
@ -11,7 +11,7 @@ export type GroupId = string;
 export type PointerType = "mouse" | "pen" | "touch";
 export type StrokeSharpness = "round" | "sharp";
 export type StrokeStyle = "solid" | "dashed" | "dotted";
-export type TextAlign = "left" | "center" | "right";
+export type TextAlign = typeof TEXT_ALIGN[keyof typeof TEXT_ALIGN];

 type VerticalAlignKeys = keyof typeof VERTICAL_ALIGN;
 export type VerticalAlign = typeof VERTICAL_ALIGN[VerticalAlignKeys];
--- a/src/tests/data/snapshots/restore.test.ts.snap
+++ b/src/tests/data/snapshots/restore.test.ts.snap
@ -296,7 +296,7 @@ Object {
  "versionNonce": 0,
  "verticalAlign": "middle",
  "width": 100,
-  "x": 0,
+  "x": -0.5,
  "y": 0,
 }
 `;