Hold ground image removals until the style can be edited; drop the clear() overrides

api/react-for-longdo.api.d.ts

@@ -332,7 +332,6 @@ declare class LongdoCircleController extends CircleController<LongdoActualCircle
     constructor(renderer: LongdoCircleOverlayRenderer);
     update(state: CircleState): Promise<void>;
     resync(): Promise<void>;
-    clear(): Promise<void>;
     /**
      * Hit-test a map click (its lat/lng) against the circles geometrically (inside
      * the fill radius) and dispatch the click on the matching circle. Does NOT use
@@ -387,7 +386,6 @@ declare class LongdoPolylineController extends PolylineController<LongdoActualPo
     readonly renderer: LongdoPolylineOverlayRenderer;
     constructor(renderer: LongdoPolylineOverlayRenderer);
     resync(): Promise<void>;
-    clear(): Promise<void>;
     /**
      * Hit-test a map click (its lat/lng) against the polylines geometrically and,
      * if the click lands within the tap tolerance of a line, dispatch the click on
@@ -485,6 +483,7 @@ declare class LongdoGroundImageOverlayRenderer extends AbstractGroundImageOverla
     private readonly canEditStyle;
     /** Last values applied to the map style, keyed by state id. */
     private readonly applied;
+    private readonly pendingRemovals;
     constructor({ holder, canEditStyle, }: {
         holder: LongdoMapViewHolder;
         canEditStyle: () => boolean;
@@ -500,6 +499,7 @@ declare class LongdoGroundImageOverlayRenderer extends AbstractGroundImageOverla
     /** Sync an already-created image source+layer to the current state (diffed). */
     private applyToExisting;
     removeGroundImage(entity: GroundImageEntity<string>): Promise<void>;
+    private removeFromStyle;
 }
 
 declare class LongdoGroundImageController implements SlottedOverlayController {

src/circle/LongdoCircleController.ts

@@ -24,11 +24,6 @@ export class LongdoCircleController extends CircleController<LongdoActualCircle>
     await this.renderer.redraw();
   }
 
-  override async clear(): Promise<void> {
-    await super.clear();
-    await this.renderer.redraw();
-  }
-
   /**
    * Hit-test a map click (its lat/lng) against the circles geometrically (inside
    * the fill radius) and dispatch the click on the matching circle. Does NOT use

src/groundimage/LongdoGroundImageController.ts

@@ -68,6 +68,8 @@ export class LongdoGroundImageController implements SlottedOverlayController {
 
   resync(): void {
     this.cancelPendingUpdates();
+    // Called when the style becomes editable again: apply removals held back meanwhile.
+    this.renderer.flushPendingRemovals();
     this.groundImageIds.clear();
     for (const state of this.groundImageStates.values()) void this.upsert(state);
   }

src/groundimage/LongdoGroundImageOverlayRenderer.ts

@@ -18,6 +18,13 @@ export class LongdoGroundImageOverlayRenderer extends AbstractGroundImageOverlay
   private readonly canEditStyle: () => boolean;
   /** Last values applied to the map style, keyed by state id. */
   private readonly applied = new Map<string, { url: string; coordsKey: string; opacity: number }>();
+  // Removals that arrived while the style could not be edited, applied by
+  // flushPendingRemovals() once it can. Dropping them instead left the image on
+  // the map for good: the controller forgets the state as soon as it asks for
+  // the removal, so nothing would ask again. canEditStyle turns false whenever
+  // isStyleLoaded() does, which includes the moments right after other overlays
+  // rewrite their sources, so clearOverlays() hit this every time.
+  private readonly pendingRemovals = new Set<string>();
 
   constructor({
     holder,
@@ -34,6 +41,8 @@ export class LongdoGroundImageOverlayRenderer extends AbstractGroundImageOverlay
   layerId(id: string): string { return `mc-gimg-lyr-${id}`; }
 
   async createGroundImage(state: GroundImageState): Promise<string | null> {
+    // Added again before a held-back removal ran: the caller wants it on the map.
+    this.pendingRemovals.delete(state.id);
     const coordinates = groundImageCoordinates(state);
     if (!coordinates) return null;
 
@@ -140,8 +149,26 @@ export class LongdoGroundImageOverlayRenderer extends AbstractGroundImageOverlay
 
   async removeGroundImage(entity: GroundImageEntity<string>): Promise<void> {
     this.applied.delete(entity.groundImage);
-    if (!this.canEditStyle()) return;
-    removeLayerIfExists(this.holder.map, this.layerId(entity.groundImage));
-    removeSourceIfExists(this.holder.map, this.sourceId(entity.groundImage));
+    if (!this.canEditStyle()) {
+      this.pendingRemovals.add(entity.groundImage);
+      return;
+    }
+    this.pendingRemovals.delete(entity.groundImage);
+    this.removeFromStyle(entity.groundImage);
+  }
+
+  /**
+   * Applies the removals held back while the style could not be edited.
+   * @internal
+   */
+  flushPendingRemovals(): void {
+    if (this.pendingRemovals.size === 0 || !this.canEditStyle()) return;
+    for (const id of this.pendingRemovals) this.removeFromStyle(id);
+    this.pendingRemovals.clear();
+  }
+
+  private removeFromStyle(id: string): void {
+    removeLayerIfExists(this.holder.map, this.layerId(id));
+    removeSourceIfExists(this.holder.map, this.sourceId(id));
   }
 }

src/polyline/LongdoPolylineController.ts

@@ -19,11 +19,6 @@ export class LongdoPolylineController extends PolylineController<LongdoActualPol
     await this.renderer.redraw();
   }
 
-  override async clear(): Promise<void> {
-    await super.clear();
-    await this.renderer.redraw();
-  }
-
   /**
    * Hit-test a map click (its lat/lng) against the polylines geometrically and,
    * if the click lands within the tap tolerance of a line, dispatch the click on

test/GroundImageRemoval.test.mjs

@@ -0,0 +1,127 @@
+import assert from 'node:assert/strict';
+import { mkdirSync, mkdtempSync, rmSync } from 'node:fs';
+import { after, test } from 'node:test';
+import { pathToFileURL } from 'node:url';
+import { resolve } from 'node:path';
+import { build } from 'tsup';
+import { createGeoPoint, createGeoRectBounds, createGroundImageState } from '@mapconductor/js-sdk-core';
+
+// Exercise the actual internal classes without exporting them as public API.
+// Only the map is replaced; no DOM, WebGL, or network is needed.
+mkdirSync(new URL('../build/', import.meta.url), { recursive: true });
+const outDir = mkdtempSync(new URL('../build/groundimage-tests-', import.meta.url));
+after(() => rmSync(outDir, { recursive: true, force: true }));
+await build({
+    entry: {
+        controller: 'src/groundimage/LongdoGroundImageController.ts',
+        renderer: 'src/groundimage/LongdoGroundImageOverlayRenderer.ts',
+    },
+    outDir, format: ['esm'], platform: 'node', splitting: false,
+    dts: false, sourcemap: false, config: false, silent: true,
+});
+const { LongdoGroundImageController } = await import(pathToFileURL(resolve(outDir, 'controller.mjs')));
+const { LongdoGroundImageOverlayRenderer } = await import(pathToFileURL(resolve(outDir, 'renderer.mjs')));
+
+const sourceId = 'mc-gimg-src-GI';
+const layerId = 'mc-gimg-lyr-GI';
+
+function state(opacity = 0.9) {
+    return createGroundImageState({
+        id: 'GI',
+        bounds: createGeoRectBounds({
+            southWest: createGeoPoint({ latitude: 35.64, longitude: 139.74 }),
+            northEast: createGeoPoint({ latitude: 35.665, longitude: 139.78 }),
+        }),
+        imageUrl: '/image.png',
+        opacity,
+    });
+}
+
+function fixture() {
+    const sources = new Map();
+    const layers = new Map();
+    let ready = true;
+    const map = {
+        getSource: (id) => sources.get(id),
+        getLayer: (id) => layers.get(id),
+        addSource(id, source) {
+            assert.ok(ready, 'cannot add a source while the style is unavailable');
+            assert.ok(!sources.has(id));
+            sources.set(id, { ...source, updateImage() {}, setCoordinates() {} });
+        },
+        addLayer(layer) {
+            assert.ok(ready, 'cannot add a layer while the style is unavailable');
+            assert.ok(sources.has(layer.source));
+            assert.ok(!layers.has(layer.id));
+            layers.set(layer.id, layer);
+        },
+        removeLayer(id) {
+            assert.ok(ready, 'cannot remove a layer while the style is unavailable');
+            layers.delete(id);
+        },
+        removeSource(id) {
+            assert.ok(ready, 'cannot remove a source while the style is unavailable');
+            assert.ok(![...layers.values()].some((layer) => layer.source === id));
+            sources.delete(id);
+        },
+        setPaintProperty(id, property, value) {
+            layers.get(id).paint[property] = value;
+        },
+        moveLayer() {},
+    };
+    const renderer = new LongdoGroundImageOverlayRenderer({ holder: { map }, canEditStyle: () => ready });
+    const controller = new LongdoGroundImageController(renderer);
+    return {
+        controller,
+        setReady(value) { ready = value; },
+        onMap: () => sources.has(sourceId) || layers.has(layerId),
+    };
+}
+
+const settle = () => new Promise((r) => setTimeout(r, 0));
+
+test('a removal while the style is not editable is applied once it is', async () => {
+    const f = fixture();
+    f.controller.composition([state()]);
+    await settle();
+    assert.ok(f.onMap());
+
+    // clearOverlays() runs right after other overlays rewrote their sources,
+    // when isStyleLoaded() -- and so canEditStyle -- is briefly false.
+    f.setReady(false);
+    f.controller.clear();
+    await settle();
+    assert.equal(f.controller.hasId('GI'), false);
+
+    f.setReady(true);
+    f.controller.resync();
+    await settle();
+    assert.equal(f.onMap(), false, 'the cleared ground image is still on the map');
+});
+
+test('adding the same id again before the style is editable keeps it', async () => {
+    const f = fixture();
+    f.controller.composition([state()]);
+    await settle();
+
+    f.setReady(false);
+    f.controller.clear();
+    await settle();
+    f.controller.composition([state(0.4)]);
+    await settle();
+
+    f.setReady(true);
+    f.controller.resync();
+    await settle();
+    assert.ok(f.onMap(), 'the re-added ground image was removed');
+    assert.equal(f.controller.hasId('GI'), true);
+});
+
+test('a removal while the style is editable is applied at once', async () => {
+    const f = fixture();
+    f.controller.composition([state()]);
+    await settle();
+    f.controller.composition([]);
+    await settle();
+    assert.equal(f.onMap(), false);
+});