From 4acdc47ef02226cd45fb941d53a7fb482d048f5c Mon Sep 17 00:00:00 2001 From: Jaiwanth Date: Tue, 5 Jan 2021 18:34:06 +0530 Subject: [PATCH] improvement: do not reset to selection for draw tool (#2721) Co-authored-by: Lipis Co-authored-by: dwelle --- src/actions/actionFinalize.tsx | 14 ++++++++---- src/components/App.tsx | 6 ++--- src/packages/excalidraw/CHANGELOG.md | 1 + .../regressionTests.test.tsx.snap | 18 +++++++-------- src/tests/regressionTests.test.tsx | 22 +++++++++++-------- 5 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/actions/actionFinalize.tsx b/src/actions/actionFinalize.tsx index 67f4da88..2842615f 100644 --- a/src/actions/actionFinalize.tsx +++ b/src/actions/actionFinalize.tsx @@ -118,11 +118,14 @@ export const actionFinalize = register({ ); } - if (!appState.elementLocked) { + if (!appState.elementLocked && appState.elementType !== "draw") { appState.selectedElementIds[multiPointElement.id] = true; } } - if (!appState.elementLocked || !multiPointElement) { + if ( + (!appState.elementLocked && appState.elementType !== "draw") || + !multiPointElement + ) { resetCursor(); } return { @@ -130,7 +133,8 @@ export const actionFinalize = register({ appState: { ...appState, elementType: - appState.elementLocked && multiPointElement + (appState.elementLocked || appState.elementType === "draw") && + multiPointElement ? appState.elementType : "selection", draggingElement: null, @@ -139,7 +143,9 @@ export const actionFinalize = register({ startBoundElement: null, suggestedBindings: [], selectedElementIds: - multiPointElement && !appState.elementLocked + multiPointElement && + !appState.elementLocked && + appState.elementType !== "draw" ? { ...appState.selectedElementIds, [multiPointElement.id]: true, diff --git a/src/components/App.tsx b/src/components/App.tsx index 30c069f4..b6132d24 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -3141,7 +3141,7 @@ class App extends React.Component { ); } this.setState({ suggestedBindings: [], startBoundElement: null }); - if (!elementLocked) { + if (!elementLocked && elementType !== "draw") { resetCursor(); this.setState((prevState) => ({ draggingElement: null, @@ -3288,7 +3288,7 @@ class App extends React.Component { return; } - if (!elementLocked && draggingElement) { + if (!elementLocked && elementType !== "draw" && draggingElement) { this.setState((prevState) => ({ selectedElementIds: { ...prevState.selectedElementIds, @@ -3312,7 +3312,7 @@ class App extends React.Component { ); } - if (!elementLocked) { + if (!elementLocked && elementType !== "draw") { resetCursor(); this.setState({ draggingElement: null, diff --git a/src/packages/excalidraw/CHANGELOG.md b/src/packages/excalidraw/CHANGELOG.md index 3a99b820..42173549 100644 --- a/src/packages/excalidraw/CHANGELOG.md +++ b/src/packages/excalidraw/CHANGELOG.md @@ -43,6 +43,7 @@ Please add the latest change on the top under the correct section. ### Improvements +- Do not reset to selection when using the draw tool [#2721](https://github.com/excalidraw/excalidraw/pull/2721) - Display proper tooltip for 2-point lines during resize, and normalize modifier key labels in hints [#2655](https://github.com/excalidraw/excalidraw/pull/2655) - Improve error message around importing images [#2619](https://github.com/excalidraw/excalidraw/pull/2619) - Add tooltip with icon for embedding scenes [#2532](https://github.com/excalidraw/excalidraw/pull/2532) diff --git a/src/tests/__snapshots__/regressionTests.test.tsx.snap b/src/tests/__snapshots__/regressionTests.test.tsx.snap index e06b481b..fe8a549d 100644 --- a/src/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/src/tests/__snapshots__/regressionTests.test.tsx.snap @@ -6364,7 +6364,7 @@ Object { "editingGroupId": null, "editingLinearElement": null, "elementLocked": false, - "elementType": "selection", + "elementType": "draw", "errorMessage": null, "exportBackground": true, "exportEmbedScene": false, @@ -6392,7 +6392,7 @@ Object { "scrollY": 0, "scrolledOutside": false, "selectedElementIds": Object { - "id7": true, + "id7": false, }, "selectedGroupIds": Object {}, "selectionElement": null, @@ -8082,7 +8082,7 @@ Object { "editingLinearElement": null, "name": "Untitled-201933152653", "selectedElementIds": Object { - "id7": true, + "id7": false, }, "viewBackgroundColor": "#ffffff", }, @@ -10433,7 +10433,7 @@ Object { "editingGroupId": null, "editingLinearElement": null, "elementLocked": false, - "elementType": "selection", + "elementType": "draw", "errorMessage": null, "exportBackground": true, "exportEmbedScene": false, @@ -10461,7 +10461,7 @@ Object { "scrollY": 0, "scrolledOutside": false, "selectedElementIds": Object { - "id0": true, + "id0": false, }, "selectedGroupIds": Object {}, "selectionElement": null, @@ -10546,7 +10546,7 @@ Object { "editingLinearElement": null, "name": "Untitled-201933152653", "selectedElementIds": Object { - "id0": true, + "id0": false, }, "viewBackgroundColor": "#ffffff", }, @@ -11489,7 +11489,7 @@ Object { "editingGroupId": null, "editingLinearElement": null, "elementLocked": false, - "elementType": "selection", + "elementType": "draw", "errorMessage": null, "exportBackground": true, "exportEmbedScene": false, @@ -11517,7 +11517,7 @@ Object { "scrollY": 0, "scrolledOutside": false, "selectedElementIds": Object { - "id0": true, + "id0": false, }, "selectedGroupIds": Object {}, "selectionElement": null, @@ -11602,7 +11602,7 @@ Object { "editingLinearElement": null, "name": "Untitled-201933152653", "selectedElementIds": Object { - "id0": true, + "id0": false, }, "viewBackgroundColor": "#ffffff", }, diff --git a/src/tests/regressionTests.test.tsx b/src/tests/regressionTests.test.tsx index efb5bc8e..c6ef7842 100644 --- a/src/tests/regressionTests.test.tsx +++ b/src/tests/regressionTests.test.tsx @@ -150,22 +150,26 @@ describe("regression tests", () => { expect(API.getSelectedElement().id).not.toEqual(prevSelectedId); }); - for (const [keys, shape] of [ - [`2${KEYS.R}`, "rectangle"], - [`3${KEYS.D}`, "diamond"], - [`4${KEYS.E}`, "ellipse"], - [`5${KEYS.A}`, "arrow"], - [`6${KEYS.L}`, "line"], - [`7${KEYS.X}`, "draw"], - ] as [string, ExcalidrawElement["type"]][]) { + for (const [keys, shape, shouldSelect] of [ + [`2${KEYS.R}`, "rectangle", true], + [`3${KEYS.D}`, "diamond", true], + [`4${KEYS.E}`, "ellipse", true], + [`5${KEYS.A}`, "arrow", true], + [`6${KEYS.L}`, "line", true], + [`7${KEYS.X}`, "draw", false], + ] as [string, ExcalidrawElement["type"], boolean][]) { for (const key of keys) { it(`key ${key} selects ${shape} tool`, () => { Keyboard.keyPress(key); + expect(h.state.elementType).toBe(shape); + mouse.down(10, 10); mouse.up(10, 10); - expect(API.getSelectedElement().type).toBe(shape); + if (shouldSelect) { + expect(API.getSelectedElement().type).toBe(shape); + } }); } }