fix: resizing arrow labels (#6789)

* fix arrow labels resizing

- min arrow labels width based on font size
- labels width and padding in % of container's width
- resize labels simply multiplying by scale

* remove no longer needed getContainerDims

* fix arrow labels font size not updated on change font size action

* fix bound arrows not updated right after resize

* fix test

* fix 3+ point arrow label resizing with shift

* fix bound text not scaling when resizing with shift & n or s handle

* fix arrow labels width not updating when moving a 2-point arrow point with shift

---------

Co-authored-by: Aakansha Doshi <[email protected]>

src/components/App.tsx

@@ -298,7 +298,6 @@ import {
   getApproxMinLineWidth,
   getBoundTextElement,
   getContainerCenter,
-  getContainerDims,
   getContainerElement,
   getDefaultLineHeight,
   getLineHeightInPx,
@@ -3548,9 +3547,8 @@ class App extends React.Component<AppProps, AppState> {
         lineHeight,
       );
       const minHeight = getApproxMinLineHeight(fontSize, lineHeight);
-      const containerDims = getContainerDims(container);
-      const newHeight = Math.max(containerDims.height, minHeight);
-      const newWidth = Math.max(containerDims.width, minWidth);
+      const newHeight = Math.max(container.height, minHeight);
+      const newWidth = Math.max(container.width, minWidth);
       mutateElement(container, { height: newHeight, width: newWidth });
       sceneX = container.x + newWidth / 2;
       sceneY = container.y + newHeight / 2;

src/constants.ts

@@ -117,6 +117,7 @@ export const FRAME_STYLE = {
 
 export const WINDOWS_EMOJI_FALLBACK_FONT = "Segoe UI Emoji";
 
+export const MIN_FONT_SIZE = 1;
 export const DEFAULT_FONT_SIZE = 20;
 export const DEFAULT_FONT_FAMILY: FontFamilyValues = FONT_FAMILY.Virgil;
 export const DEFAULT_TEXT_ALIGN = "left";
@@ -239,6 +240,8 @@ export const VERSIONS = {
 } as const;
 
 export const BOUND_TEXT_PADDING = 5;
+export const ARROW_LABEL_WIDTH_FRACTION = 0.7;
+export const ARROW_LABEL_FONT_SIZE_TO_MIN_WIDTH_RATIO = 11;
 
 export const VERTICAL_ALIGN = {
   TOP: "top",

src/element/linearElementEditor.ts

@@ -264,11 +264,11 @@ export class LinearElementEditor {
             };
           }),
         );
+      }
 
-        const boundTextElement = getBoundTextElement(element);
-        if (boundTextElement) {
-          handleBindTextResize(element, false);
-        }
+      const boundTextElement = getBoundTextElement(element);
+      if (boundTextElement) {
+        handleBindTextResize(element, false);
       }
 
       // suggest bindings for first and last point if selected

src/element/resizeElements.ts

@@ -1,4 +1,4 @@
-import { SHIFT_LOCKING_ANGLE } from "../constants";
+import { MIN_FONT_SIZE, SHIFT_LOCKING_ANGLE } from "../constants";
 import { rescalePoints } from "../points";
 
 import {
@@ -204,8 +204,6 @@ const rescalePointsInElement = (
       }
     : {};
 
-const MIN_FONT_SIZE = 1;
-
 const measureFontSizeFromWidth = (
   element: NonDeleted<ExcalidrawTextElement>,
   nextWidth: number,
@@ -589,24 +587,42 @@ export const resizeSingleElement = (
     });
   }
 
+  if (
+    isArrowElement(element) &&
+    boundTextElement &&
+    shouldMaintainAspectRatio
+  ) {
+    const fontSize =
+      (resizedElement.width / element.width) * boundTextElement.fontSize;
+    if (fontSize < MIN_FONT_SIZE) {
+      return;
+    }
+    boundTextFont.fontSize = fontSize;
+  }
+
   if (
     resizedElement.width !== 0 &&
     resizedElement.height !== 0 &&
     Number.isFinite(resizedElement.x) &&
     Number.isFinite(resizedElement.y)
   ) {
+    mutateElement(element, resizedElement);
+
     updateBoundElements(element, {
       newSize: { width: resizedElement.width, height: resizedElement.height },
     });
 
-    mutateElement(element, resizedElement);
     if (boundTextElement && boundTextFont != null) {
       mutateElement(boundTextElement, {
         fontSize: boundTextFont.fontSize,
         baseline: boundTextFont.baseline,
       });
     }
-    handleBindTextResize(element, transformHandleDirection);
+    handleBindTextResize(
+      element,
+      transformHandleDirection,
+      shouldMaintainAspectRatio,
+    );
   }
 };
 
@@ -722,12 +738,8 @@ export const resizeMultipleElements = (
       fontSize?: ExcalidrawTextElement["fontSize"];
       baseline?: ExcalidrawTextElement["baseline"];
       scale?: ExcalidrawImageElement["scale"];
+      boundTextFontSize?: ExcalidrawTextElement["fontSize"];
     };
-    boundText: {
-      element: ExcalidrawTextElementWithContainer;
-      fontSize: ExcalidrawTextElement["fontSize"];
-      baseline: ExcalidrawTextElement["baseline"];
-    } | null;
   }[] = [];
 
   for (const { orig, latest } of targetElements) {
@@ -798,50 +810,39 @@ export const resizeMultipleElements = (
       }
     }
 
-    let boundText: typeof elementsAndUpdates[0]["boundText"] = null;
-
-    const boundTextElement = getBoundTextElement(latest);
-
-    if (boundTextElement || isTextElement(orig)) {
-      const updatedElement = {
-        ...latest,
-        width,
-        height,
-      };
-      const metrics = measureFontSizeFromWidth(
-        boundTextElement ?? (orig as ExcalidrawTextElement),
-        boundTextElement
-          ? getBoundTextMaxWidth(updatedElement)
-          : updatedElement.width,
-        boundTextElement
-          ? getBoundTextMaxHeight(updatedElement, boundTextElement)
-          : updatedElement.height,
-      );
-
+    if (isTextElement(orig)) {
+      const metrics = measureFontSizeFromWidth(orig, width, height);
       if (!metrics) {
         return;
       }
+      update.fontSize = metrics.size;
+      update.baseline = metrics.baseline;
+    }
 
-      if (isTextElement(orig)) {
-        update.fontSize = metrics.size;
-        update.baseline = metrics.baseline;
-      }
+    const boundTextElement = pointerDownState.originalElements.get(
+      getBoundTextElementId(orig) ?? "",
+    ) as ExcalidrawTextElementWithContainer | undefined;
 
-      if (boundTextElement) {
-        boundText = {
-          element: boundTextElement,
-          fontSize: metrics.size,
-          baseline: metrics.baseline,
-        };
+    if (boundTextElement) {
+      const newFontSize = boundTextElement.fontSize * scale;
+      if (newFontSize < MIN_FONT_SIZE) {
+        return;
       }
+      update.boundTextFontSize = newFontSize;
     }
 
-    elementsAndUpdates.push({ element: latest, update, boundText });
+    elementsAndUpdates.push({
+      element: latest,
+      update,
+    });
   }
 
   const elementsToUpdate = elementsAndUpdates.map(({ element }) => element);
 
-  for (const { element, update, boundText } of elementsAndUpdates) {
+  for (const {
+    element,
+    update: { boundTextFontSize, ...update },
+  } of elementsAndUpdates) {
     const { width, height, angle } = update;
 
     mutateElement(element, update, false);
@@ -851,17 +852,17 @@ export const resizeMultipleElements = (
       newSize: { width, height },
     });
 
-    if (boundText) {
-      const { element: boundTextElement, ...boundTextUpdates } = boundText;
+    const boundTextElement = getBoundTextElement(element);
+    if (boundTextElement && boundTextFontSize) {
       mutateElement(
         boundTextElement,
         {
-          ...boundTextUpdates,
+          fontSize: boundTextFontSize,
           angle: isLinearElement(element) ? undefined : angle,
         },
         false,
       );
-      handleBindTextResize(element, transformHandleType);
+      handleBindTextResize(element, transformHandleType, true);
     }
   }
 

src/element/textElement.ts

@@ -10,6 +10,8 @@ import {
 } from "./types";
 import { mutateElement } from "./mutateElement";
 import {
+  ARROW_LABEL_FONT_SIZE_TO_MIN_WIDTH_RATIO,
+  ARROW_LABEL_WIDTH_FRACTION,
   BOUND_TEXT_PADDING,
   DEFAULT_FONT_FAMILY,
   DEFAULT_FONT_SIZE,
@@ -65,7 +67,7 @@ export const redrawTextBoundingBox = (
   boundTextUpdates.text = textElement.text;
 
   if (container) {
-    maxWidth = getBoundTextMaxWidth(container);
+    maxWidth = getBoundTextMaxWidth(container, textElement);
     boundTextUpdates.text = wrapText(
       textElement.originalText,
       getFontString(textElement),
@@ -83,13 +85,12 @@ export const redrawTextBoundingBox = (
   boundTextUpdates.baseline = metrics.baseline;
 
   if (container) {
-    const containerDims = getContainerDims(container);
     const maxContainerHeight = getBoundTextMaxHeight(
       container,
       textElement as ExcalidrawTextElementWithContainer,
     );
 
-    let nextHeight = containerDims.height;
+    let nextHeight = container.height;
     if (metrics.height > maxContainerHeight) {
       nextHeight = computeContainerDimensionForBoundText(
         metrics.height,
@@ -155,6 +156,7 @@ export const bindTextToShapeAfterDuplication = (
 export const handleBindTextResize = (
   container: NonDeletedExcalidrawElement,
   transformHandleType: MaybeTransformHandleType,
+  shouldMaintainAspectRatio = false,
 ) => {
   const boundTextElementId = getBoundTextElementId(container);
   if (!boundTextElementId) {
@@ -175,15 +177,17 @@ export const handleBindTextResize = (
     let text = textElement.text;
     let nextHeight = textElement.height;
     let nextWidth = textElement.width;
-    const containerDims = getContainerDims(container);
     const maxWidth = getBoundTextMaxWidth(container);
     const maxHeight = getBoundTextMaxHeight(
       container,
       textElement as ExcalidrawTextElementWithContainer,
     );
-    let containerHeight = containerDims.height;
+    let containerHeight = container.height;
     let nextBaseLine = textElement.baseline;
-    if (transformHandleType !== "n" && transformHandleType !== "s") {
+    if (
+      shouldMaintainAspectRatio ||
+      (transformHandleType !== "n" && transformHandleType !== "s")
+    ) {
       if (text) {
         text = wrapText(
           textElement.originalText,
@@ -207,7 +211,7 @@ export const handleBindTextResize = (
         container.type,
       );
 
-      const diff = containerHeight - containerDims.height;
+      const diff = containerHeight - container.height;
       // fix the y coord when resizing from ne/nw/n
       const updatedY =
         !isArrowElement(container) &&
@@ -687,16 +691,6 @@ export const getContainerElement = (
   return null;
 };
 
-export const getContainerDims = (element: ExcalidrawElement) => {
-  const MIN_WIDTH = 300;
-  if (isArrowElement(element)) {
-    const width = Math.max(element.width, MIN_WIDTH);
-    const height = element.height;
-    return { width, height };
-  }
-  return { width: element.width, height: element.height };
-};
-
 export const getContainerCenter = (
   container: ExcalidrawElement,
   appState: AppState,
@@ -887,12 +881,19 @@ export const computeContainerDimensionForBoundText = (
   return dimension + padding;
 };
 
-export const getBoundTextMaxWidth = (container: ExcalidrawElement) => {
-  const width = getContainerDims(container).width;
+export const getBoundTextMaxWidth = (
+  container: ExcalidrawElement,
+  boundTextElement: ExcalidrawTextElement | null = getBoundTextElement(
+    container,
+  ),
+) => {
+  const { width } = container;
   if (isArrowElement(container)) {
-    return width - BOUND_TEXT_PADDING * 8 * 2;
+    const minWidth =
+      (boundTextElement?.fontSize ?? DEFAULT_FONT_SIZE) *
+      ARROW_LABEL_FONT_SIZE_TO_MIN_WIDTH_RATIO;
+    return Math.max(ARROW_LABEL_WIDTH_FRACTION * width, minWidth);
   }
-
   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
@@ -911,7 +912,7 @@ export const getBoundTextMaxHeight = (
   container: ExcalidrawElement,
   boundTextElement: ExcalidrawTextElementWithContainer,
 ) => {
-  const height = getContainerDims(container).height;
+  const { height } = container;
   if (isArrowElement(container)) {
     const containerHeight = height - BOUND_TEXT_PADDING * 8 * 2;
     if (containerHeight <= 0) {

src/element/textWysiwyg.tsx

@@ -23,7 +23,6 @@ import { AppState } from "../types";
 import { mutateElement } from "./mutateElement";
 import {
   getBoundTextElementId,
-  getContainerDims,
   getContainerElement,
   getTextElementAngle,
   getTextWidth,
@@ -177,20 +176,19 @@ export const textWysiwyg = ({
           updatedTextElement,
           editable,
         );
-        const containerDims = getContainerDims(container);
 
         let originalContainerData;
         if (propertiesUpdated) {
           originalContainerData = updateOriginalContainerCache(
             container.id,
-            containerDims.height,
+            container.height,
           );
         } else {
           originalContainerData = originalContainerCache[container.id];
           if (!originalContainerData) {
             originalContainerData = updateOriginalContainerCache(
               container.id,
-              containerDims.height,
+              container.height,
             );
           }
         }
@@ -214,7 +212,7 @@ export const textWysiwyg = ({
           // autoshrink container height until original container height
           // is reached when text is removed
           !isArrowElement(container) &&
-          containerDims.height > originalContainerData.height &&
+          container.height > originalContainerData.height &&
           textElementHeight < maxHeight
         ) {
           const targetContainerHeight = computeContainerDimensionForBoundText(

src/tests/linearElementEditor.test.tsx

@@ -1128,7 +1128,7 @@ describe("Test Linear Elements", () => {
         height: 500,
       });
       const arrow = UI.createElement("arrow", {
-        x: 210,
+        x: -10,
         y: 250,
         width: 400,
         height: 1,
@@ -1152,8 +1152,8 @@ describe("Test Linear Elements", () => {
       expect(
         wrapText(textElement.originalText, font, getBoundTextMaxWidth(arrow)),
       ).toMatchInlineSnapshot(`
-        "Online whiteboard collaboration
-        made easy"
+        "Online whiteboard 
+        collaboration made easy"
       `);
       const handleBindTextResizeSpy = vi.spyOn(
         textElementUtils,
@@ -1165,7 +1165,7 @@ describe("Test Linear Elements", () => {
       mouse.moveTo(200, 0);
       mouse.upAt(200, 0);
 
-      expect(arrow.width).toBe(170);
+      expect(arrow.width).toBe(200);
       expect(rect.x).toBe(200);
       expect(rect.y).toBe(0);
       expect(handleBindTextResizeSpy).toHaveBeenCalledWith(