fix: selectedGroupIds not being stored in history (#3630)

thanks!

src/element/newElement.ts

@@ -307,7 +307,19 @@ export const duplicateElement = <TElement extends Mutable<ExcalidrawElement>>(
   overrides?: Partial<TElement>,
 ): TElement => {
   let copy: TElement = deepCopyElement(element);
-  copy.id = process.env.NODE_ENV === "test" ? `${copy.id}_copy` : randomId();
+  if (process.env.NODE_ENV === "test") {
+    copy.id = `${copy.id}_copy`;
+    // `window.h` may not be defined in some unit tests
+    if (
+      window.h?.app
+        ?.getSceneElementsIncludingDeleted()
+        .find((el) => el.id === copy.id)
+    ) {
+      copy.id += "_copy";
+    }
+  } else {
+    copy.id = randomId();
+  }
   copy.seed = randomInteger();
   copy.groupIds = getNewGroupIdsForDuplication(
     copy.groupIds,

src/history.ts

@@ -21,6 +21,7 @@ interface DehydratedHistoryEntry {
 const clearAppStatePropertiesForHistory = (appState: AppState) => {
   return {
     selectedElementIds: appState.selectedElementIds,
+    selectedGroupIds: appState.selectedGroupIds,
     viewBackgroundColor: appState.viewBackgroundColor,
     editingLinearElement: appState.editingLinearElement,
     editingGroupId: appState.editingGroupId,
@@ -169,7 +170,7 @@ class History {
           continue;
         }
       }
-      if (key === "selectedElementIds") {
+      if (key === "selectedElementIds" || key === "selectedGroupIds") {
         continue;
       }
       if (nextEntry.appState[key] !== lastEntry.appState[key]) {

src/tests/__snapshots__/contextmenu.test.tsx.snap

@@ -115,6 +115,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -127,6 +128,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -304,6 +306,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -316,6 +319,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -352,6 +356,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -411,6 +416,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -611,6 +617,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -623,6 +630,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -659,6 +667,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -718,6 +727,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -892,6 +902,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -904,6 +915,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1053,6 +1065,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -1065,6 +1078,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1099,6 +1113,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1276,6 +1291,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -1288,6 +1304,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1324,6 +1341,7 @@ Object {
         "selectedElementIds": Object {
           "id0_copy": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1534,6 +1552,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -1546,6 +1565,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1582,6 +1602,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1643,6 +1664,9 @@ Object {
           "id1": true,
           "id2": true,
         },
+        "selectedGroupIds": Object {
+          "id3": true,
+        },
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1847,6 +1871,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -1859,6 +1884,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1895,6 +1921,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -1954,6 +1981,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2013,6 +2041,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2072,6 +2101,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2131,6 +2161,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2190,6 +2221,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2249,6 +2281,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2308,6 +2341,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2367,6 +2401,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2567,6 +2602,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -2579,6 +2615,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2615,6 +2652,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2674,6 +2712,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2874,6 +2913,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -2886,6 +2926,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2922,6 +2963,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -2981,6 +3023,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -3185,6 +3228,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -3197,6 +3241,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -3233,6 +3278,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -3294,6 +3340,9 @@ Object {
           "id1": true,
           "id2": true,
         },
+        "selectedGroupIds": Object {
+          "id3": true,
+        },
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -3359,6 +3408,7 @@ Object {
           "id1": true,
           "id2": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -3565,6 +3615,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -3577,6 +3628,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -3613,6 +3665,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -3825,6 +3878,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -3837,6 +3891,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -3873,6 +3928,7 @@ Object {
         "selectedElementIds": Object {
           "id1": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -3935,6 +3991,9 @@ Object {
           "id2": true,
           "id3": true,
         },
+        "selectedGroupIds": Object {
+          "id4": true,
+        },
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [
@@ -4085,6 +4144,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -4212,6 +4272,7 @@ Object {
         "editingLinearElement": null,
         "name": "Untitled-201933152653",
         "selectedElementIds": Object {},
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [],
@@ -4224,6 +4285,7 @@ Object {
         "selectedElementIds": Object {
           "id0": true,
         },
+        "selectedGroupIds": Object {},
         "viewBackgroundColor": "#ffffff",
       },
       "elements": Array [

src/tests/history.test.tsx

@@ -1,7 +1,7 @@
 import React from "react";
-import { render } from "./test-utils";
+import { assertSelectedElements, render } from "./test-utils";
 import ExcalidrawApp from "../excalidraw-app";
-import { UI } from "./helpers/ui";
+import { Keyboard, Pointer, UI } from "./helpers/ui";
 import { API } from "./helpers/api";
 import { getDefaultAppState } from "../appState";
 import { waitFor } from "@testing-library/react";
@@ -10,6 +10,8 @@ import { EXPORT_DATA_TYPES } from "../constants";
 
 const { h } = window;
 
+const mouse = new Pointer("mouse");
+
 describe("history", () => {
   it("initializing scene should end up with single history entry", async () => {
     await render(<ExcalidrawApp />, {
@@ -110,4 +112,78 @@ describe("history", () => {
       expect.objectContaining({ id: "A", isDeleted: true }),
     ]);
   });
+
+  it("undo/redo works properly with groups", async () => {
+    await render(<ExcalidrawApp />);
+    const rect1 = API.createElement({ type: "rectangle", groupIds: ["A"] });
+    const rect2 = API.createElement({ type: "rectangle", groupIds: ["A"] });
+
+    h.elements = [rect1, rect2];
+    mouse.select(rect1);
+    assertSelectedElements([rect1, rect2]);
+    expect(h.state.selectedGroupIds).toEqual({ A: true });
+
+    Keyboard.withModifierKeys({ ctrl: true }, () => {
+      Keyboard.keyPress("d");
+    });
+    expect(h.elements.length).toBe(4);
+    assertSelectedElements([h.elements[2], h.elements[3]]);
+    expect(h.state.selectedGroupIds).not.toEqual(
+      expect.objectContaining({ A: true }),
+    );
+
+    Keyboard.withModifierKeys({ ctrl: true }, () => {
+      Keyboard.keyPress("z");
+    });
+    expect(h.elements.length).toBe(4);
+    expect(h.elements).toEqual([
+      expect.objectContaining({ id: rect1.id, isDeleted: false }),
+      expect.objectContaining({ id: rect2.id, isDeleted: false }),
+      expect.objectContaining({ id: `${rect1.id}_copy`, isDeleted: true }),
+      expect.objectContaining({ id: `${rect2.id}_copy`, isDeleted: true }),
+    ]);
+    expect(h.state.selectedGroupIds).toEqual({ A: true });
+
+    Keyboard.withModifierKeys({ ctrl: true, shift: true }, () => {
+      Keyboard.keyPress("z");
+    });
+    expect(h.elements.length).toBe(4);
+    expect(h.elements).toEqual([
+      expect.objectContaining({ id: rect1.id, isDeleted: false }),
+      expect.objectContaining({ id: rect2.id, isDeleted: false }),
+      expect.objectContaining({ id: `${rect1.id}_copy`, isDeleted: false }),
+      expect.objectContaining({ id: `${rect2.id}_copy`, isDeleted: false }),
+    ]);
+    expect(h.state.selectedGroupIds).not.toEqual(
+      expect.objectContaining({ A: true }),
+    );
+
+    // undo again, and duplicate once more
+    // -------------------------------------------------------------------------
+
+    Keyboard.withModifierKeys({ ctrl: true }, () => {
+      Keyboard.keyPress("z");
+      Keyboard.keyPress("d");
+    });
+    expect(h.elements.length).toBe(6);
+    expect(h.elements).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({ id: rect1.id, isDeleted: false }),
+        expect.objectContaining({ id: rect2.id, isDeleted: false }),
+        expect.objectContaining({ id: `${rect1.id}_copy`, isDeleted: true }),
+        expect.objectContaining({ id: `${rect2.id}_copy`, isDeleted: true }),
+        expect.objectContaining({
+          id: `${rect1.id}_copy_copy`,
+          isDeleted: false,
+        }),
+        expect.objectContaining({
+          id: `${rect2.id}_copy_copy`,
+          isDeleted: false,
+        }),
+      ]),
+    );
+    expect(h.state.selectedGroupIds).not.toEqual(
+      expect.objectContaining({ A: true }),
+    );
+  });
 });

src/tests/regressionTests.test.tsx

@@ -7,21 +7,19 @@ import * as Renderer from "../renderer/renderScene";
 import { setDateTimeForTests } from "../utils";
 import { API } from "./helpers/api";
 import { Keyboard, Pointer, UI } from "./helpers/ui";
-import { fireEvent, render, screen, waitFor } from "./test-utils";
+import {
+  assertSelectedElements,
+  fireEvent,
+  render,
+  screen,
+  waitFor,
+} from "./test-utils";
 import { defaultLang } from "../i18n";
 
 const { h } = window;
 
 const renderScene = jest.spyOn(Renderer, "renderScene");
 
-const assertSelectedElements = (...elements: ExcalidrawElement[]) => {
-  expect(
-    API.getSelectedElements().map((element) => {
-      return element.id;
-    }),
-  ).toEqual(expect.arrayContaining(elements.map((element) => element.id)));
-};
-
 const mouse = new Pointer("mouse");
 const finger1 = new Pointer("touch", 1);
 const finger2 = new Pointer("touch", 2);

src/tests/selection.test.tsx

@@ -5,6 +5,7 @@ import {
   fireEvent,
   mockBoundingClientRect,
   restoreOriginalGetBoundingClientRect,
+  assertSelectedElements,
 } from "./test-utils";
 import ExcalidrawApp from "../excalidraw-app";
 import * as Renderer from "../renderer/renderScene";
@@ -12,7 +13,6 @@ 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")!);
@@ -28,12 +28,6 @@ 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(<ExcalidrawApp />);
@@ -68,7 +62,7 @@ describe("inner box-selection", () => {
       mouse.moveTo(290, 290);
       mouse.up();
 
-      assertSelectedIds([rect2.id, rect3.id]);
+      assertSelectedElements([rect2.id, rect3.id]);
     });
   });
 
@@ -105,7 +99,7 @@ describe("inner box-selection", () => {
       mouse.moveTo(rect2.x + rect2.width + 10, rect2.y + rect2.height + 10);
       mouse.up();
 
-      assertSelectedIds([rect2.id, rect3.id]);
+      assertSelectedElements([rect2.id, rect3.id]);
       expect(h.state.selectedGroupIds).toEqual({ A: true });
     });
   });
@@ -140,10 +134,10 @@ describe("inner box-selection", () => {
     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]);
+      assertSelectedElements([rect2.id, rect3.id]);
       expect(h.state.selectedGroupIds).toEqual({ A: true });
       mouse.moveTo(rect2.x - 10, rect2.y - 10);
-      assertSelectedIds([rect1.id]);
+      assertSelectedElements([rect1.id]);
       expect(h.state.selectedGroupIds).toEqual({});
       mouse.up();
     });

src/tests/test-utils.ts

@@ -13,6 +13,8 @@ import { ImportedDataState } from "../data/types";
 import { STORAGE_KEYS } from "../excalidraw-app/data/localStorage";
 
 import { SceneData } from "../types";
+import { getSelectedElements } from "../scene/selection";
+import { ExcalidrawElement } from "../element/types";
 
 const customQueries = {
   ...queries,
@@ -124,3 +126,22 @@ export const mockBoundingClientRect = () => {
 export const restoreOriginalGetBoundingClientRect = () => {
   global.window.HTMLDivElement.prototype.getBoundingClientRect = originalGetBoundingClientRect;
 };
+
+export const assertSelectedElements = (
+  ...elements: (
+    | (ExcalidrawElement["id"] | ExcalidrawElement)[]
+    | ExcalidrawElement["id"]
+    | ExcalidrawElement
+  )[]
+) => {
+  const { h } = window;
+  const selectedElementIds = getSelectedElements(
+    h.app.getSceneElements(),
+    h.state,
+  ).map((el) => el.id);
+  const ids = elements
+    .flat()
+    .map((item) => (typeof item === "string" ? item : item.id));
+  expect(selectedElementIds.length).toBe(ids.length);
+  expect(selectedElementIds).toEqual(expect.arrayContaining(ids));
+};