diff --git a/src/groups.ts b/src/groups.ts index 8dbe2ba5..dd5512ba 100644 --- a/src/groups.ts +++ b/src/groups.ts @@ -71,6 +71,7 @@ export const selectGroupsForSelectedElements = (function () { selectedElements: readonly NonDeleted[], elements: readonly NonDeleted[], appState: Pick, + prevAppState: InteractiveCanvasAppState, ): SelectGroupsReturnType => { if ( lastReturnValue !== undefined && @@ -134,10 +135,13 @@ export const selectGroupsForSelectedElements = (function () { lastReturnValue = { editingGroupId: appState.editingGroupId, selectedGroupIds, - selectedElementIds: { - ...appState.selectedElementIds, - ...selectedElementIdsInGroups, - }, + selectedElementIds: makeNextSelectedElementIds( + { + ...appState.selectedElementIds, + ...selectedElementIdsInGroups, + }, + prevAppState, + ), }; return lastReturnValue; @@ -181,7 +185,7 @@ export const selectGroupsForSelectedElements = (function () { }; } - return _selectGroups(selectedElements, elements, appState); + return _selectGroups(selectedElements, elements, appState, prevAppState); }; selectGroupsForSelectedElements.clearCache = () => { diff --git a/src/tests/__snapshots__/regressionTests.test.tsx.snap b/src/tests/__snapshots__/regressionTests.test.tsx.snap index e9bbfc7b..832f1652 100644 --- a/src/tests/__snapshots__/regressionTests.test.tsx.snap +++ b/src/tests/__snapshots__/regressionTests.test.tsx.snap @@ -1734,7 +1734,7 @@ exports[`regression tests > Cmd/Ctrl-click exclusively select element under poin exports[`regression tests > Cmd/Ctrl-click exclusively select element under pointer > [end of test] number of elements 1`] = `0`; -exports[`regression tests > Cmd/Ctrl-click exclusively select element under pointer > [end of test] number of renders 1`] = `32`; +exports[`regression tests > Cmd/Ctrl-click exclusively select element under pointer > [end of test] number of renders 1`] = `30`; exports[`regression tests > Drags selected element when hitting only bounding box and keeps element selected > [end of test] appState 1`] = ` { @@ -4609,7 +4609,7 @@ exports[`regression tests > click-drag to select a group > [end of test] history exports[`regression tests > click-drag to select a group > [end of test] number of elements 1`] = `0`; -exports[`regression tests > click-drag to select a group > [end of test] number of renders 1`] = `19`; +exports[`regression tests > click-drag to select a group > [end of test] number of renders 1`] = `18`; exports[`regression tests > deleting last but one element in editing group should unselect the group > [end of test] appState 1`] = ` { @@ -15305,7 +15305,7 @@ exports[`regression tests > single-clicking on a subgroup of a selected group sh exports[`regression tests > single-clicking on a subgroup of a selected group should not alter selection > [end of test] number of elements 1`] = `0`; -exports[`regression tests > single-clicking on a subgroup of a selected group should not alter selection > [end of test] number of renders 1`] = `31`; +exports[`regression tests > single-clicking on a subgroup of a selected group should not alter selection > [end of test] number of renders 1`] = `30`; exports[`regression tests > spacebar + drag scrolls the canvas > [end of test] appState 1`] = ` { diff --git a/src/tests/linearElementEditor.test.tsx b/src/tests/linearElementEditor.test.tsx index bedcb0df..7c14b6ef 100644 --- a/src/tests/linearElementEditor.test.tsx +++ b/src/tests/linearElementEditor.test.tsx @@ -275,7 +275,7 @@ describe("Test Linear Elements", () => { // drag line from midpoint drag(midpoint, [midpoint[0] + delta, midpoint[1] + delta]); expect(renderInteractiveScene).toHaveBeenCalledTimes(14); - expect(renderStaticScene).toHaveBeenCalledTimes(8); + expect(renderStaticScene).toHaveBeenCalledTimes(6); expect(line.points.length).toEqual(3); expect(line.points).toMatchInlineSnapshot(` @@ -418,7 +418,7 @@ describe("Test Linear Elements", () => { ]); expect(renderInteractiveScene).toHaveBeenCalledTimes(21); - expect(renderStaticScene).toHaveBeenCalledTimes(11); + expect(renderStaticScene).toHaveBeenCalledTimes(9); expect(line.points.length).toEqual(5); @@ -521,7 +521,7 @@ describe("Test Linear Elements", () => { deletePoint(points[2]); expect(line.points.length).toEqual(3); expect(renderInteractiveScene).toHaveBeenCalledTimes(21); - expect(renderStaticScene).toHaveBeenCalledTimes(12); + expect(renderStaticScene).toHaveBeenCalledTimes(9); const newMidPoints = LinearElementEditor.getEditorMidPoints( line, @@ -568,7 +568,7 @@ describe("Test Linear Elements", () => { lastSegmentMidpoint[1] + delta, ]); expect(renderInteractiveScene).toHaveBeenCalledTimes(21); - expect(renderStaticScene).toHaveBeenCalledTimes(11); + expect(renderStaticScene).toHaveBeenCalledTimes(9); expect(line.points.length).toEqual(5); expect((h.elements[0] as ExcalidrawLinearElement).points) diff --git a/src/tests/selection.test.tsx b/src/tests/selection.test.tsx index 1afde34a..acae9dd8 100644 --- a/src/tests/selection.test.tsx +++ b/src/tests/selection.test.tsx @@ -308,7 +308,7 @@ describe("select single element on the scene", () => { fireEvent.pointerUp(canvas); expect(renderInteractiveScene).toHaveBeenCalledTimes(9); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderStaticScene).toHaveBeenCalledTimes(8); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(1); expect(h.state.selectedElementIds[h.elements[0].id]).toBeTruthy(); @@ -338,7 +338,7 @@ describe("select single element on the scene", () => { fireEvent.pointerUp(canvas); expect(renderInteractiveScene).toHaveBeenCalledTimes(9); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderStaticScene).toHaveBeenCalledTimes(8); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(1); expect(h.state.selectedElementIds[h.elements[0].id]).toBeTruthy(); @@ -368,7 +368,7 @@ describe("select single element on the scene", () => { fireEvent.pointerUp(canvas); expect(renderInteractiveScene).toHaveBeenCalledTimes(9); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderStaticScene).toHaveBeenCalledTimes(8); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(1); expect(h.state.selectedElementIds[h.elements[0].id]).toBeTruthy(); @@ -411,7 +411,7 @@ describe("select single element on the scene", () => { fireEvent.pointerUp(canvas); expect(renderInteractiveScene).toHaveBeenCalledTimes(9); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderStaticScene).toHaveBeenCalledTimes(8); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(1); expect(h.state.selectedElementIds[h.elements[0].id]).toBeTruthy(); @@ -453,7 +453,7 @@ describe("select single element on the scene", () => { fireEvent.pointerUp(canvas); expect(renderInteractiveScene).toHaveBeenCalledTimes(9); - expect(renderStaticScene).toHaveBeenCalledTimes(9); + expect(renderStaticScene).toHaveBeenCalledTimes(8); expect(h.state.selectionElement).toBeNull(); expect(h.elements.length).toEqual(1); expect(h.state.selectedElementIds[h.elements[0].id]).toBeTruthy(); @@ -477,3 +477,46 @@ describe("tool locking & selection", () => { } }); }); + +describe("selectedElementIds stability", () => { + beforeEach(async () => { + await render(); + }); + + it("box-selection should be stable when not changing selection", () => { + const rectangle = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 10, + height: 10, + }); + + h.elements = [rectangle]; + + const selectedElementIds_1 = h.state.selectedElementIds; + + mouse.downAt(-100, -100); + mouse.moveTo(-50, -50); + mouse.up(); + + expect(h.state.selectedElementIds).toBe(selectedElementIds_1); + + mouse.downAt(-50, -50); + mouse.moveTo(50, 50); + + const selectedElementIds_2 = h.state.selectedElementIds; + + expect(selectedElementIds_2).toEqual({ [rectangle.id]: true }); + + mouse.moveTo(60, 60); + + // box-selecting further without changing selection should keep + // selectedElementIds stable (the same object) + expect(h.state.selectedElementIds).toBe(selectedElementIds_2); + + mouse.up(); + + expect(h.state.selectedElementIds).toBe(selectedElementIds_2); + }); +});