fix(geo-editor): keep edit overlay at the edited layer's z-position (#1015) (#1017)

* fix(geo-editor): keep edit overlay at the edited layer's z-position

When geometry editing is active, the Geoman edit layers (and the GeoEditor
selection layers) could drift below other layers loaded above the edited
layer, so the features being edited disappeared behind them.

MapController.syncLayers reorders the map's style layers on every layers
change and has no knowledge of the editor overlay, so the overlay drifted out
of place after any re-sync (e.g. toggling another layer's visibility). The
editor now repositions the overlay to sit directly above the edited layer's
own (hidden) map layers whenever it (re)shows the overlay or the layers array
changes during a session, restoring the layer-tree order.

The ordering decision is a pure helper (planGeoEditorOverlayOrder) with unit
tests.

Fixes #1015

* refactor(geo-editor): harden overlay-ordering helpers per review

- Fall back to the conventional layer ids when nativeLayerIds holds only
  non-string entries.
- Document where the geo-editor selection-layer prefix comes from and that it
  degrades safely if the library renames those layers.
- Coalesce the scheduled display re-applies so a burst of unrelated layer
  updates during an edit session collapses to one apply per phase.

Author: giswqs

packages/plugins/src/plugins/geo-editor-geometry.ts

@@ -160,3 +160,91 @@ export function reconcileEditedFeatures(
     }),
   };
 }
+
+/** A map style layer described by what role it plays for overlay ordering. */
+export interface OverlayOrderLayer {
+  /** The MapLibre style layer id. */
+  id: string;
+  /** True for the GeoEditor overlay (Geoman `gm_*` and `geo-editor-*`) layers. */
+  isOverlay: boolean;
+  /** True for the edited layer's own (anchor) map layers. */
+  isAnchor: boolean;
+}
+
+/** The move the caller should apply to keep the overlay above the edited layer. */
+export interface OverlayOrderPlan {
+  /** The ids of the overlay layers, in their current bottom-to-top order. */
+  overlayIds: string[];
+  /**
+   * The MapLibre `beforeId` to pass to `moveLayer` for each overlay layer: the
+   * first non-overlay layer above the edited layer, or `undefined` to move the
+   * overlay to the very top (nothing but overlay sits above the edited layer).
+   */
+  beforeId: string | undefined;
+}
+
+/**
+ * Decide how to reposition the GeoEditor overlay so it renders at the edited
+ * layer's slot in the stack. `MapController.syncLayers` reorders the map's
+ * layers on every layers change and has no knowledge of the overlay, so without
+ * this the overlay drifts below any layer stacked above the edited one and the
+ * edit features disappear behind it (issue #1015).
+ *
+ * Returns `null` when nothing should move: the edited layer is not on the map
+ * (no anchor), there are no overlay layers, or the overlay already sits in one
+ * contiguous run directly above the anchor (so re-applying would needlessly
+ * churn the style and re-fire `styledata`).
+ *
+ * @param layers The map's style layers, bottom-to-top, tagged with their roles.
+ * @returns The reposition plan, or `null` when no move is needed.
+ */
+export function planGeoEditorOverlayOrder(
+  layers: OverlayOrderLayer[],
+): OverlayOrderPlan | null {
+  let lastAnchorIndex = -1;
+  for (let i = 0; i < layers.length; i += 1) {
+    if (layers[i].isAnchor) lastAnchorIndex = i;
+  }
+  // Without the edited layer on the map there is no anchor; leave the overlay
+  // where Geoman placed it (on top) rather than guessing a position.
+  if (lastAnchorIndex < 0) return null;
+
+  const overlayIds = layers
+    .filter((layer) => layer.isOverlay)
+    .map((layer) => layer.id);
+  if (overlayIds.length === 0) return null;
+
+  if (overlayLayersAlreadyPositioned(layers, overlayIds.length, lastAnchorIndex)) {
+    return null;
+  }
+
+  // Anchor the overlay just below the first non-overlay layer above the edited
+  // layer (or on top of the map when nothing sits above it).
+  let beforeId: string | undefined;
+  for (let i = lastAnchorIndex + 1; i < layers.length; i += 1) {
+    if (!layers[i].isOverlay) {
+      beforeId = layers[i].id;
+      break;
+    }
+  }
+
+  return { overlayIds, beforeId };
+}
+
+/**
+ * Whether the overlay layers already sit in one contiguous run directly above
+ * the edited layer's anchor layers, so no reposition is needed.
+ */
+function overlayLayersAlreadyPositioned(
+  layers: OverlayOrderLayer[],
+  overlayCount: number,
+  lastAnchorIndex: number,
+): boolean {
+  const start = lastAnchorIndex + 1;
+  if (start + overlayCount > layers.length) return false;
+  for (let i = 0; i < overlayCount; i += 1) {
+    if (!layers[start + i].isOverlay) return false;
+  }
+  const after = layers[start + overlayCount];
+  return !(after && after.isOverlay);
+}

packages/plugins/src/plugins/maplibre-geo-editor.ts

@@ -11,6 +11,7 @@ import { GeoEditor, type GeoEditorOptions } from "maplibre-gl-geo-editor";
 import {
   SKETCHES_SOURCE_KIND,
   canEditLayerGeometry,
+  planGeoEditorOverlayOrder,
   reconcileEditedFeatures,
   tagFeatureKeys,
 } from "./geo-editor-geometry";
@@ -791,17 +792,18 @@ function bindSketchesStoreSync(): void {
       const previousTarget = previous.layers.find(
         (layer) => layer.id === editTargetLayerId,
       );
-      if (
-        previousTarget &&
-        (target.visible !== previousTarget.visible ||
-          target.opacity !== previousTarget.opacity ||
-          target.style !== previousTarget.style)
-      ) {
-        // If the user toggled the target layer back on while editing, re-hide it
-        // so its stale normal rendering does not double-draw over Geoman.
-        if (target.visible && !previousTarget.visible) {
-          setEditTargetStoreVisible(editTargetLayerId, false);
-        }
+      // If the user toggled the target layer back on while editing, re-hide it
+      // so its stale normal rendering does not double-draw over Geoman.
+      if (target.visible && previousTarget && !previousTarget.visible) {
+        setEditTargetStoreVisible(editTargetLayerId, false);
+      }
+      // Any change to the layers array (this layer's visibility/opacity/style, or
+      // another layer being added, removed, reordered, or toggled) re-runs
+      // MapController.syncLayers, which reorders the map's style layers and can
+      // push the editor's overlay below a layer stacked above the edited one
+      // (issue #1015). Re-apply the edit display so the overlay returns to the
+      // edited layer's slot in the stack.
+      if (state.layers !== previous.layers) {
         scheduleApplySketchesMapDisplay();
       }
       return;
@@ -884,12 +886,14 @@ function applySketchesMapDisplay(): void {
   // visibility for the target is unnecessary and would race the layer sync.
   if (editTargetLayerId) {
     showGeomanDisplayLayers();
+    positionGeoEditorOverlayLayers();
     scheduleShowGeomanDisplayLayersOnStyleData();
     return;
   }
 
   if (isGeoEditorInteractionMode()) {
     showGeomanDisplayLayers();
+    positionGeoEditorOverlayLayers();
     scheduleShowGeomanDisplayLayersOnStyleData();
     setSketchesMapLayerSuppressed(true);
     return;
@@ -899,9 +903,30 @@ function applySketchesMapDisplay(): void {
   setSketchesMapLayerSuppressed(false);
 }
 
+// Coalesce flags so a burst of store updates (e.g. an opacity slider dragged on
+// any layer while a geometry-edit session is active, each of which re-runs
+// MapController.syncLayers and can disturb the overlay ordering) collapses to a
+// single apply per phase instead of one apply per update. The two phases are
+// kept: a microtask pass and a macrotask pass, so the display is re-applied both
+// before and after the layer sync that the same update triggers.
+let applyMicrotaskPending = false;
+let applyMacrotaskPending = false;
+
 function scheduleApplySketchesMapDisplay(): void {
-  queueMicrotask(() => applySketchesMapDisplay());
-  window.setTimeout(() => applySketchesMapDisplay(), 0);
+  if (!applyMicrotaskPending) {
+    applyMicrotaskPending = true;
+    queueMicrotask(() => {
+      applyMicrotaskPending = false;
+      applySketchesMapDisplay();
+    });
+  }
+  if (!applyMacrotaskPending) {
+    applyMacrotaskPending = true;
+    window.setTimeout(() => {
+      applyMacrotaskPending = false;
+      applySketchesMapDisplay();
+    }, 0);
+  }
 }
 
 function scheduleShowGeomanDisplayLayersOnStyleData(): void {
@@ -912,6 +937,7 @@ function scheduleShowGeomanDisplayLayersOnStyleData(): void {
     pendingStyleDataListener = null;
     if (editTargetLayerId || isGeoEditorInteractionMode()) {
       showGeomanDisplayLayers();
+      positionGeoEditorOverlayLayers();
     }
   };
   map.once("styledata", pendingStyleDataListener);
@@ -985,6 +1011,72 @@ function showGeomanDisplayLayers(): void {
   setGeomanDisplayLayersVisibility("visible");
 }
 
+/**
+ * The edited layer's own map layers, used as the z-anchor for the editor
+ * overlay. Add-Vector-Layer layers carry their layer ids in
+ * `metadata.nativeLayerIds`; plain geojson layers use the conventional
+ * `layer-<id>-*` ids. Only ids that actually exist on the map are returned.
+ */
+function geoEditorTargetAnchorLayerIds(
+  map: maplibregl.Map,
+  layer: GeoLibreLayer,
+): string[] {
+  const nativeLayerIds = layer.metadata.nativeLayerIds;
+  // Filter to strings first, then fall back: a non-empty `nativeLayerIds` that
+  // holds only non-string entries must still fall back to the conventional ids.
+  const stringIds = Array.isArray(nativeLayerIds)
+    ? nativeLayerIds.filter((id): id is string => typeof id === "string")
+    : [];
+  const candidates =
+    stringIds.length > 0 ? stringIds : sketchesMapLayerIds(layer.id);
+  return candidates.filter((id) => map.getLayer(id));
+}
+
+/**
+ * Move the editor's overlay layers (Geoman's `gm_*` display layers and the
+ * GeoEditor selection layers) to sit immediately above the edited layer's own
+ * (hidden) map layers, so the features being edited render at that layer's
+ * position in the tree.
+ *
+ * `MapController.syncLayers` reorders the store's layers on every layers change
+ * and has no knowledge of the overlay, so without this the overlay drifts below
+ * any layer stacked above the edited one and the edit features disappear behind
+ * it (issue #1015). Idempotent: `planGeoEditorOverlayOrder` returns `null` when
+ * the overlay is already in place, so the `styledata` that `moveLayer` triggers
+ * does not loop.
+ */
+function positionGeoEditorOverlayLayers(): void {
+  const map = appApi?.getMap?.();
+  if (!map) return;
+  const styleLayers = map.getStyle()?.layers;
+  if (!styleLayers || styleLayers.length === 0) return;
+
+  const target = activeEditableLayer(useAppStore.getState().layers);
+  if (!target) return;
+  const anchorIds = new Set(geoEditorTargetAnchorLayerIds(map, target));
+  // Without the edited layer on the map there is no anchor; leave the overlay
+  // where Geoman placed it (on top) rather than guessing a position.
+  if (anchorIds.size === 0) return;
+
+  const plan = planGeoEditorOverlayOrder(
+    styleLayers.map((layer) => ({
+      id: layer.id,
+      isOverlay: isGeoEditorOverlayLayer(layer),
+      isAnchor: anchorIds.has(layer.id),
+    })),
+  );
+  if (!plan) return;
+
+  // Moving each id before the same anchor preserves the overlay's draw order.
+  for (const id of plan.overlayIds) {
+    try {
+      map.moveLayer(id, plan.beforeId);
+    } catch {
+      // A layer may have been removed by a concurrent style change.
+    }
+  }
+}
+
 function applyGeomanSketchesStyle(
   map: maplibregl.Map,
   sketchesLayer: GeoLibreLayer,
@@ -1078,6 +1170,28 @@ function setGeomanPaintProperty(
   }
 }
 
+/**
+ * Every map layer that belongs to the editor overlay and must be kept above the
+ * edited layer: Geoman's `gm_*` display layers plus the GeoEditor's own
+ * selection layers. Used only for z-ordering, so it is broader than
+ * `isGeomanDisplayLayer` (which drives show/hide of the Geoman layers).
+ *
+ * The `geo-editor` prefix matches the layers `maplibre-gl-geo-editor` adds for
+ * its selection highlight (observed ids `geo-editor-selection-{fill,line,
+ * circle}-layer` on the `geo-editor-selection-source` source). The library does
+ * not export those ids, so this stays a prefix heuristic; if a future version
+ * renames them the overlay would simply not be re-stacked (a safe degradation,
+ * no error). The Geoman `gm_*` layers are matched by `isGeomanDisplayLayer`.
+ */
+function isGeoEditorOverlayLayer(layer: maplibregl.LayerSpecification): boolean {
+  if (isGeomanDisplayLayer(layer)) return true;
+  const id = layer.id.toLowerCase();
+  if (id.startsWith("geo-editor")) return true;
+  if (!("source" in layer)) return false;
+  const source = layer.source;
+  return typeof source === "string" && source.startsWith("geo-editor");
+}
+
 function isGeomanDisplayLayer(layer: maplibregl.LayerSpecification): boolean {
   const id = layer.id.toLowerCase();
   if (id.startsWith("gm_") || id.startsWith("gm-")) {

tests/geo-editor-geometry.test.ts

@@ -4,7 +4,9 @@ import type { FeatureCollection } from "geojson";
 import type { GeoLibreLayer } from "../packages/core/src/types";
 import {
   GEOMETRY_EDIT_FID_PROPERTY,
+  type OverlayOrderLayer,
   canEditLayerGeometry,
+  planGeoEditorOverlayOrder,
   reconcileEditedFeatures,
   tagFeatureKeys,
 } from "../packages/plugins/src/plugins/geo-editor-geometry";
@@ -256,3 +258,75 @@ describe("reconcileEditedFeatures", () => {
     assert.equal(new Set(ids).size, ids.length);
   });
 });
+
+describe("planGeoEditorOverlayOrder", () => {
+  function row(
+    id: string,
+    flags: Partial<OverlayOrderLayer> = {},
+  ): OverlayOrderLayer {
+    return { id, isOverlay: false, isAnchor: false, ...flags };
+  }
+
+  it("raises overlay above a layer stacked over the edited layer (issue #1015)", () => {
+    // Bottom-to-top: the overlay has sunk below the raster, which is stacked
+    // above the (hidden) edited layer; it must move back up to the edited slot.
+    const plan = planGeoEditorOverlayOrder([
+      row("basemap"),
+      row("gm_main-fill", { isOverlay: true }),
+      row("geo-editor-selection-fill", { isOverlay: true }),
+      row("xyz-raster"),
+      row("edited-fill", { isAnchor: true }),
+      row("edited-line", { isAnchor: true }),
+    ]);
+    assert.deepEqual(plan, {
+      overlayIds: ["gm_main-fill", "geo-editor-selection-fill"],
+      // The edited layer is the topmost data layer here, so nothing real sits
+      // above it: the overlay goes to the very top.
+      beforeId: undefined,
+    });
+  });
+
+  it("anchors the overlay just below the first layer above the edited layer", () => {
+    // The overlay has sunk to the bottom; the edited layer is genuinely below a
+    // raster, so the overlay must return to the edited layer's slot (below the
+    // raster), not jump to the very top.
+    const plan = planGeoEditorOverlayOrder([
+      row("basemap"),
+      row("gm_main-fill", { isOverlay: true }),
+      row("edited-fill", { isAnchor: true }),
+      row("raster-on-top"),
+    ]);
+    assert.deepEqual(plan, {
+      overlayIds: ["gm_main-fill"],
+      beforeId: "raster-on-top",
+    });
+  });
+
+  it("returns null when the overlay already sits directly above the anchor", () => {
+    const plan = planGeoEditorOverlayOrder([
+      row("basemap"),
+      row("edited-fill", { isAnchor: true }),
+      row("edited-line", { isAnchor: true }),
+      row("gm_main-fill", { isOverlay: true }),
+      row("geo-editor-selection-fill", { isOverlay: true }),
+      row("raster-on-top"),
+    ]);
+    assert.equal(plan, null);
+  });
+
+  it("returns null when the edited layer is not on the map (no anchor)", () => {
+    const plan = planGeoEditorOverlayOrder([
+      row("basemap"),
+      row("gm_main-fill", { isOverlay: true }),
+    ]);
+    assert.equal(plan, null);
+  });
+
+  it("returns null when there are no overlay layers", () => {
+    const plan = planGeoEditorOverlayOrder([
+      row("basemap"),
+      row("edited-fill", { isAnchor: true }),
+    ]);
+    assert.equal(plan, null);
+  });
+});