Browse Source

fix: cleanup the condition for dragging elements (#5555)

Aakansha Doshi 2 years ago
parent
commit
fe56975f19
2 changed files with 34 additions and 44 deletions
  1. 6 12
      src/components/App.tsx
  2. 28 32
      src/tests/regressionTests.test.tsx

+ 6 - 12
src/components/App.tsx

@@ -4172,21 +4172,15 @@ class App extends React.Component<AppProps, AppState> {
         (element) => this.isASelectedElement(element),
         (element) => this.isASelectedElement(element),
       );
       );
 
 
+      const isSelectingPointsInLineEditor =
+        this.state.editingLinearElement &&
+        event.shiftKey &&
+        this.state.editingLinearElement.elementId ===
+          pointerDownState.hit.element?.id;
       if (
       if (
         (hasHitASelectedElement ||
         (hasHitASelectedElement ||
           pointerDownState.hit.hasHitCommonBoundingBoxOfSelectedElements) &&
           pointerDownState.hit.hasHitCommonBoundingBoxOfSelectedElements) &&
-        // this allows for box-selecting points when clicking inside the
-        // line's bounding box
-        (!this.state.editingLinearElement || !event.shiftKey) &&
-        // box-selecting without shift when editing line, not clicking on a line
-        (!this.state.editingLinearElement ||
-          this.state.editingLinearElement?.elementId !==
-            pointerDownState.hit.element?.id ||
-          pointerDownState.hit.hasHitElementInside) &&
-        (!this.state.selectedLinearElement ||
-          this.state.selectedLinearElement?.elementId !==
-            pointerDownState.hit.element?.id ||
-          pointerDownState.hit.hasHitElementInside)
+        !isSelectingPointsInLineEditor
       ) {
       ) {
         const selectedElements = getSelectedElements(
         const selectedElements = getSelectedElements(
           this.scene.getNonDeletedElements(),
           this.scene.getNonDeletedElements(),

+ 28 - 32
src/tests/regressionTests.test.tsx

@@ -862,46 +862,42 @@ describe("regression tests", () => {
     expect(API.getSelectedElements().length).toBe(0);
     expect(API.getSelectedElements().length).toBe(0);
   });
   });
 
 
-  it(
-    "drags selected elements from point inside common bounding box that doesn't hit any element " +
-      "and keeps elements selected after dragging",
-    () => {
-      UI.clickTool("rectangle");
-      mouse.down();
-      mouse.up(10, 10);
+  it("drags selected elements from point inside common bounding box that doesn't hit any element and keeps elements selected after dragging", () => {
+    UI.clickTool("rectangle");
+    mouse.down();
+    mouse.up(10, 10);
 
 
-      UI.clickTool("ellipse");
-      mouse.down(100, 100);
-      mouse.up(10, 10);
+    UI.clickTool("ellipse");
+    mouse.down(100, 100);
+    mouse.up(10, 10);
 
 
-      // Selects first element without deselecting the second element
-      // Second element is already selected because creating it was our last action
-      mouse.reset();
-      Keyboard.withModifierKeys({ shift: true }, () => {
-        mouse.click(5, 5);
-      });
+    // Selects first element without deselecting the second element
+    // Second element is already selected because creating it was our last action
+    mouse.reset();
+    Keyboard.withModifierKeys({ shift: true }, () => {
+      mouse.click(5, 5);
+    });
 
 
-      expect(API.getSelectedElements().length).toBe(2);
+    expect(API.getSelectedElements().length).toBe(2);
 
 
-      const { x: firstElementPrevX, y: firstElementPrevY } =
-        API.getSelectedElements()[0];
-      const { x: secondElementPrevX, y: secondElementPrevY } =
-        API.getSelectedElements()[1];
+    const { x: firstElementPrevX, y: firstElementPrevY } =
+      API.getSelectedElements()[0];
+    const { x: secondElementPrevX, y: secondElementPrevY } =
+      API.getSelectedElements()[1];
 
 
-      // drag elements from point on common bounding box that doesn't hit any of the elements
-      mouse.reset();
-      mouse.down(50, 50);
-      mouse.up(25, 25);
+    // drag elements from point on common bounding box that doesn't hit any of the elements
+    mouse.reset();
+    mouse.down(50, 50);
+    mouse.up(25, 25);
 
 
-      expect(API.getSelectedElements()[0].x).toEqual(firstElementPrevX + 25);
-      expect(API.getSelectedElements()[0].y).toEqual(firstElementPrevY + 25);
+    expect(API.getSelectedElements()[0].x).toEqual(firstElementPrevX + 25);
+    expect(API.getSelectedElements()[0].y).toEqual(firstElementPrevY + 25);
 
 
-      expect(API.getSelectedElements()[1].x).toEqual(secondElementPrevX + 25);
-      expect(API.getSelectedElements()[1].y).toEqual(secondElementPrevY + 25);
+    expect(API.getSelectedElements()[1].x).toEqual(secondElementPrevX + 25);
+    expect(API.getSelectedElements()[1].y).toEqual(secondElementPrevY + 25);
 
 
-      expect(API.getSelectedElements().length).toBe(2);
-    },
-  );
+    expect(API.getSelectedElements().length).toBe(2);
+  });
 
 
   it(
   it(
     "given a group of selected elements with an element that is not selected inside the group common bounding box " +
     "given a group of selected elements with an element that is not selected inside the group common bounding box " +