From c1247742eaafd768a616dffc0f656ba56e301107 Mon Sep 17 00:00:00 2001
From: David Luzar <luzar.david@gmail.com>
Date: Wed, 26 Jul 2023 23:28:11 +0200
Subject: [PATCH] fix: prevent binding focus NaN value (#6803)

Co-authored-by: Aakansha Doshi <aakansha1216@gmail.com>
---
 src/data/restore.ts        | 12 +++++++--
 src/element/collision.ts   | 11 +++++---
 src/global.d.ts            |  6 +++++
 src/tests/binding.test.tsx | 55 ++++++++++++++++++++++++++++++++++++++
 src/tests/test-utils.ts    | 16 +++++++++++
 5 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/src/data/restore.ts b/src/data/restore.ts
index 49b146b5..08fbe093 100644
--- a/src/data/restore.ts
+++ b/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,
diff --git a/src/element/collision.ts b/src/element/collision.ts
index c8164a70..1878b93b 100644
--- a/src/element/collision.ts
+++ b/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 = (
diff --git a/src/global.d.ts b/src/global.d.ts
index 3a666e11..977e32dd 100644
--- a/src/global.d.ts
+++ b/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;
+  }
+}
diff --git a/src/tests/binding.test.tsx b/src/tests/binding.test.tsx
index 07af3656..5a566328 100644
--- a/src/tests/binding.test.tsx
+++ b/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", {
diff --git a/src/tests/test-utils.ts b/src/tests/test-utils.ts
index bb771b19..1855a7fe 100644
--- a/src/tests/test-utils.ts
+++ b/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,
+    };
+  },
+});