fix: prevent binding focus NaN value (#6803)

Co-authored-by: Aakansha Doshi <aakansha1216@gmail.com>

src/data/restore.ts

@@ -3,6 +3,7 @@ import {
   ExcalidrawSelectionElement,
   ExcalidrawTextElement,
   FontFamilyValues,
+  PointBinding,
   StrokeRoundness,
 } from "../element/types";
 import {
@@ -83,6 +84,13 @@ const getFontFamilyByName = (fontFamilyName: string): FontFamilyValues => {
   return DEFAULT_FONT_FAMILY;
 };
 
+const repairBinding = (binding: PointBinding | null) => {
+  if (!binding) {
+    return null;
+  }
+  return { ...binding, focus: binding.focus || 0 };
+};
+
 const restoreElementWithProperties = <
   T extends Required<Omit<ExcalidrawElement, "customData">> & {
     customData?: ExcalidrawElement["customData"];
@@ -258,8 +266,8 @@ const restoreElement = (
           (element.type as ExcalidrawElement["type"] | "draw") === "draw"
             ? "line"
             : element.type,
-        startBinding: element.startBinding,
-        endBinding: element.endBinding,
+        startBinding: repairBinding(element.startBinding),
+        endBinding: repairBinding(element.endBinding),
         lastCommittedPoint: null,
         startArrowhead,
         endArrowhead,

src/element/collision.ts

@@ -655,18 +655,23 @@ export const determineFocusDistance = (
   const c = line[1];
   const mabs = Math.abs(m);
   const nabs = Math.abs(n);
+  let ret;
   switch (element.type) {
     case "rectangle":
     case "image":
     case "text":
     case "embeddable":
     case "frame":
-      return c / (hwidth * (nabs + q * mabs));
+      ret = c / (hwidth * (nabs + q * mabs));
+      break;
     case "diamond":
-      return mabs < nabs ? c / (nabs * hwidth) : c / (mabs * hheight);
+      ret = mabs < nabs ? c / (nabs * hwidth) : c / (mabs * hheight);
+      break;
     case "ellipse":
-      return c / (hwidth * Math.sqrt(n ** 2 + q ** 2 * m ** 2));
+      ret = c / (hwidth * Math.sqrt(n ** 2 + q ** 2 * m ** 2));
+      break;
   }
+  return ret || 0;
 };
 
 export const determineFocusPoint = (

src/global.d.ts

@@ -120,3 +120,9 @@ declare module "image-blob-reduce" {
   const reduce: ImageBlobReduce.ImageBlobReduceStatic;
   export = reduce;
 }
+
+declare namespace jest {
+  interface Expect {
+    toBeNonNaNNumber(): void;
+  }
+}

src/tests/binding.test.tsx

@@ -15,6 +15,61 @@ describe("element binding", () => {
     await render(<ExcalidrawApp />);
   });
 
+  it("should create valid binding if duplicate start/end points", async () => {
+    const rect = API.createElement({
+      type: "rectangle",
+      x: 0,
+      width: 50,
+      height: 50,
+    });
+    const arrow = API.createElement({
+      type: "arrow",
+      x: 100,
+      y: 0,
+      width: 100,
+      height: 1,
+      points: [
+        [0, 0],
+        [0, 0],
+        [100, 0],
+        [100, 0],
+      ],
+    });
+    h.elements = [rect, arrow];
+    expect(arrow.startBinding).toBe(null);
+
+    API.setSelectedElements([arrow]);
+
+    expect(API.getSelectedElements()).toEqual([arrow]);
+    mouse.downAt(100, 0);
+    mouse.moveTo(55, 0);
+    mouse.up(0, 0);
+    expect(arrow.startBinding).toEqual({
+      elementId: rect.id,
+      focus: expect.toBeNonNaNNumber(),
+      gap: expect.toBeNonNaNNumber(),
+    });
+
+    mouse.downAt(100, 0);
+    mouse.move(-45, 0);
+    mouse.up();
+    expect(arrow.startBinding).toEqual({
+      elementId: rect.id,
+      focus: expect.toBeNonNaNNumber(),
+      gap: expect.toBeNonNaNNumber(),
+    });
+
+    mouse.down();
+    mouse.move(-50, 0);
+    mouse.up();
+    expect(arrow.startBinding).toBe(null);
+    expect(arrow.endBinding).toEqual({
+      elementId: rect.id,
+      focus: expect.toBeNonNaNNumber(),
+      gap: expect.toBeNonNaNNumber(),
+    });
+  });
+
   //@TODO fix the test with rotation
   it.skip("rotation of arrow should rebind both ends", () => {
     const rectLeft = UI.createElement("rectangle", {

src/tests/test-utils.ts

@@ -228,3 +228,19 @@ export const togglePopover = (label: string) => {
 
   UI.clickLabeledElement(label);
 };
+
+expect.extend({
+  toBeNonNaNNumber(received) {
+    const pass = typeof received === "number" && !isNaN(received);
+    if (pass) {
+      return {
+        message: () => `expected ${received} not to be a non-NaN number`,
+        pass: true,
+      };
+    }
+    return {
+      message: () => `expected ${received} to be a non-NaN number`,
+      pass: false,
+    };
+  },
+});