From 6d40039f080326727aa132264a7a7c82e520f04b Mon Sep 17 00:00:00 2001 From: David Luzar Date: Thu, 20 May 2021 22:28:34 +0200 Subject: [PATCH] feat: allow inner-drag-selecting with cmd/ctrl (#3603) * feat: allow inner-drag-selecting with cmd/ctrl * don't use cursor when pressing cmd/ctrl * ensure we reset deselected groups * add tests * add docs * couple fixes around group selection --- src/components/App.tsx | 71 +++++++++++++++----- src/groups.ts | 6 +- src/tests/helpers/api.ts | 5 ++ src/tests/helpers/ui.ts | 36 ++++++++++ src/tests/selection.test.tsx | 127 +++++++++++++++++++++++++++++++++++ src/tests/zindex.test.tsx | 3 +- 6 files changed, 228 insertions(+), 20 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 6b5d9e041..22b8a2b87 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -259,6 +259,7 @@ export type PointerDownState = Readonly<{ hasBeenDuplicated: boolean; hasHitCommonBoundingBoxOfSelectedElements: boolean; }; + withCmdOrCtrl: boolean; drag: { // Might change during the pointer interation hasOccurred: boolean; @@ -2260,11 +2261,13 @@ class App extends React.Component { } else if (isOverScrollBar) { setCursor(this.canvas, CURSOR_TYPE.AUTO); } else if ( - hitElement || - this.isHittingCommonBoundingBoxOfSelectedElements( - scenePointer, - selectedElements, - ) + // if using cmd/ctrl, we're not dragging + !event[KEYS.CTRL_OR_CMD] && + (hitElement || + this.isHittingCommonBoundingBoxOfSelectedElements( + scenePointer, + selectedElements, + )) ) { setCursor(this.canvas, CURSOR_TYPE.MOVE); } else { @@ -2542,6 +2545,7 @@ class App extends React.Component { return { origin, + withCmdOrCtrl: event[KEYS.CTRL_OR_CMD], originInGrid: tupleToCoors( getGridPoint(origin.x, origin.y, this.state.gridSize), ), @@ -2746,6 +2750,9 @@ class App extends React.Component { if (hitElement != null) { // on CMD/CTRL, drill down to hit element regardless of groups etc. if (event[KEYS.CTRL_OR_CMD]) { + if (!this.state.selectedElementIds[hitElement.id]) { + pointerDownState.hit.wasAddedToSelection = true; + } this.setState((prevState) => ({ ...editGroupForSelectedElement(prevState, hitElement), previousSelectedElementIds: this.state.selectedElementIds, @@ -3184,7 +3191,9 @@ class App extends React.Component { this.scene.getElements(), this.state, ); - if (selectedElements.length > 0) { + // prevent dragging even if we're no longer holding cmd/ctrl otherwise + // it would have weird results (stuff jumping all over the screen) + if (selectedElements.length > 0 && !pointerDownState.withCmdOrCtrl) { const [dragX, dragY] = getGridPoint( pointerCoords.x - pointerDownState.drag.offset.x, pointerCoords.y - pointerDownState.drag.offset.y, @@ -3326,11 +3335,25 @@ class App extends React.Component { if (this.state.elementType === "selection") { const elements = this.scene.getElements(); if (!event.shiftKey && isSomeElementSelected(elements, this.state)) { - this.setState({ - selectedElementIds: {}, - selectedGroupIds: {}, - editingGroupId: null, - }); + if (pointerDownState.withCmdOrCtrl && pointerDownState.hit.element) { + this.setState((prevState) => + selectGroupsForSelectedElements( + { + ...prevState, + selectedElementIds: { + [pointerDownState.hit.element!.id]: true, + }, + }, + this.scene.getElements(), + ), + ); + } else { + this.setState({ + selectedElementIds: {}, + selectedGroupIds: {}, + editingGroupId: null, + }); + } } const elementsWithinSelection = getElementsWithinSelection( elements, @@ -3346,6 +3369,14 @@ class App extends React.Component { map[element.id] = true; return map; }, {} as any), + ...(pointerDownState.hit.element + ? { + // if using ctrl/cmd, select the hitElement only if we + // haven't box-selected anything else + [pointerDownState.hit.element + .id]: !elementsWithinSelection.length, + } + : null), }, }, this.scene.getElements(), @@ -3610,12 +3641,18 @@ class App extends React.Component { } else { // remove element from selection while // keeping prev elements selected - this.setState((prevState) => ({ - selectedElementIds: { - ...prevState.selectedElementIds, - [hitElement!.id]: false, - }, - })); + this.setState((prevState) => + selectGroupsForSelectedElements( + { + ...prevState, + selectedElementIds: { + ...prevState.selectedElementIds, + [hitElement!.id]: false, + }, + }, + this.scene.getElements(), + ), + ); } } else { // add element to selection while diff --git a/src/groups.ts b/src/groups.ts index 205041704..f374e816b 100644 --- a/src/groups.ts +++ b/src/groups.ts @@ -67,10 +67,14 @@ export const selectGroupsForSelectedElements = ( appState: AppState, elements: readonly NonDeleted[], ): AppState => { - let nextAppState = { ...appState }; + let nextAppState: AppState = { ...appState, selectedGroupIds: {} }; const selectedElements = getSelectedElements(elements, appState); + if (!selectedElements.length) { + return { ...nextAppState, editingGroupId: null }; + } + for (const selectedElement of selectedElements) { let groupIds = selectedElement.groupIds; if (appState.editingGroupId) { diff --git a/src/tests/helpers/api.ts b/src/tests/helpers/api.ts index 57b1238b7..ee2ac252b 100644 --- a/src/tests/helpers/api.ts +++ b/src/tests/helpers/api.ts @@ -57,6 +57,7 @@ export class API { width = 100, height = width, isDeleted = false, + groupIds = [], ...rest }: { type: T; @@ -66,6 +67,7 @@ export class API { width?: number; id?: string; isDeleted?: boolean; + groupIds?: string[]; // generic element props strokeColor?: ExcalidrawGenericElement["strokeColor"]; backgroundColor?: ExcalidrawGenericElement["backgroundColor"]; @@ -152,6 +154,9 @@ export class API { if (isDeleted) { element.isDeleted = isDeleted; } + if (groupIds) { + element.groupIds = groupIds; + } return element as any; }; diff --git a/src/tests/helpers/ui.ts b/src/tests/helpers/ui.ts index 26dbc24ab..8e70a8c03 100644 --- a/src/tests/helpers/ui.ts +++ b/src/tests/helpers/ui.ts @@ -122,6 +122,9 @@ export class Pointer { }; } + // incremental (moving by deltas) + // --------------------------------------------------------------------------- + move(dx: number, dy: number) { if (dx !== 0 || dy !== 0) { this.clientX += dx; @@ -150,6 +153,39 @@ export class Pointer { fireEvent.doubleClick(GlobalTestState.canvas, this.getEvent()); } + // absolute coords + // --------------------------------------------------------------------------- + + moveTo(x: number, y: number) { + this.clientX = x; + this.clientY = y; + fireEvent.pointerMove(GlobalTestState.canvas, this.getEvent()); + } + + downAt(x = this.clientX, y = this.clientY) { + this.clientX = x; + this.clientY = y; + fireEvent.pointerDown(GlobalTestState.canvas, this.getEvent()); + } + + upAt(x = this.clientX, y = this.clientY) { + this.clientX = x; + this.clientY = y; + fireEvent.pointerUp(GlobalTestState.canvas, this.getEvent()); + } + + clickAt(x: number, y: number) { + this.downAt(x, y); + this.upAt(); + } + + doubleClickAt(x: number, y: number) { + this.moveTo(x, y); + fireEvent.doubleClick(GlobalTestState.canvas, this.getEvent()); + } + + // --------------------------------------------------------------------------- + select( /** if multiple elements supplied, they're shift-selected */ elements: ExcalidrawElement | ExcalidrawElement[], diff --git a/src/tests/selection.test.tsx b/src/tests/selection.test.tsx index 6fa1bddae..3d92cfd00 100644 --- a/src/tests/selection.test.tsx +++ b/src/tests/selection.test.tsx @@ -10,6 +10,9 @@ import ExcalidrawApp from "../excalidraw-app"; import * as Renderer from "../renderer/renderScene"; import { KEYS } from "../keys"; import { reseed } from "../random"; +import { API } from "./helpers/api"; +import { Keyboard, Pointer } from "./helpers/ui"; +import { getSelectedElements } from "../scene"; // Unmount ReactDOM from root ReactDOM.unmountComponentAtNode(document.getElementById("root")!); @@ -23,6 +26,130 @@ beforeEach(() => { const { h } = window; +const mouse = new Pointer("mouse"); + +const assertSelectedIds = (ids: string[]) => { + expect( + getSelectedElements(h.app.getSceneElements(), h.state).map((el) => el.id), + ).toEqual(ids); +}; + +describe("inner box-selection", () => { + beforeEach(async () => { + await render(); + }); + it("selecting elements visually nested inside another", async () => { + const rect1 = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 300, + height: 300, + backgroundColor: "red", + fillStyle: "solid", + }); + const rect2 = API.createElement({ + type: "rectangle", + x: 50, + y: 50, + width: 50, + height: 50, + }); + const rect3 = API.createElement({ + type: "rectangle", + x: 150, + y: 150, + width: 50, + height: 50, + }); + h.elements = [rect1, rect2, rect3]; + Keyboard.withModifierKeys({ ctrl: true }, () => { + mouse.downAt(40, 40); + mouse.moveTo(290, 290); + mouse.up(); + + assertSelectedIds([rect2.id, rect3.id]); + }); + }); + + it("selecting grouped elements visually nested inside another", async () => { + const rect1 = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 300, + height: 300, + backgroundColor: "red", + fillStyle: "solid", + }); + const rect2 = API.createElement({ + type: "rectangle", + x: 50, + y: 50, + width: 50, + height: 50, + groupIds: ["A"], + }); + const rect3 = API.createElement({ + type: "rectangle", + x: 150, + y: 150, + width: 50, + height: 50, + groupIds: ["A"], + }); + h.elements = [rect1, rect2, rect3]; + + Keyboard.withModifierKeys({ ctrl: true }, () => { + mouse.downAt(40, 40); + mouse.moveTo(rect2.x + rect2.width + 10, rect2.y + rect2.height + 10); + mouse.up(); + + assertSelectedIds([rect2.id, rect3.id]); + expect(h.state.selectedGroupIds).toEqual({ A: true }); + }); + }); + + it("selecting & deselecting grouped elements visually nested inside another", async () => { + const rect1 = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 300, + height: 300, + backgroundColor: "red", + fillStyle: "solid", + }); + const rect2 = API.createElement({ + type: "rectangle", + x: 50, + y: 50, + width: 50, + height: 50, + groupIds: ["A"], + }); + const rect3 = API.createElement({ + type: "rectangle", + x: 150, + y: 150, + width: 50, + height: 50, + groupIds: ["A"], + }); + h.elements = [rect1, rect2, rect3]; + Keyboard.withModifierKeys({ ctrl: true }, () => { + mouse.downAt(rect2.x - 20, rect2.x - 20); + mouse.moveTo(rect2.x + rect2.width + 10, rect2.y + rect2.height + 10); + assertSelectedIds([rect2.id, rect3.id]); + expect(h.state.selectedGroupIds).toEqual({ A: true }); + mouse.moveTo(rect2.x - 10, rect2.y - 10); + assertSelectedIds([rect1.id]); + expect(h.state.selectedGroupIds).toEqual({}); + mouse.up(); + }); + }); +}); + describe("selection element", () => { it("create selection element on pointer down", async () => { const { getByToolName, container } = await render(); diff --git a/src/tests/zindex.test.tsx b/src/tests/zindex.test.tsx index 39d751de1..baf60f49d 100644 --- a/src/tests/zindex.test.tsx +++ b/src/tests/zindex.test.tsx @@ -56,9 +56,8 @@ const populateElements = ( y, width, height, + groupIds, }); - // @ts-ignore - element.groupIds = groupIds; if (isSelected) { selectedElementIds[element.id] = true; }