Fix layer group expansion state after deletion (#2146)

Deleting a layer changes the starting indices of later groups, so their
stored expansion state no longer matches. Preserve that state through
surviving layer IDs when rebuilding the group keys.

This keeps expanded groups open after deleting an earlier layer or the
first layer of a group. Separate groups with the same prefix retain
their own states; if deletion joins two groups, the merged group stays
expanded when either was expanded.

- Fixes #1781.

Adds five deletion regression tests, a small style fixture, and a
changelog entry.

Validation:

- All 33 layer-list E2E tests and 50 unit tests pass.
- Lint and the production build, including TypeScript checking, pass.
- Manually checked OSM Liberty: all 15 groups remain expanded after
deleting the background layer.
- The full E2E suite reports 160 passing tests and one failure in
`modals > open > upload` (`Could not get styleItem from localStorage`).
The same failure reproduces with the original production code restored.

---------

Co-authored-by: Harel M <harel.mazor@gmail.com>

CHANGELOG.md

@@ -8,6 +8,7 @@
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
+- Preserve expanded layer groups when deleting layers, including the first layer of a group
 - The map's data listener now fires on tile loads again, so source and vector layer field autocompletion is populated
 - The `maputnik` desktop binary now opens the default browser automatically on startup (opt out with `--no-browser`)
 - _...Add new stuff here..._

e2e/fixtures/grouped-layers-style.json

@@ -0,0 +1,16 @@
+{
+  "version": 8,
+  "name": "Grouped layers",
+  "sources": {},
+  "layers": [
+    { "id": "background", "type": "background" },
+    { "id": "park-a", "type": "background" },
+    { "id": "park-b", "type": "background" },
+    { "id": "park-c", "type": "background" },
+    { "id": "road", "type": "background" },
+    { "id": "park-d", "type": "background" },
+    { "id": "park-e", "type": "background" },
+    { "id": "water-a", "type": "background" },
+    { "id": "water-b", "type": "background" }
+  ]
+}

e2e/layers-list.spec.ts

@@ -306,6 +306,75 @@ describe("layers list", () => {
   });
 
   describe("groups", () => {
+    for (const control of ["menu", "trash"] as const) {
+      describe(`after deletion using ${control}`, () => {
+        beforeEach(async () => {
+          await when.setStyle("grouped_layers");
+        });
+
+        test("keeps all groups expanded when an earlier layer is deleted", async () => {
+          await when.click("skip-target-layer-list");
+          await then(get.elementByTestId("layer-list-item:park-b")).shouldBeVisible();
+          await when.deleteLayer("background", control);
+          await then(get.elementByTestId("layer-list-item:background")).shouldNotExist();
+          await then(get.elementByTestId("layer-list-group:park-0")).shouldExist();
+          await expect(get.elementByTestId("layer-list-group:water-6").getByRole("button")).toHaveAttribute("aria-expanded", "true");
+          await then(get.elementByTestId("layer-list-item:park-b")).shouldBeVisible();
+          await then(get.elementByTestId("layer-list-item:park-e")).shouldBeVisible();
+          await then(get.elementByTestId("layer-list-item:water-b")).shouldBeVisible();
+        });
+
+        test("preserves separate states for groups with the same prefix", async () => {
+          await when.click("layer-list-group:park-1");
+          await when.deleteLayer("background", control);
+          await then(get.elementByTestId("layer-list-group:park-0")).shouldExist();
+          await when.click("layer-list-item:road");
+          await expect(get.elementByTestId("layer-list-group:park-0").getByRole("button")).toHaveAttribute("aria-expanded", "true");
+          await expect(get.elementByTestId("layer-list-group:park-4").getByRole("button")).toHaveAttribute("aria-expanded", "false");
+          await then(get.elementByTestId("layer-list-item:park-b")).shouldBeVisible();
+          await then(get.elementByTestId("layer-list-item:park-e")).shouldNotBeVisible();
+        });
+
+        test("keeps a group expanded when its first layer is deleted", async () => {
+          await when.click("layer-list-group:park-1");
+          await when.deleteLayer("park-a", control);
+          await then(get.elementByTestId("layer-list-item:park-a")).shouldNotExist();
+          await when.click("layer-list-item:road");
+          await expect(get.elementByTestId("layer-list-group:park-1").getByRole("button")).toHaveAttribute("aria-expanded", "true");
+          await then(get.elementByTestId("layer-list-item:park-c")).shouldBeVisible();
+        });
+
+        for (const expandedGroup of ["park-1", "park-5"]) {
+          test(`keeps a merged group expanded when ${expandedGroup} was expanded`, async () => {
+            await when.click("layer-list-group:" + expandedGroup);
+            await when.deleteLayer("road", control);
+            await then(get.elementByTestId("layer-list-item:road")).shouldNotExist();
+            await expect(get.elementByTestId("layer-list-group:park-1").getByRole("button")).toHaveAttribute("aria-expanded", "true");
+            await then(get.elementByTestId("layer-list-item:park-b")).shouldBeVisible();
+            await then(get.elementByTestId("layer-list-item:park-e")).shouldBeVisible();
+          });
+        }
+
+        test("keeps a merged group collapsed when both groups were collapsed", async () => {
+          await when.deleteLayer("road", control);
+          await then(get.elementByTestId("layer-list-item:road")).shouldNotExist();
+          await when.click("layer-list-item:background");
+          await expect(get.elementByTestId("layer-list-group:park-1").getByRole("button")).toHaveAttribute("aria-expanded", "false");
+          await then(get.elementByTestId("layer-list-item:park-b")).shouldNotBeVisible();
+          await then(get.elementByTestId("layer-list-item:park-e")).shouldNotBeVisible();
+        });
+
+        test("preserves later groups after deleting an entire group", async () => {
+          await when.click("skip-target-layer-list");
+          await when.deleteLayer("park-a", control);
+          await when.deleteLayer("park-b", control);
+          await when.deleteLayer("park-c", control);
+          await expect(get.elementByTestId("layer-list-group:water-4").getByRole("button")).toHaveAttribute("aria-expanded", "true");
+          await then(get.elementByTestId("layer-list-item:water-b")).shouldBeVisible();
+        });
+      });
+    }
+
     test("simple", async () => {
       await when.setStyle("geojson");
 

e2e/maputnik-driver.ts

@@ -39,6 +39,7 @@ export class MaputnikDriver {
         "example-style-with-zoom-7-and-center-0-51.json",
         "example-style-with-zoom-5-and-center-50-50.json",
         "access-token-style.json",
+        "grouped-layers-style.json",
       ];
       for (const fixture of styleFixtures) {
         await this.helper.given.interceptAndMockResponse({
@@ -63,6 +64,18 @@ export class MaputnikDriver {
 
     modal: this.modalDriver.when,
 
+    deleteLayer: async (id: string, control: "menu" | "trash" = "menu") => {
+      if (control === "trash") {
+        await this.helper.when.hover("layer-list-item:" + id);
+        await this.helper.when.click("layer-list-item:" + id + ":delete");
+        return;
+      }
+
+      await this.helper.when.click("layer-list-item:" + id);
+      await this.helper.when.click("skip-target-layer-editor");
+      await this.helper.when.click("menu-delete-layer");
+    },
+
     setStyle: async (
       styleProperties:
         | "geojson"
@@ -73,6 +86,7 @@ export class MaputnikDriver {
         | "font"
         | "zoom_7_center_0_51"
         | "access_tokens"
+        | "grouped_layers"
         | "",
       zoom?: number
     ) => {
@@ -85,6 +99,7 @@ export class MaputnikDriver {
         font: "example-style-with-fonts.json",
         zoom_7_center_0_51: "example-style-with-zoom-7-and-center-0-51.json",
         access_tokens: "access-token-style.json",
+        grouped_layers: "grouped-layers-style.json",
       };
 
       const url = new URL(baseUrl);

package.json

@@ -29,7 +29,7 @@
   "dependencies": {
     "@codemirror/lang-json": "^6.0.2",
     "@codemirror/lint": "^6.9.7",
-    "@codemirror/state": "^6.7.3",
+    "@codemirror/state": "^6.7.4",
     "@codemirror/theme-one-dark": "^6.1.3",
     "@codemirror/view": "^6.43.11",
     "@dnd-kit/core": "^6.3.1",
@@ -101,7 +101,7 @@
   "devDependencies": {
     "@eslint/js": "^10.0.1",
     "@istanbuljs/nyc-config-typescript": "^1.0.2",
-    "@playwright/test": "^1.62.1",
+    "@playwright/test": "^1.63.0",
     "@stylistic/eslint-plugin": "^5.10.0",
     "@types/codemirror": "^5.60.18",
     "@types/color": "^4.2.1",
@@ -125,20 +125,20 @@
     "@types/string-hash": "^1.1.3",
     "@types/wicg-file-system-access": "^2023.10.7",
     "@vitejs/plugin-react": "^6.1.1",
-    "@vitest/coverage-v8": "^4.1.11",
+    "@vitest/coverage-v8": "^5.0.0",
     "cors": "^2.8.6",
-    "eslint": "^10.9.1",
+    "eslint": "^10.10.0",
     "eslint-plugin-react": "^7.37.5",
     "eslint-plugin-react-hooks": "^7.1.1",
     "eslint-plugin-react-refresh": "^0.5.6",
-    "i18next-cli": "^1.73.0",
+    "i18next-cli": "^1.73.1",
     "istanbul": "^0.4.5",
     "istanbul-lib-coverage": "^3.2.2",
     "nyc": "^18.0.0",
     "postcss": "^8.5.28",
     "react-hot-loader": "^4.13.1",
     "sass": "^1.104.0",
-    "stylelint": "^17.14.1",
+    "stylelint": "^17.15.0",
     "stylelint-config-recommended-scss": "^17.0.1",
     "stylelint-scss": "^7.2.0",
     "typescript": "^6.0.3",
@@ -146,6 +146,6 @@
     "uuid": "^14.0.2",
     "vite": "^8.2.2",
     "vite-plugin-istanbul": "^9.0.1",
-    "vitest": "^4.1.11"
+    "vitest": "^5.0.0"
   }
 }

src/components/LayerList.tsx

@@ -106,13 +106,13 @@ class LayerListContainerInternal extends React.Component<LayerListContainerInter
     });
   };
 
-  groupedLayers(): (LayerSpecification & {key: string})[][] {
+  groupedLayers(layers = this.props.layers): (LayerSpecification & {key: string})[][] {
     const groups = [];
     const layerIdCount = new Map();
 
-    for (let i = 0; i < this.props.layers.length; i++) {
-      const origLayer = this.props.layers[i];
-      const previousLayer = this.props.layers[i-1];
+    for (let i = 0; i < layers.length; i++) {
+      const origLayer = layers[i];
+      const previousLayer = layers[i-1];
       layerIdCount.set(origLayer.id,
         layerIdCount.has(origLayer.id) ? layerIdCount.get(origLayer.id) + 1 : 0
       );
@@ -192,7 +192,39 @@ class LayerListContainerInternal extends React.Component<LayerListContainerInter
     return propsChanged;
   }
 
+  /**
+   * Preserves group state through surviving layer IDs when group indices change.
+   * If deletion joins groups, the result stays expanded when either group was.
+   */
+  private preserveLayerGroupState(previousLayers: LayerSpecification[]) {
+    if (previousLayers === this.props.layers) return;
+
+    const collapsedByLayer = new Map<string, boolean>();
+    let idx = 0;
+    for (const layers of this.groupedLayers(previousLayers)) {
+      const collapsed = this.isCollapsed(layerPrefix(layers[0].id), idx);
+      for (const layer of layers) {
+        collapsedByLayer.set(layer.id, collapsed);
+      }
+      idx += layers.length;
+    }
+
+    const collapsedGroups: {[key: string]: boolean} = {};
+    idx = 0;
+    for (const layers of this.groupedLayers()) {
+      const lookupKey = [layerPrefix(layers[0].id), idx].join("-");
+      collapsedGroups[lookupKey] = layers.every(layer => collapsedByLayer.get(layer.id) !== false);
+      idx += layers.length;
+    }
+
+    if (!lodash.isEqual(collapsedGroups, this.state.collapsedGroups)) {
+      this.setState({ collapsedGroups });
+    }
+  }
+
   componentDidUpdate (prevProps: LayerListContainerProps) {
+    this.preserveLayerGroupState(prevProps.layers);
+
     if (prevProps.selectedLayerIndex !== this.props.selectedLayerIndex) {
       const selectedItemNode = this.selectedItemRef.current;
       if (selectedItemNode && selectedItemNode.node) {