From 296e3677cf3828c079b98ec612b5e0047392a630 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Forja?= Date: Tue, 11 Aug 2020 11:42:08 +0100 Subject: [PATCH] Fix single element bounding box bug (#2008) Co-authored-by: Michal Srb Co-authored-by: dwelle --- src/element/collision.ts | 57 ++++++-- .../regressionTests.test.tsx.snap | 137 ++++++++++++++++++ src/tests/regressionTests.test.tsx | 10 ++ 3 files changed, 189 insertions(+), 15 deletions(-) diff --git a/src/element/collision.ts b/src/element/collision.ts index 562a9df2..244dd5b1 100644 --- a/src/element/collision.ts +++ b/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, +) => appState.selectedElementIds[element.id]; + +const doesPointHitElementBoundingBox = ( + element: NonDeleted, + [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, { 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) { diff --git a/src/tests/__snapshots__/regressionTests.test.tsx.snap b/src/tests/__snapshots__/regressionTests.test.tsx.snap index 524c515a..0601f0f0 100644 --- a/src/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/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 {}, diff --git a/src/tests/regressionTests.test.tsx b/src/tests/regressionTests.test.tsx index d159fdf6..ec880d02 100644 --- a/src/tests/regressionTests.test.tsx +++ b/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); + }); });