From 7b36de04764dfe94b68b8ce5d16c46b8bcf5d1c6 Mon Sep 17 00:00:00 2001
From: Ryan Di <ryan.weihao.di@gmail.com>
Date: Sat, 27 Jul 2024 21:02:00 +0800
Subject: [PATCH] fix: linear elements not selected on pointer up from hitting
 its bound text (#8285)

Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com>
---
 packages/excalidraw/components/App.tsx     | 36 ++--------------------
 packages/excalidraw/element/collision.ts   | 11 +++++--
 packages/excalidraw/element/textElement.ts |  3 +-
 packages/excalidraw/shapes.tsx             | 30 ++++++++++++++++++
 4 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx
index 6f1ac7ffd3..cf23af641b 100644
--- a/packages/excalidraw/components/App.tsx
+++ b/packages/excalidraw/components/App.tsx
@@ -224,8 +224,7 @@ import type {
   ScrollBars,
 } from "../scene/types";
 import { getStateForZoom } from "../scene/zoom";
-import { findShapeByKey, getElementShape } from "../shapes";
-import type { GeometricShape } from "../../utils/geometry/shape";
+import { findShapeByKey, getBoundTextShape, getElementShape } from "../shapes";
 import { getSelectionBoxShape } from "../../utils/geometry/shape";
 import { isPointInShape } from "../../utils/collision";
 import type {
@@ -4515,37 +4514,6 @@ class App extends React.Component<AppProps, AppState> {
     return null;
   }
 
-  private getBoundTextShape(element: ExcalidrawElement): GeometricShape | null {
-    const boundTextElement = getBoundTextElement(
-      element,
-      this.scene.getNonDeletedElementsMap(),
-    );
-
-    if (boundTextElement) {
-      if (element.type === "arrow") {
-        return getElementShape(
-          {
-            ...boundTextElement,
-            // arrow's bound text accurate position is not stored in the element's property
-            // but rather calculated and returned from the following static method
-            ...LinearElementEditor.getBoundTextElementPosition(
-              element,
-              boundTextElement,
-              this.scene.getNonDeletedElementsMap(),
-            ),
-          },
-          this.scene.getNonDeletedElementsMap(),
-        );
-      }
-      return getElementShape(
-        boundTextElement,
-        this.scene.getNonDeletedElementsMap(),
-      );
-    }
-
-    return null;
-  }
-
   private getElementAtPosition(
     x: number,
     y: number,
@@ -4677,7 +4645,7 @@ class App extends React.Component<AppProps, AppState> {
     const hitBoundTextOfElement = hitElementBoundText(
       x,
       y,
-      this.getBoundTextShape(element),
+      getBoundTextShape(element, this.scene.getNonDeletedElementsMap()),
     );
     if (hitBoundTextOfElement) {
       return true;
diff --git a/packages/excalidraw/element/collision.ts b/packages/excalidraw/element/collision.ts
index 704c245564..954326ca0d 100644
--- a/packages/excalidraw/element/collision.ts
+++ b/packages/excalidraw/element/collision.ts
@@ -18,6 +18,7 @@ import {
   isImageElement,
   isTextElement,
 } from "./typeChecks";
+import { getBoundTextShape } from "../shapes";
 
 export const shouldTestInside = (element: ExcalidrawElement) => {
   if (element.type === "arrow") {
@@ -97,6 +98,12 @@ export const hitElementBoundingBoxOnly = (
 ) => {
   return (
     !hitElementItself(hitArgs) &&
+    // bound text is considered part of the element (even if it's outside the bounding box)
+    !hitElementBoundText(
+      hitArgs.x,
+      hitArgs.y,
+      getBoundTextShape(hitArgs.element, elementsMap),
+    ) &&
     hitElementBoundingBox(hitArgs.x, hitArgs.y, hitArgs.element, elementsMap)
   );
 };
@@ -105,6 +112,6 @@ export const hitElementBoundText = (
   x: number,
   y: number,
   textShape: GeometricShape | null,
-) => {
-  return textShape && isPointInShape([x, y], textShape);
+): boolean => {
+  return !!textShape && isPointInShape([x, y], textShape);
 };
diff --git a/packages/excalidraw/element/textElement.ts b/packages/excalidraw/element/textElement.ts
index 0a84813704..696f809e6b 100644
--- a/packages/excalidraw/element/textElement.ts
+++ b/packages/excalidraw/element/textElement.ts
@@ -635,8 +635,7 @@ export const getMaxCharWidth = (font: FontString) => {
 
 export const getBoundTextElementId = (container: ExcalidrawElement | null) => {
   return container?.boundElements?.length
-    ? container?.boundElements?.filter((ele) => ele.type === "text")[0]?.id ||
-        null
+    ? container?.boundElements?.find((ele) => ele.type === "text")?.id || null
     : null;
 };
 
diff --git a/packages/excalidraw/shapes.tsx b/packages/excalidraw/shapes.tsx
index 5c0e986769..acdca238dc 100644
--- a/packages/excalidraw/shapes.tsx
+++ b/packages/excalidraw/shapes.tsx
@@ -20,6 +20,8 @@ import {
 } from "./components/icons";
 import { getElementAbsoluteCoords } from "./element";
 import { shouldTestInside } from "./element/collision";
+import { LinearElementEditor } from "./element/linearElementEditor";
+import { getBoundTextElement } from "./element/textElement";
 import type { ElementsMap, ExcalidrawElement } from "./element/types";
 import { KEYS } from "./keys";
 import { ShapeCache } from "./scene/ShapeCache";
@@ -159,3 +161,31 @@ export const getElementShape = (
     }
   }
 };
+
+export const getBoundTextShape = (
+  element: ExcalidrawElement,
+  elementsMap: ElementsMap,
+): GeometricShape | null => {
+  const boundTextElement = getBoundTextElement(element, elementsMap);
+
+  if (boundTextElement) {
+    if (element.type === "arrow") {
+      return getElementShape(
+        {
+          ...boundTextElement,
+          // arrow's bound text accurate position is not stored in the element's property
+          // but rather calculated and returned from the following static method
+          ...LinearElementEditor.getBoundTextElementPosition(
+            element,
+            boundTextElement,
+            elementsMap,
+          ),
+        },
+        elementsMap,
+      );
+    }
+    return getElementShape(boundTextElement, elementsMap);
+  }
+
+  return null;
+};