diff --git a/src/components/App.tsx b/src/components/App.tsx index e312cf31..5599b951 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -2462,8 +2462,7 @@ class App extends React.Component { // otherwise, it will trigger selection based on current // state of the box if (!this.state.selectedElementIds[hitElement.id]) { - // if we are currently editing a group, treat all selections outside of the group - // as exiting editing mode. + // if we are currently editing a group, exiting editing mode and deselect the group. if ( this.state.editingGroupId && !isElementInGroup(hitElement, this.state.editingGroupId) @@ -2473,7 +2472,6 @@ class App extends React.Component { selectedGroupIds: {}, editingGroupId: null, }); - return true; } // Add hit element to selection. At this point if we're not holding diff --git a/src/tests/__snapshots__/regressionTests.test.tsx.snap b/src/tests/__snapshots__/regressionTests.test.tsx.snap index f9cee43f..0f5db1c9 100644 --- a/src/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/src/tests/__snapshots__/regressionTests.test.tsx.snap @@ -18537,7 +18537,9 @@ Object { "offsetLeft": 0, "offsetTop": 0, "openMenu": null, - "previousSelectedElementIds": Object {}, + "previousSelectedElementIds": Object { + "id0": true, + }, "resizingElement": null, "scrollX": 0, "scrollY": 0, @@ -18575,7 +18577,7 @@ Object { "groupIds": Array [ "id3", ], - "height": 10, + "height": 50, "id": "id1", "isDeleted": false, "opacity": 100, @@ -18588,9 +18590,9 @@ Object { "type": "rectangle", "version": 3, "versionNonce": 1116226695, - "width": 10, - "x": 30, - "y": 10, + "width": 50, + "x": 100, + "y": 100, } `; @@ -18604,7 +18606,7 @@ Object { "id5", "id3", ], - "height": 10, + "height": 50, "id": "id0", "isDeleted": false, "opacity": 100, @@ -18617,9 +18619,9 @@ Object { "type": "rectangle", "version": 4, "versionNonce": 400692809, - "width": 10, - "x": 10, - "y": 10, + "width": 50, + "x": 0, + "y": 0, } `; @@ -18633,7 +18635,7 @@ Object { "id5", "id3", ], - "height": 10, + "height": 50, "id": "id2", "isDeleted": false, "opacity": 100, @@ -18646,9 +18648,9 @@ Object { "type": "rectangle", "version": 4, "versionNonce": 1604849351, - "width": 10, - "x": 50, - "y": 10, + "width": 50, + "x": 200, + "y": 200, } `; @@ -18684,7 +18686,7 @@ Object { "boundElementIds": null, "fillStyle": "hachure", "groupIds": Array [], - "height": 10, + "height": 50, "id": "id0", "isDeleted": false, "opacity": 100, @@ -18697,9 +18699,9 @@ Object { "type": "rectangle", "version": 2, "versionNonce": 1278240551, - "width": 10, - "x": 10, - "y": 10, + "width": 50, + "x": 0, + "y": 0, }, ], }, @@ -18720,7 +18722,7 @@ Object { "boundElementIds": null, "fillStyle": "hachure", "groupIds": Array [], - "height": 10, + "height": 50, "id": "id0", "isDeleted": false, "opacity": 100, @@ -18733,9 +18735,9 @@ Object { "type": "rectangle", "version": 2, "versionNonce": 1278240551, - "width": 10, - "x": 10, - "y": 10, + "width": 50, + "x": 0, + "y": 0, }, Object { "angle": 0, @@ -18743,7 +18745,7 @@ Object { "boundElementIds": null, "fillStyle": "hachure", "groupIds": Array [], - "height": 10, + "height": 50, "id": "id1", "isDeleted": false, "opacity": 100, @@ -18756,9 +18758,9 @@ Object { "type": "rectangle", "version": 2, "versionNonce": 453191, - "width": 10, - "x": 30, - "y": 10, + "width": 50, + "x": 100, + "y": 100, }, ], }, @@ -18779,7 +18781,7 @@ Object { "boundElementIds": null, "fillStyle": "hachure", "groupIds": Array [], - "height": 10, + "height": 50, "id": "id0", "isDeleted": false, "opacity": 100, @@ -18792,9 +18794,9 @@ Object { "type": "rectangle", "version": 2, "versionNonce": 1278240551, - "width": 10, - "x": 10, - "y": 10, + "width": 50, + "x": 0, + "y": 0, }, Object { "angle": 0, @@ -18802,7 +18804,7 @@ Object { "boundElementIds": null, "fillStyle": "hachure", "groupIds": Array [], - "height": 10, + "height": 50, "id": "id1", "isDeleted": false, "opacity": 100, @@ -18815,9 +18817,9 @@ Object { "type": "rectangle", "version": 2, "versionNonce": 453191, - "width": 10, - "x": 30, - "y": 10, + "width": 50, + "x": 100, + "y": 100, }, Object { "angle": 0, @@ -18825,7 +18827,7 @@ Object { "boundElementIds": null, "fillStyle": "hachure", "groupIds": Array [], - "height": 10, + "height": 50, "id": "id2", "isDeleted": false, "opacity": 100, @@ -18838,9 +18840,9 @@ Object { "type": "rectangle", "version": 2, "versionNonce": 2019559783, - "width": 10, - "x": 50, - "y": 10, + "width": 50, + "x": 200, + "y": 200, }, ], }, @@ -18865,7 +18867,7 @@ Object { "groupIds": Array [ "id3", ], - "height": 10, + "height": 50, "id": "id0", "isDeleted": false, "opacity": 100, @@ -18878,9 +18880,9 @@ Object { "type": "rectangle", "version": 3, "versionNonce": 1150084233, - "width": 10, - "x": 10, - "y": 10, + "width": 50, + "x": 0, + "y": 0, }, Object { "angle": 0, @@ -18890,7 +18892,7 @@ Object { "groupIds": Array [ "id3", ], - "height": 10, + "height": 50, "id": "id1", "isDeleted": false, "opacity": 100, @@ -18903,9 +18905,9 @@ Object { "type": "rectangle", "version": 3, "versionNonce": 1116226695, - "width": 10, - "x": 30, - "y": 10, + "width": 50, + "x": 100, + "y": 100, }, Object { "angle": 0, @@ -18915,7 +18917,7 @@ Object { "groupIds": Array [ "id3", ], - "height": 10, + "height": 50, "id": "id2", "isDeleted": false, "opacity": 100, @@ -18928,9 +18930,9 @@ Object { "type": "rectangle", "version": 3, "versionNonce": 1014066025, - "width": 10, - "x": 50, - "y": 10, + "width": 50, + "x": 200, + "y": 200, }, ], }, @@ -18955,7 +18957,7 @@ Object { "groupIds": Array [ "id3", ], - "height": 10, + "height": 50, "id": "id0", "isDeleted": false, "opacity": 100, @@ -18968,9 +18970,9 @@ Object { "type": "rectangle", "version": 3, "versionNonce": 1150084233, - "width": 10, - "x": 10, - "y": 10, + "width": 50, + "x": 0, + "y": 0, }, Object { "angle": 0, @@ -18980,7 +18982,7 @@ Object { "groupIds": Array [ "id3", ], - "height": 10, + "height": 50, "id": "id1", "isDeleted": false, "opacity": 100, @@ -18993,9 +18995,9 @@ Object { "type": "rectangle", "version": 3, "versionNonce": 1116226695, - "width": 10, - "x": 30, - "y": 10, + "width": 50, + "x": 100, + "y": 100, }, Object { "angle": 0, @@ -19005,7 +19007,7 @@ Object { "groupIds": Array [ "id3", ], - "height": 10, + "height": 50, "id": "id2", "isDeleted": false, "opacity": 100, @@ -19018,9 +19020,9 @@ Object { "type": "rectangle", "version": 3, "versionNonce": 1014066025, - "width": 10, - "x": 50, - "y": 10, + "width": 50, + "x": 200, + "y": 200, }, ], }, @@ -19045,7 +19047,7 @@ Object { "groupIds": Array [ "id3", ], - "height": 10, + "height": 50, "id": "id1", "isDeleted": false, "opacity": 100, @@ -19058,9 +19060,9 @@ Object { "type": "rectangle", "version": 3, "versionNonce": 1116226695, - "width": 10, - "x": 30, - "y": 10, + "width": 50, + "x": 100, + "y": 100, }, Object { "angle": 0, @@ -19071,7 +19073,7 @@ Object { "id5", "id3", ], - "height": 10, + "height": 50, "id": "id0", "isDeleted": false, "opacity": 100, @@ -19084,9 +19086,9 @@ Object { "type": "rectangle", "version": 4, "versionNonce": 400692809, - "width": 10, - "x": 10, - "y": 10, + "width": 50, + "x": 0, + "y": 0, }, Object { "angle": 0, @@ -19097,7 +19099,7 @@ Object { "id5", "id3", ], - "height": 10, + "height": 50, "id": "id2", "isDeleted": false, "opacity": 100, @@ -19110,9 +19112,9 @@ Object { "type": "rectangle", "version": 4, "versionNonce": 1604849351, - "width": 10, - "x": 50, - "y": 10, + "width": 50, + "x": 200, + "y": 200, }, ], }, @@ -19138,7 +19140,7 @@ Object { "groupIds": Array [ "id3", ], - "height": 10, + "height": 50, "id": "id1", "isDeleted": false, "opacity": 100, @@ -19151,9 +19153,9 @@ Object { "type": "rectangle", "version": 3, "versionNonce": 1116226695, - "width": 10, - "x": 30, - "y": 10, + "width": 50, + "x": 100, + "y": 100, }, Object { "angle": 0, @@ -19164,7 +19166,7 @@ Object { "id5", "id3", ], - "height": 10, + "height": 50, "id": "id0", "isDeleted": false, "opacity": 100, @@ -19177,9 +19179,9 @@ Object { "type": "rectangle", "version": 4, "versionNonce": 400692809, - "width": 10, - "x": 10, - "y": 10, + "width": 50, + "x": 0, + "y": 0, }, Object { "angle": 0, @@ -19190,7 +19192,7 @@ Object { "id5", "id3", ], - "height": 10, + "height": 50, "id": "id2", "isDeleted": false, "opacity": 100, @@ -19203,9 +19205,9 @@ Object { "type": "rectangle", "version": 4, "versionNonce": 1604849351, - "width": 10, - "x": 50, - "y": 10, + "width": 50, + "x": 200, + "y": 200, }, ], }, @@ -19215,7 +19217,7 @@ Object { exports[`regression tests supports nested groups: [end of test] number of elements 1`] = `3`; -exports[`regression tests supports nested groups: [end of test] number of renders 1`] = `29`; +exports[`regression tests supports nested groups: [end of test] number of renders 1`] = `28`; exports[`regression tests switches from group of selected elements to another element on pointer down: [end of test] appState 1`] = ` Object { diff --git a/src/tests/helpers/ui.ts b/src/tests/helpers/ui.ts index 1ae57204..5f8f23c7 100644 --- a/src/tests/helpers/ui.ts +++ b/src/tests/helpers/ui.ts @@ -169,6 +169,12 @@ export class Pointer { this.click(element.x, element.y); this.reset(); } + + doubleClickOn(element: ExcalidrawElement) { + this.reset(); + this.doubleClick(element.x, element.y); + this.reset(); + } } const mouse = new Pointer("mouse"); @@ -178,32 +184,72 @@ export class UI { fireEvent.click(GlobalTestState.renderResult.getByToolName(toolName)); }; + /** + * Creates an Excalidraw element, and returns a proxy that wraps it so that + * accessing props will return the latest ones from the object existing in + * the app's elements array. This is because across the app lifecycle we tend + * to recreate element objects and the returned reference will become stale. + * + * If you need to get the actual element, not the proxy, call `get()` method + * on the proxy object. + */ static createElement( type: T, { - x = 0, - y = 0, + position = 0, + x = position, + y = position, size = 10, width = size, height = width, }: { + position?: number; x?: number; y?: number; size?: number; width?: number; height?: number; } = {}, - ): T extends "arrow" | "line" | "draw" + ): (T extends "arrow" | "line" | "draw" ? ExcalidrawLinearElement : T extends "text" ? ExcalidrawTextElement - : ExcalidrawElement { + : ExcalidrawElement) & { + /** Returns the actual, current element from the elements array, instead + of the proxy */ + get(): T extends "arrow" | "line" | "draw" + ? ExcalidrawLinearElement + : T extends "text" + ? ExcalidrawTextElement + : ExcalidrawElement; + } { UI.clickTool(type); mouse.reset(); mouse.down(x, y); mouse.reset(); mouse.up(x + (width ?? height ?? size), y + (height ?? size)); - return h.elements[h.elements.length - 1] as any; + + const origElement = h.elements[h.elements.length - 1] as any; + + return new Proxy( + {}, + { + get(target, prop) { + const currentElement = h.elements.find( + (element) => element.id === origElement.id, + ) as any; + if (prop === "get") { + if (currentElement.hasOwnProperty("get")) { + throw new Error( + "trying to get `get` test property, but ExcalidrawElement seems to define its own", + ); + } + return () => currentElement; + } + return currentElement[prop]; + }, + }, + ) as any; } static group(elements: ExcalidrawElement[]) { diff --git a/src/tests/move.test.tsx b/src/tests/move.test.tsx index 9d58b087..e97ace31 100644 --- a/src/tests/move.test.tsx +++ b/src/tests/move.test.tsx @@ -69,9 +69,9 @@ describe("move element", () => { // bind line to two rectangles bindOrUnbindLinearElement( - line as NonDeleted, - rectA as ExcalidrawRectangleElement, - rectB as ExcalidrawRectangleElement, + line.get() as NonDeleted, + rectA.get() as ExcalidrawRectangleElement, + rectB.get() as ExcalidrawRectangleElement, ); // select the second rectangles diff --git a/src/tests/regressionTests.test.tsx b/src/tests/regressionTests.test.tsx index 1a2aaa6a..eb3edbdf 100644 --- a/src/tests/regressionTests.test.tsx +++ b/src/tests/regressionTests.test.tsx @@ -558,64 +558,46 @@ describe("regression tests", () => { }); it("supports nested groups", () => { - const positions: number[][] = []; - - UI.clickTool("rectangle"); - mouse.down(10, 10); - mouse.up(10, 10); - positions.push(mouse.getPosition()); - - UI.clickTool("rectangle"); - mouse.down(10, -10); - mouse.up(10, 10); - positions.push(mouse.getPosition()); - - UI.clickTool("rectangle"); - mouse.down(10, -10); - mouse.up(10, 10); - positions.push(mouse.getPosition()); + const rectA = UI.createElement("rectangle", { position: 0, size: 50 }); + const rectB = UI.createElement("rectangle", { position: 100, size: 50 }); + const rectC = UI.createElement("rectangle", { position: 200, size: 50 }); Keyboard.withModifierKeys({ ctrl: true }, () => { Keyboard.keyPress(KEYS.A); Keyboard.codePress(CODES.G); }); - mouse.doubleClick(); + mouse.doubleClickOn(rectC); Keyboard.withModifierKeys({ shift: true }, () => { - mouse.restorePosition(...positions[0]); - mouse.click(); + mouse.clickOn(rectA); }); Keyboard.withModifierKeys({ ctrl: true }, () => { Keyboard.codePress(CODES.G); }); - const groupIds = h.elements[2].groupIds; - expect(groupIds.length).toBe(2); - expect(h.elements[1].groupIds).toEqual(groupIds); - expect(h.elements[0].groupIds).toEqual(groupIds.slice(1)); + expect(rectC.groupIds.length).toBe(2); + expect(rectA.groupIds).toEqual(rectC.groupIds); + expect(rectB.groupIds).toEqual(rectA.groupIds.slice(1)); - mouse.click(50, 50); + mouse.click(0, 100); expect(API.getSelectedElements().length).toBe(0); - mouse.restorePosition(...positions[0]); - mouse.click(); + + mouse.clickOn(rectA); expect(API.getSelectedElements().length).toBe(3); expect(h.state.editingGroupId).toBe(null); - mouse.doubleClick(); + mouse.doubleClickOn(rectA); expect(API.getSelectedElements().length).toBe(2); - expect(h.state.editingGroupId).toBe(groupIds[1]); + expect(h.state.editingGroupId).toBe(rectA.groupIds[1]); - mouse.doubleClick(); + mouse.doubleClickOn(rectA); expect(API.getSelectedElements().length).toBe(1); - expect(h.state.editingGroupId).toBe(groupIds[0]); + expect(h.state.editingGroupId).toBe(rectA.groupIds[0]); - // click out of the group - mouse.restorePosition(...positions[1]); - mouse.click(); - expect(API.getSelectedElements().length).toBe(0); - mouse.click(); + // click outside current (sub)group + mouse.clickOn(rectB); expect(API.getSelectedElements().length).toBe(3); - mouse.doubleClick(); + mouse.doubleClickOn(rectB); expect(API.getSelectedElements().length).toBe(1); });