Fix single element bounding box bug (#2008)

Co-authored-by: Michal Srb <xixixao@seznam.cz>
Co-authored-by: dwelle <luzar.david@gmail.com>

src/element/collision.ts

@@ -26,18 +26,15 @@ import { getShapeForElement } from "../renderer/renderElement";
 
 const isElementDraggableFromInside = (
   element: NonDeletedExcalidrawElement,
-  appState: AppState,
 ): boolean => {
   if (element.type === "arrow") {
     return false;
   }
-  const dragFromInside =
-    element.backgroundColor !== "transparent" ||
-    appState.selectedElementIds[element.id];
+  const isDraggableFromInside = element.backgroundColor !== "transparent";
   if (element.type === "line" || element.type === "draw") {
-    return dragFromInside && isPathALoop(element.points);
+    return isDraggableFromInside && isPathALoop(element.points);
   }
-  return dragFromInside;
+  return isDraggableFromInside;
 };
 
 export const hitTest = (
@@ -48,16 +45,51 @@ export const hitTest = (
 ): boolean => {
   // How many pixels off the shape boundary we still consider a hit
   const threshold = 10 / appState.zoom;
+  const point: Point = [x, y];
+
+  if (isElementSelected(appState, element)) {
+    return doesPointHitElementBoundingBox(element, point, threshold);
+  }
+
   const check =
     element.type === "text"
       ? isStrictlyInside
-      : isElementDraggableFromInside(element, appState)
+      : isElementDraggableFromInside(element)
       ? isInsideCheck
       : isNearCheck;
-  const point: Point = [x, y];
   return hitTestPointAgainstElement({ element, point, threshold, check });
 };
 
+const isElementSelected = (
+  appState: AppState,
+  element: NonDeleted<ExcalidrawElement>,
+) => appState.selectedElementIds[element.id];
+
+const doesPointHitElementBoundingBox = (
+  element: NonDeleted<ExcalidrawElement>,
+  [x, y]: Point,
+  threshold: number,
+) => {
+  const [x1, y1, x2, y2] = getElementAbsoluteCoords(element);
+  const elementCenterX = (x1 + x2) / 2;
+  const elementCenterY = (y1 + y2) / 2;
+  // reverse rotate to take element's angle into account.
+  const [rotatedX, rotatedY] = rotate(
+    x,
+    y,
+    elementCenterX,
+    elementCenterY,
+    -element.angle,
+  );
+
+  return (
+    rotatedX > x1 - threshold &&
+    rotatedX < x2 + threshold &&
+    rotatedY > y1 - threshold &&
+    rotatedY < y2 + threshold
+  );
+};
+
 export const bindingBorderTest = (
   element: NonDeleted<ExcalidrawBindableElement>,
   { x, y }: { x: number; y: number },
@@ -235,7 +267,7 @@ const hitTestLinear = (args: HitTestArgs): boolean => {
 
   if (args.check === isInsideCheck) {
     const hit = shape.some((subshape) =>
-      hitTestCurveInside(subshape, relX, relY, threshold),
+      hitTestCurveInside(subshape, relX, relY),
     );
     if (hit) {
       return true;
@@ -656,12 +688,7 @@ const pointInBezierEquation = (
   return false;
 };
 
-const hitTestCurveInside = (
-  drawable: Drawable,
-  x: number,
-  y: number,
-  lineThreshold: number,
-) => {
+const hitTestCurveInside = (drawable: Drawable, x: number, y: number) => {
   const ops = getCurvePathOps(drawable);
   const points: Point[] = [];
   for (const operation of ops) {

src/tests/__snapshots__/regressionTests.test.tsx.snap

@@ -6056,6 +6056,143 @@ exports[`regression tests hotkey x selects draw tool: [end of test] number of el
 
 exports[`regression tests hotkey x selects draw tool: [end of test] number of renders 1`] = `7`;
 
+exports[`regression tests keeps selected element selected when click hits element bounding box but doesn't hit the element: [end of test] appState 1`] = `
+Object {
+  "collaborators": Map {},
+  "currentItemBackgroundColor": "transparent",
+  "currentItemFillStyle": "hachure",
+  "currentItemFontFamily": 1,
+  "currentItemFontSize": 20,
+  "currentItemOpacity": 100,
+  "currentItemRoughness": 1,
+  "currentItemStrokeColor": "#000000",
+  "currentItemStrokeStyle": "solid",
+  "currentItemStrokeWidth": 1,
+  "currentItemTextAlign": "left",
+  "cursorButton": "up",
+  "cursorX": 0,
+  "cursorY": 0,
+  "draggingElement": null,
+  "editingElement": null,
+  "editingGroupId": null,
+  "editingLinearElement": null,
+  "elementLocked": false,
+  "elementType": "selection",
+  "errorMessage": null,
+  "exportBackground": true,
+  "gridSize": null,
+  "height": 768,
+  "isBindingEnabled": true,
+  "isCollaborating": false,
+  "isLibraryOpen": false,
+  "isLoading": false,
+  "isResizing": false,
+  "isRotating": false,
+  "lastPointerDownWith": "mouse",
+  "multiElement": null,
+  "name": "Untitled-201933152653",
+  "offsetLeft": 0,
+  "offsetTop": 0,
+  "openMenu": null,
+  "previousSelectedElementIds": Object {
+    "id0": true,
+  },
+  "resizingElement": null,
+  "scrollX": 0,
+  "scrollY": 0,
+  "scrolledOutside": false,
+  "selectedElementIds": Object {
+    "id0": true,
+    "id1": true,
+  },
+  "selectedGroupIds": Object {},
+  "selectionElement": null,
+  "shouldAddWatermark": false,
+  "shouldCacheIgnoreZoom": false,
+  "showShortcutsDialog": false,
+  "startBoundElement": null,
+  "suggestedBindings": Array [],
+  "username": "",
+  "viewBackgroundColor": "#ffffff",
+  "width": 1024,
+  "zenModeEnabled": false,
+  "zoom": 1,
+}
+`;
+
+exports[`regression tests keeps selected element selected when click hits element bounding box but doesn't hit the element: [end of test] element 0 1`] = `
+Object {
+  "angle": 0,
+  "backgroundColor": "transparent",
+  "boundElementIds": null,
+  "fillStyle": "hachure",
+  "groupIds": Array [],
+  "height": 100,
+  "id": "id0",
+  "isDeleted": false,
+  "opacity": 100,
+  "roughness": 1,
+  "seed": 337897,
+  "strokeColor": "#000000",
+  "strokeStyle": "solid",
+  "strokeWidth": 1,
+  "type": "ellipse",
+  "version": 2,
+  "versionNonce": 1278240551,
+  "width": 100,
+  "x": 0,
+  "y": 0,
+}
+`;
+
+exports[`regression tests keeps selected element selected when click hits element bounding box but doesn't hit the element: [end of test] history 1`] = `
+Object {
+  "recording": false,
+  "redoStack": Array [],
+  "stateHistory": Array [
+    Object {
+      "appState": Object {
+        "editingGroupId": null,
+        "editingLinearElement": null,
+        "name": "Untitled-201933152653",
+        "selectedElementIds": Object {
+          "id0": true,
+        },
+        "viewBackgroundColor": "#ffffff",
+      },
+      "elements": Array [
+        Object {
+          "angle": 0,
+          "backgroundColor": "transparent",
+          "boundElementIds": null,
+          "fillStyle": "hachure",
+          "groupIds": Array [],
+          "height": 100,
+          "id": "id0",
+          "isDeleted": false,
+          "opacity": 100,
+          "roughness": 1,
+          "seed": 337897,
+          "strokeColor": "#000000",
+          "strokeStyle": "solid",
+          "strokeWidth": 1,
+          "type": "ellipse",
+          "version": 2,
+          "versionNonce": 1278240551,
+          "width": 100,
+          "x": 0,
+          "y": 0,
+        },
+      ],
+    },
+  ],
+}
+`;
+
+exports[`regression tests keeps selected element selected when click hits element bounding box but doesn't hit the element: [end of test] number of elements 1`] = `1`;
+
+exports[`regression tests keeps selected element selected when click hits element bounding box but doesn't hit the element: [end of test] number of renders 1`] = `9`;
+
 exports[`regression tests make a group and duplicate it: [end of test] appState 1`] = `
 Object {
   "collaborators": Map {},

src/tests/regressionTests.test.tsx

@@ -1212,4 +1212,14 @@ describe("regression tests", () => {
     expect(h.elements[0].groupIds).toHaveLength(0);
     expect(h.elements[1].groupIds).toHaveLength(0);
   });
+
+  it("keeps selected element selected when click hits element bounding box but doesn't hit the element", () => {
+    clickTool("ellipse");
+    mouse.down(0, 0);
+    mouse.up(100, 100);
+
+    // click on bounding box but not on element
+    mouse.click(0, 0);
+    expect(getSelectedElements().length).toBe(1);
+  });
 });