diff --git a/api/react-for-tomtom.api.d.ts b/api/react-for-tomtom.api.d.ts index 451a89d..e05eda0 100644 --- a/api/react-for-tomtom.api.d.ts +++ b/api/react-for-tomtom.api.d.ts @@ -257,7 +257,6 @@ declare class TomTomCircleController extends CircleController; resync(): Promise; - clear(): Promise; /** * 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 @@ -312,7 +311,6 @@ declare class TomTomPolylineController extends PolylineController; - clear(): Promise; /** * 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 @@ -410,6 +408,7 @@ declare class TomTomGroundImageOverlayRenderer 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: TomTomMapViewHolder; canEditStyle: () => boolean; @@ -425,6 +424,7 @@ declare class TomTomGroundImageOverlayRenderer extends AbstractGroundImageOverla /** Sync an already-created image source+layer to the current state (diffed). */ private applyToExisting; removeGroundImage(entity: GroundImageEntity): Promise; + private removeFromStyle; } declare class TomTomGroundImageController implements SlottedOverlayController { diff --git a/src/circle/TomTomCircleController.ts b/src/circle/TomTomCircleController.ts index 8ae7412..b7d693d 100644 --- a/src/circle/TomTomCircleController.ts +++ b/src/circle/TomTomCircleController.ts @@ -24,11 +24,6 @@ export class TomTomCircleController extends CircleController await this.renderer.redraw(); } - override async clear(): Promise { - 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 diff --git a/src/groundimage/TomTomGroundImageController.ts b/src/groundimage/TomTomGroundImageController.ts index b0c609f..c730e85 100644 --- a/src/groundimage/TomTomGroundImageController.ts +++ b/src/groundimage/TomTomGroundImageController.ts @@ -68,6 +68,8 @@ export class TomTomGroundImageController 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); } diff --git a/src/groundimage/TomTomGroundImageOverlayRenderer.ts b/src/groundimage/TomTomGroundImageOverlayRenderer.ts index 8f983cb..1d4ae38 100644 --- a/src/groundimage/TomTomGroundImageOverlayRenderer.ts +++ b/src/groundimage/TomTomGroundImageOverlayRenderer.ts @@ -18,6 +18,13 @@ export class TomTomGroundImageOverlayRenderer extends AbstractGroundImageOverlay private readonly canEditStyle: () => boolean; /** Last values applied to the map style, keyed by state id. */ private readonly applied = new Map(); + // 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(); constructor({ holder, @@ -34,6 +41,8 @@ export class TomTomGroundImageOverlayRenderer extends AbstractGroundImageOverlay layerId(id: string): string { return `mc-gimg-lyr-${id}`; } async createGroundImage(state: GroundImageState): Promise { + // 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 TomTomGroundImageOverlayRenderer extends AbstractGroundImageOverlay async removeGroundImage(entity: GroundImageEntity): Promise { 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)); } } diff --git a/src/polyline/TomTomPolylineController.ts b/src/polyline/TomTomPolylineController.ts index 0829125..8d04462 100644 --- a/src/polyline/TomTomPolylineController.ts +++ b/src/polyline/TomTomPolylineController.ts @@ -19,11 +19,6 @@ export class TomTomPolylineController extends PolylineController { - 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 diff --git a/test/GroundImageRemoval.test.mjs b/test/GroundImageRemoval.test.mjs new file mode 100644 index 0000000..d6baebc --- /dev/null +++ b/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/TomTomGroundImageController.ts', + renderer: 'src/groundimage/TomTomGroundImageOverlayRenderer.ts', + }, + outDir, format: ['esm'], platform: 'node', splitting: false, + dts: false, sourcemap: false, config: false, silent: true, +}); +const { TomTomGroundImageController } = await import(pathToFileURL(resolve(outDir, 'controller.mjs'))); +const { TomTomGroundImageOverlayRenderer } = 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 TomTomGroundImageOverlayRenderer({ holder: { map }, canEditStyle: () => ready }); + const controller = new TomTomGroundImageController(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); +});