fix(editor): detect line height from the migrated font props on restore (#12033)

* fix(editor): detect line height from the migrated font props on restore

The text branch of `restoreElement` parses the legacy `font` prop into local
`fontSize` and `fontFamily`, but line-height detection kept reading
`element.fontSize` and `element.fontFamily`. A diagram old enough to carry
`font` has neither, so `detectLineHeight` divided by `undefined` and the
element was restored with `lineHeight: NaN`. A non-numeric `height` produced
the same result.

Move the detection into `restoreLineHeight`, pass it the already-migrated
values, and fall back to the font family's fixed line height when the
detected value isn't a usable number.

---------

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
Co-authored-by: Mahiro Hirakawa <mahirohirakawa@glovrex.com>

packages/excalidraw/data/restore.ts

@@ -295,6 +295,26 @@ const getFontFamilyByName = (fontFamilyName: string): FontFamilyValues => {
   return DEFAULT_FONT_FAMILY;
 };
 
+/**
+ * Line-height might not be specified either when creating elements
+ * programmatically, or when importing old diagrams. For the latter we want to
+ * detect the original line height, which will likely differ from our per-font
+ * fixed line height we now use, to maintain backward compatibility.
+ *
+ * Takes the element with the font properties already migrated: a diagram old
+ * enough to carry the legacy `font` prop has neither `fontSize` nor
+ * `fontFamily`, so detecting off the raw element divides by `undefined`.
+ */
+const restoreLineHeight = (
+  element: ExcalidrawTextElement,
+): ExcalidrawTextElement["lineHeight"] => {
+  const detected = element.height ? detectLineHeight(element) : null;
+
+  return isFiniteNumber(detected) && detected > 0
+    ? detected
+    : getLineHeight(element.fontFamily);
+};
+
 const repairBinding = <T extends ExcalidrawArrowElement>(
   element: T,
   binding: FixedPointBinding | null,
@@ -547,19 +567,9 @@ export const restoreElement = (
       }
       const text = (typeof element.text === "string" && element.text) || "";
 
-      // line-height might not be specified either when creating elements
-      // programmatically, or when importing old diagrams.
-      // For the latter we want to detect the original line height which
-      // will likely differ from our per-font fixed line height we now use,
-      // to maintain backward compatibility.
       const lineHeight =
         element.lineHeight ||
-        (element.height
-          ? // detect line-height from current element height and font-size
-            detectLineHeight(element)
-          : // no element height likely means programmatic use, so default
-            // to a fixed line height
-            getLineHeight(element.fontFamily));
+        restoreLineHeight({ ...element, fontSize, fontFamily, text });
       element = restoreElementWithProperties(element, {
         fontSize,
         fontFamily,

packages/excalidraw/tests/data/restore.test.ts

@@ -6,6 +6,7 @@ import {
   DEFAULT_FONT_SIZE,
   DEFAULT_SIDEBAR,
   FONT_FAMILY,
+  getLineHeight,
   ROUNDNESS,
   STICKY_NOTE_MAX_FONT_SIZE,
   DEFAULT_STICKY_NOTE_BG,
@@ -212,6 +213,49 @@ describe("restoreElements", () => {
     });
   });
 
+  it("should detect the line height of a text element that only has the legacy font prop", () => {
+    const textElement: any = API.createElement({
+      type: "text",
+      id: "id-text-legacy-font",
+    });
+
+    // a diagram old enough to carry `font` has none of the props that
+    // replaced it
+    delete textElement.fontSize;
+    delete textElement.fontFamily;
+    delete textElement.lineHeight;
+    textElement.font = "20px Virgil";
+    textElement.text = "hi";
+    textElement.height = 50;
+
+    const restoredText = restore.restoreElements(
+      [textElement],
+      null,
+    )[0] as ExcalidrawTextElement;
+
+    expect(restoredText.fontSize).toBe(20);
+    expect(restoredText.fontFamily).toBe(FONT_FAMILY.Virgil);
+    expect(restoredText.lineHeight).toBe(2.5);
+  });
+
+  it("should fall back to the font line height when it can't be detected", () => {
+    const textElement: any = API.createElement({
+      type: "text",
+      id: "id-text-unusable-height",
+    });
+
+    delete textElement.lineHeight;
+    textElement.text = "hi";
+    textElement.height = "unknown";
+
+    const restoredText = restore.restoreElements(
+      [textElement],
+      null,
+    )[0] as ExcalidrawTextElement;
+
+    expect(restoredText.lineHeight).toBe(getLineHeight(textElement.fontFamily));
+  });
+
   it("should sanitize non-finite font sizes on text elements", () => {
     const textElement: any = API.createElement({
       type: "text",