fix: prevent double-click to edit/create text scenarios on line (#9597)

* fix : double click on line enables line editor

* fix : prevent double-click to edit/create text
when inside line editor

* refactor: use lineCheck instead of arrowCheck in
doubleClick handler to align with updated logic

* fix: replace negative arrowCheck with lineCheck in
dbl click handler and fix double-click bind text
test in linearElementEditor tests

* clean up test

* simplify check

* add tests

* prevent text editing on dblclick when inside arrow editor

---------

Co-authored-by: dwelle <[email protected]>

packages/element/src/typeChecks.ts

@@ -129,6 +129,15 @@ export const isElbowArrow = (
   return isArrowElement(element) && element.elbowed;
 };
 
+/**
+ * sharp or curved arrow, but not elbow
+ */
+export const isSimpleArrow = (
+  element?: ExcalidrawElement,
+): element is ExcalidrawArrowElement => {
+  return isArrowElement(element) && !element.elbowed;
+};
+
 export const isSharpArrow = (
   element?: ExcalidrawElement,
 ): element is ExcalidrawArrowElement => {

packages/element/tests/linearElementEditor.test.tsx

@@ -1,6 +1,5 @@
 import { pointCenter, pointFrom } from "@excalidraw/math";
 import { act, queryByTestId, queryByText } from "@testing-library/react";
-import React from "react";
 import { vi } from "vitest";
 
 import {
@@ -33,6 +32,8 @@ import { getBoundTextElementPosition, getBoundTextMaxWidth } from "../src";
 import { LinearElementEditor } from "../src";
 import { newArrowElement } from "../src";
 
+import { getTextEditor } from "../../excalidraw/tests/queries/dom";
+
 import type {
   ExcalidrawElement,
   ExcalidrawLinearElement,
@@ -252,7 +253,17 @@ describe("Test Linear Elements", () => {
     expect(h.state.editingLinearElement?.elementId).toEqual(h.elements[0].id);
   });
 
-  it("should enter line editor when using double clicked with ctrl key", () => {
+  it("should enter line editor on ctrl+dblclick (simple arrow)", () => {
+    createTwoPointerLinearElement("arrow");
+    expect(h.state.editingLinearElement?.elementId).toBeUndefined();
+
+    Keyboard.withModifierKeys({ ctrl: true }, () => {
+      mouse.doubleClick();
+    });
+    expect(h.state.editingLinearElement?.elementId).toEqual(h.elements[0].id);
+  });
+
+  it("should enter line editor on ctrl+dblclick (line)", () => {
     createTwoPointerLinearElement("line");
     expect(h.state.editingLinearElement?.elementId).toBeUndefined();
 
@@ -262,6 +273,39 @@ describe("Test Linear Elements", () => {
     expect(h.state.editingLinearElement?.elementId).toEqual(h.elements[0].id);
   });
 
+  it("should enter line editor on dblclick (line)", () => {
+    createTwoPointerLinearElement("line");
+    expect(h.state.editingLinearElement?.elementId).toBeUndefined();
+
+    mouse.doubleClick();
+    expect(h.state.editingLinearElement?.elementId).toEqual(h.elements[0].id);
+  });
+
+  it("should not enter line editor on dblclick (arrow)", async () => {
+    createTwoPointerLinearElement("arrow");
+    expect(h.state.editingLinearElement?.elementId).toBeUndefined();
+
+    mouse.doubleClick();
+    expect(h.state.editingLinearElement).toEqual(null);
+    await getTextEditor(".excalidraw-textEditorContainer > textarea");
+  });
+
+  it("shouldn't create text element on double click in line editor (arrow)", async () => {
+    createTwoPointerLinearElement("arrow");
+    const arrow = h.elements[0] as ExcalidrawLinearElement;
+    enterLineEditingMode(arrow);
+
+    expect(h.state.editingLinearElement?.elementId).toEqual(arrow.id);
+
+    mouse.doubleClick();
+    expect(h.state.editingLinearElement?.elementId).toEqual(arrow.id);
+    expect(h.elements.length).toEqual(1);
+
+    expect(
+      document.querySelector(".excalidraw-textEditorContainer > textarea"),
+    ).toBe(null);
+  });
+
   describe("Inside editor", () => {
     it("should not drag line and add midpoint when dragged irrespective of threshold", () => {
       createTwoPointerLinearElement("line");
@@ -1063,13 +1107,7 @@ describe("Test Linear Elements", () => {
 
       expect(h.elements.length).toBe(1);
       mouse.doubleClickAt(line.x, line.y);
-
-      expect(h.elements.length).toBe(2);
-
-      const text = h.elements[1] as ExcalidrawTextElementWithContainer;
-      expect(text.type).toBe("text");
-      expect(text.containerId).toBeNull();
-      expect(line.boundElements).toBeNull();
+      expect(h.elements.length).toBe(1);
     });
 
     // TODO fix #7029 and rewrite this test

packages/excalidraw/components/App.tsx

@@ -230,6 +230,8 @@ import {
   CaptureUpdateAction,
   type ElementUpdate,
   hitElementBoundingBox,
+  isLineElement,
+  isSimpleArrow,
 } from "@excalidraw/element";
 
 import type { LocalPoint, Radians } from "@excalidraw/math";
@@ -5438,17 +5440,17 @@ class App extends React.Component<AppProps, AppState> {
     );
 
     if (selectedElements.length === 1 && isLinearElement(selectedElements[0])) {
+      const selectedLinearElement: ExcalidrawLinearElement =
+        selectedElements[0];
       if (
-        event[KEYS.CTRL_OR_CMD] &&
-        (!this.state.editingLinearElement ||
-          this.state.editingLinearElement.elementId !==
-            selectedElements[0].id) &&
-        !isElbowArrow(selectedElements[0])
+        ((event[KEYS.CTRL_OR_CMD] && isSimpleArrow(selectedLinearElement)) ||
+          isLineElement(selectedLinearElement)) &&
+        this.state.editingLinearElement?.elementId !== selectedLinearElement.id
       ) {
         this.store.scheduleCapture();
         this.setState({
           editingLinearElement: new LinearElementEditor(
-            selectedElements[0],
+            selectedLinearElement,
             this.scene.getNonDeletedElementsMap(),
           ),
         });
@@ -5515,6 +5517,13 @@ class App extends React.Component<AppProps, AppState> {
 
           return;
         }
+      } else if (
+        this.state.editingLinearElement &&
+        this.state.editingLinearElement.elementId ===
+          selectedLinearElement.id &&
+        isLineElement(selectedLinearElement)
+      ) {
+        return;
       }
     }
 
@@ -5563,36 +5572,43 @@ class App extends React.Component<AppProps, AppState> {
         return;
       }
 
-      const container = this.getTextBindableContainerAtPosition(sceneX, sceneY);
+      // shouldn't edit/create text when inside line editor (often false positive)
 
-      if (container) {
-        if (
-          hasBoundTextElement(container) ||
-          !isTransparent(container.backgroundColor) ||
-          hitElementItself({
-            point: pointFrom(sceneX, sceneY),
-            element: container,
-            elementsMap: this.scene.getNonDeletedElementsMap(),
-            threshold: this.getElementHitThreshold(container),
-          })
-        ) {
-          const midPoint = getContainerCenter(
-            container,
-            this.state,
-            this.scene.getNonDeletedElementsMap(),
-          );
+      if (!this.state.editingLinearElement) {
+        const container = this.getTextBindableContainerAtPosition(
+          sceneX,
+          sceneY,
+        );
+
+        if (container) {
+          if (
+            hasBoundTextElement(container) ||
+            !isTransparent(container.backgroundColor) ||
+            hitElementItself({
+              point: pointFrom(sceneX, sceneY),
+              element: container,
+              elementsMap: this.scene.getNonDeletedElementsMap(),
+              threshold: this.getElementHitThreshold(container),
+            })
+          ) {
+            const midPoint = getContainerCenter(
+              container,
+              this.state,
+              this.scene.getNonDeletedElementsMap(),
+            );
 
-          sceneX = midPoint.x;
-          sceneY = midPoint.y;
+            sceneX = midPoint.x;
+            sceneY = midPoint.y;
+          }
         }
-      }
 
-      this.startTextEditing({
-        sceneX,
-        sceneY,
-        insertAtParentCenter: !event.altKey,
-        container,
-      });
+        this.startTextEditing({
+          sceneX,
+          sceneY,
+          insertAtParentCenter: !event.altKey,
+          container,
+        });
+      }
     }
   };