From e301be82b86cfeb0b4dca1b93e2531256e87ab54 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 19 Jun 2026 09:19:12 +0100 Subject: [PATCH 01/14] IM-373 added a temp assertion of mapStyle --- .../src/adapters/maplibre/maplibreLayerAdapter.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js index a3641f2b..cfb9c943 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js @@ -38,12 +38,19 @@ export default class MaplibreLayerAdapter { // ─── Lifecycle ────────────────────────────────────────────────────────────── + assertMapStyle (methodName, mapStyle) { + if (mapStyle !== datasetRegistry.mapStyle) { + console.error(`MaplibreLayerAdapter.${methodName} has mapStyle ${mapStyle.id}, but datasetRegistry has ${datasetRegistry.mapStyle.id}.`) + } + } + /** * Initialise all datasets: register patterns, add layers, then wait for idle. * @param {Object} mapStyle * @returns {Promise} Resolves once the map has processed all layers. */ async init (mapStyle) { + this.assertMapStyle('init', mapStyle) const { patternConfigs, symbolConfigs } = datasetRegistry.getPatternAndSymbolConfigs() await this.addPatternsAndSymbolsToMap(patternConfigs, symbolConfigs, mapStyle) @@ -60,6 +67,7 @@ export default class MaplibreLayerAdapter { } async addPatternsAndSymbolsToMap (patterns, symbols, mapStyle) { + this.assertMapStyle('addPatternsAndSymbolsToMap', mapStyle) const mapStyleId = mapStyle.id return Promise.all([ this._mapProvider.addPatternsToMap(patterns, mapStyleId, this._patternRegistry), @@ -91,6 +99,7 @@ export default class MaplibreLayerAdapter { * @returns {Promise} */ async onMapStyleChange (newMapStyle, dynamicSources) { + this.assertMapStyle('onMapStyleChange', newMapStyle) // MapLibre wipes all sources/layers on style change — must wait for idle first await new Promise(resolve => this._map.once('idle', resolve)) @@ -115,6 +124,7 @@ export default class MaplibreLayerAdapter { * @returns {Promise} */ async onMapSizeChange (mapStyle) { + this.assertMapStyle('onMapSizeChange', mapStyle) const { patternConfigs, symbolConfigs } = datasetRegistry.getPatternAndSymbolConfigs() await this.addPatternsAndSymbolsToMap(patternConfigs, symbolConfigs, mapStyle) @@ -142,6 +152,7 @@ export default class MaplibreLayerAdapter { * @param {Object} mapStyle */ async addDataset (datasetId, mapStyle) { + this.assertMapStyle('addDataset', mapStyle) const registryDataset = datasetRegistry.getDataset(datasetId) await this.addPatternsAndSymbolsToMap(registryDataset.patternConfigs, registryDataset.symbolConfigs, mapStyle) this._addLayers(registryDataset, mapStyle) @@ -196,6 +207,7 @@ export default class MaplibreLayerAdapter { * @returns {Promise} */ async applyStyle (datasetId, mapStyle) { + this.assertMapStyle('applyStyle', mapStyle) const registryDataset = datasetRegistry.getDataset(datasetId) registryDataset.layerIds.forEach(layerId => this.removeLayer(layerId)) await this.addPatternsAndSymbolsToMap(registryDataset.patternConfigs, registryDataset.symbolConfigs, mapStyle) @@ -254,6 +266,7 @@ export default class MaplibreLayerAdapter { } _addLayers (registryDataset, mapStyle) { + this.assertMapStyle('_addLayers', mapStyle) const sourceId = addDatasetLayers(this._map, registryDataset, mapStyle, this._symbolRegistry, this._patternRegistry, this._pixelRatio) this._datasetSourceMap.set(registryDataset.id, sourceId) this._maintainSymbolOrdering(registryDataset) From a206f160f173288f671463b75d048313deeef6dc Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 19 Jun 2026 14:03:55 +0100 Subject: [PATCH 02/14] IM-373 WIP towards moving the mapStyle dependency out of the layerAdapter --- .../beta/datasets/src/adapters/layerAdapter.js | 9 +++++++++ .../adapters/maplibre/maplibreLayerAdapter.js | 6 +++++- .../datasets/src/initialise/DatasetsInit.jsx | 12 ++++++++++-- .../src/initialise/initialiseDatasets.js | 17 +++++++++-------- .../datasets/src/registry/datasetRegistry.js | 10 +++++++--- 5 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 plugins/beta/datasets/src/adapters/layerAdapter.js diff --git a/plugins/beta/datasets/src/adapters/layerAdapter.js b/plugins/beta/datasets/src/adapters/layerAdapter.js new file mode 100644 index 00000000..2188f3a1 --- /dev/null +++ b/plugins/beta/datasets/src/adapters/layerAdapter.js @@ -0,0 +1,9 @@ +export class LayerAdapter { + attachDynamicSources (dynamicSources) { + this._dynamicSources = dynamicSources + } + + get dynamicSources () { + return this._dynamicSources + } +} diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js index cfb9c943..2c4f82c8 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js @@ -1,3 +1,4 @@ +import { LayerAdapter } from '../layerAdapter.js' import { addDatasetLayers } from './layerBuilders.js' import { MapLibreDataset } from './registry/mapLibreDataset.js' import { datasetRegistry } from '../../registry/datasetRegistry.js' @@ -14,13 +15,14 @@ import { datasetRegistry } from '../../registry/datasetRegistry.js' * Symbol image rasterisation is delegated to the map provider via * `mapProvider.addSymbolsToMap()`, keeping this adapter free of provider internals. */ -export default class MaplibreLayerAdapter { +export default class MaplibreLayerAdapter extends LayerAdapter { /** * @param {Object} mapProvider - Map provider instance (e.g. MapLibreProvider) * @param {Object} symbolRegistry * @param {Object} patternRegistry */ constructor (mapProvider, symbolRegistry, patternRegistry) { + super() this._mapProvider = mapProvider this._map = mapProvider.map this._symbolRegistry = symbolRegistry @@ -41,6 +43,8 @@ export default class MaplibreLayerAdapter { assertMapStyle (methodName, mapStyle) { if (mapStyle !== datasetRegistry.mapStyle) { console.error(`MaplibreLayerAdapter.${methodName} has mapStyle ${mapStyle.id}, but datasetRegistry has ${datasetRegistry.mapStyle.id}.`) + } else { + console.log(`MaplibreLayerAdapter.${methodName} has mapStyle ${mapStyle.id}, which matches datasetRegistry.`) } } diff --git a/plugins/beta/datasets/src/initialise/DatasetsInit.jsx b/plugins/beta/datasets/src/initialise/DatasetsInit.jsx index 0f45026a..7f4fb53b 100755 --- a/plugins/beta/datasets/src/initialise/DatasetsInit.jsx +++ b/plugins/beta/datasets/src/initialise/DatasetsInit.jsx @@ -49,8 +49,16 @@ export function DatasetsInit ({ pluginConfig, pluginState, appState, mapState, m initDatasets() }, [isBaseMapReady, appState.mode]) - useEffect(() => datasetRegistry.attach(pluginState.mappedDatasets, pluginState.orderedDatasets, mapState.mapStyle), - [pluginState.mappedDatasets, pluginState.orderedDatasets, mapState.mapStyle]) + useEffect(() => datasetRegistry.attach(pluginState.mappedDatasets, pluginState.orderedDatasets), + [pluginState.mappedDatasets, pluginState.orderedDatasets]) + + useEffect(() => { + datasetRegistry.attachMapStyle(mapState.mapStyle) + if (layerAdapter?.onMapStyleChange) { + layerAdapter.onMapStyleChange(mapState.mapStyle) + } + }, + [mapState.mapStyle]) useEffect(() => attachGlobalState(pluginState.globals), [pluginState.globals]) diff --git a/plugins/beta/datasets/src/initialise/initialiseDatasets.js b/plugins/beta/datasets/src/initialise/initialiseDatasets.js index 7772866a..18648380 100644 --- a/plugins/beta/datasets/src/initialise/initialiseDatasets.js +++ b/plugins/beta/datasets/src/initialise/initialiseDatasets.js @@ -46,24 +46,25 @@ export const initialiseDatasets = ({ eventBus.emit('datasets:ready') }) - let currentMapStyle = mapStyle + // let currentMapStyle = mapStyle // Handle basemap style changes — delegate entirely to the adapter - const onSetMapStyle = (newMapStyle) => { - currentMapStyle = newMapStyle - adapter.onMapStyleChange(newMapStyle, dynamicSources) - } + // const onSetMapStyle = (newMapStyle) => { + // currentMapStyle = newMapStyle + // adapter.onMapStyleChange(newMapStyle, dynamicSources) + // } const onMapSizeChange = () => { - adapter.onMapSizeChange(currentMapStyle) + // adapter.onMapSizeChange(currentMapStyle) + adapter.onMapSizeChange() } - eventBus.on(events.MAP_SET_STYLE, onSetMapStyle) + // eventBus.on(events.MAP_SET_STYLE, onSetMapStyle) eventBus.on(events.MAP_SIZE_CHANGE, onMapSizeChange) return { remove () { - eventBus.off(events.MAP_SET_STYLE, onSetMapStyle) + // eventBus.off(events.MAP_SET_STYLE, onSetMapStyle) eventBus.off(events.MAP_SIZE_CHANGE, onMapSizeChange) // Clean up dynamic sources diff --git a/plugins/beta/datasets/src/registry/datasetRegistry.js b/plugins/beta/datasets/src/registry/datasetRegistry.js index f7939a58..aebdc2d4 100644 --- a/plugins/beta/datasets/src/registry/datasetRegistry.js +++ b/plugins/beta/datasets/src/registry/datasetRegistry.js @@ -5,9 +5,7 @@ const datasetRegistry = { attach (datasetsRef, orderedDatasetsRef, mapStyle) { this._datasets = datasetsRef this._orderedDatasets = orderedDatasetsRef - if (mapStyle) { - this._mapStyle = mapStyle - } + this.attachMapStyle(mapStyle) this._invalidateChangedDatasets() }, @@ -30,6 +28,12 @@ const datasetRegistry = { attachCreateDataset (createDataset) { this._createDataset = createDataset }, _createDataset: (datasetDefinition) => createDataset(datasetDefinition), + attachMapStyle (mapStyle) { + if (mapStyle) { + this._mapStyle = mapStyle + } + }, + get mapStyle () { return this._mapStyle }, From 9e4746b21004374be52e86bc47938ca0fcdef008 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Thu, 25 Jun 2026 17:03:34 +0100 Subject: [PATCH 03/14] IM-373 removed maplibreLayerAdapter's mapStyle dependency --- demo/js/esri-datasets.js | 2 +- .../src/adapters/esri/esriLayerAdapter.js | 4 +- .../adapters/maplibre/maplibreLayerAdapter.js | 58 +++++++------------ .../src/initialise/initialiseDatasets.js | 12 +--- 4 files changed, 25 insertions(+), 51 deletions(-) diff --git a/demo/js/esri-datasets.js b/demo/js/esri-datasets.js index a8200c8a..670728c7 100644 --- a/demo/js/esri-datasets.js +++ b/demo/js/esri-datasets.js @@ -145,5 +145,5 @@ const testGlobalVisibility = () => { } interactiveMap.on('datasets:ready', function () { - testGlobalVisibility() + // testGlobalVisibility() }) \ No newline at end of file diff --git a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js index 05e36b01..ebd917ea 100644 --- a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js +++ b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js @@ -1,10 +1,12 @@ import VectorTileLayer from '@arcgis/core/layers/VectorTileLayer.js' import GroupLayer from '@arcgis/core/layers/GroupLayer.js' +import { LayerAdapter } from '../layerAdapter.js' import { datasetRegistry } from '../../registry/datasetRegistry.js' import { EsriDataset } from './registry/esriDataset.js' -export default class EsriLayerAdapter { +export default class EsriLayerAdapter extends LayerAdapter { constructor (mapProvider, symbolRegistry, patternRegistry) { + super() this._mapProvider = mapProvider this._map = mapProvider.map // TODO: Implement symbolRegistry and patternRegistry usage in the adapter diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js index 2c4f82c8..dc214ad4 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js @@ -40,26 +40,16 @@ export default class MaplibreLayerAdapter extends LayerAdapter { // ─── Lifecycle ────────────────────────────────────────────────────────────── - assertMapStyle (methodName, mapStyle) { - if (mapStyle !== datasetRegistry.mapStyle) { - console.error(`MaplibreLayerAdapter.${methodName} has mapStyle ${mapStyle.id}, but datasetRegistry has ${datasetRegistry.mapStyle.id}.`) - } else { - console.log(`MaplibreLayerAdapter.${methodName} has mapStyle ${mapStyle.id}, which matches datasetRegistry.`) - } - } - /** * Initialise all datasets: register patterns, add layers, then wait for idle. - * @param {Object} mapStyle * @returns {Promise} Resolves once the map has processed all layers. */ - async init (mapStyle) { - this.assertMapStyle('init', mapStyle) + async init () { const { patternConfigs, symbolConfigs } = datasetRegistry.getPatternAndSymbolConfigs() - await this.addPatternsAndSymbolsToMap(patternConfigs, symbolConfigs, mapStyle) + await this.addPatternsAndSymbolsToMap(patternConfigs, symbolConfigs) this._symbolLayerIds.clear() - datasetRegistry.forEachDataset(registryDataset => this._addLayers(registryDataset, mapStyle)) + datasetRegistry.forEachDataset(registryDataset => this._addLayers(registryDataset)) await new Promise(resolve => this._map.once('idle', resolve)) } @@ -70,8 +60,8 @@ export default class MaplibreLayerAdapter extends LayerAdapter { this._symbolLayerIds.delete(layerId) } - async addPatternsAndSymbolsToMap (patterns, symbols, mapStyle) { - this.assertMapStyle('addPatternsAndSymbolsToMap', mapStyle) + async addPatternsAndSymbolsToMap (patterns, symbols) { + const mapStyle = datasetRegistry.mapStyle const mapStyleId = mapStyle.id return Promise.all([ this._mapProvider.addPatternsToMap(patterns, mapStyleId, this._patternRegistry), @@ -98,39 +88,35 @@ export default class MaplibreLayerAdapter extends LayerAdapter { /** * Re-register patterns and re-add all layers after a basemap style change, * then reapply cached dynamic source data and hidden-feature filters. - * @param {Object} newMapStyle - * @param {Map} dynamicSources - datasetId → dynamic source instance * @returns {Promise} */ - async onMapStyleChange (newMapStyle, dynamicSources) { - this.assertMapStyle('onMapStyleChange', newMapStyle) + async onMapStyleChange () { // MapLibre wipes all sources/layers on style change — must wait for idle first await new Promise(resolve => this._map.once('idle', resolve)) const { patternConfigs, symbolConfigs } = datasetRegistry.getPatternAndSymbolConfigs() - await this.addPatternsAndSymbolsToMap(patternConfigs, symbolConfigs, newMapStyle) + await this.addPatternsAndSymbolsToMap(patternConfigs, symbolConfigs) this._symbolLayerIds.clear() datasetRegistry.forEachDataset(registryDataset => { - this._addLayers(registryDataset, newMapStyle) + this._addLayers(registryDataset) this._applyFeatureFilter(registryDataset) }) // TODO: check dynamicSources still work // Re-push cached data for dynamic sources - dynamicSources.forEach(source => source.reapply()) + this.dynamicSources.forEach(source => source.reapply()) } /** * Re-register symbols at the new pixel ratio and update icon-image on all symbol layers. * Called when the map size changes so symbols are rasterised at the correct resolution. - * @param {Object} mapStyle * @returns {Promise} */ - async onMapSizeChange (mapStyle) { - this.assertMapStyle('onMapSizeChange', mapStyle) + async onMapSizeChange () { + const { mapStyle } = datasetRegistry const { patternConfigs, symbolConfigs } = datasetRegistry.getPatternAndSymbolConfigs() - await this.addPatternsAndSymbolsToMap(patternConfigs, symbolConfigs, mapStyle) + await this.addPatternsAndSymbolsToMap(patternConfigs, symbolConfigs) datasetRegistry.forEach(registryDataset => { const { fillLayerId, symbolLayerId } = registryDataset @@ -153,13 +139,11 @@ export default class MaplibreLayerAdapter extends LayerAdapter { /** * Add a single dataset's source and layers to the map. * @param {string} datasetId - * @param {Object} mapStyle */ - async addDataset (datasetId, mapStyle) { - this.assertMapStyle('addDataset', mapStyle) + async addDataset (datasetId) { const registryDataset = datasetRegistry.getDataset(datasetId) - await this.addPatternsAndSymbolsToMap(registryDataset.patternConfigs, registryDataset.symbolConfigs, mapStyle) - this._addLayers(registryDataset, mapStyle) + await this.addPatternsAndSymbolsToMap(registryDataset.patternConfigs, registryDataset.symbolConfigs) + this._addLayers(registryDataset) } /** @@ -207,15 +191,13 @@ export default class MaplibreLayerAdapter extends LayerAdapter { /** * Update a dataset's style and re-render all its layers. * @param {string} datasetId - Updated dataset (style changes already merged in) - * @param {Object} mapStyle * @returns {Promise} */ - async applyStyle (datasetId, mapStyle) { - this.assertMapStyle('applyStyle', mapStyle) + async applyStyle (datasetId) { const registryDataset = datasetRegistry.getDataset(datasetId) registryDataset.layerIds.forEach(layerId => this.removeLayer(layerId)) - await this.addPatternsAndSymbolsToMap(registryDataset.patternConfigs, registryDataset.symbolConfigs, mapStyle) - this._addLayers(registryDataset, mapStyle) + await this.addPatternsAndSymbolsToMap(registryDataset.patternConfigs, registryDataset.symbolConfigs) + this._addLayers(registryDataset) } /** @@ -269,8 +251,8 @@ export default class MaplibreLayerAdapter extends LayerAdapter { return this._mapProvider.map.getPixelRatio() } - _addLayers (registryDataset, mapStyle) { - this.assertMapStyle('_addLayers', mapStyle) + _addLayers (registryDataset) { + const { mapStyle } = datasetRegistry const sourceId = addDatasetLayers(this._map, registryDataset, mapStyle, this._symbolRegistry, this._patternRegistry, this._pixelRatio) this._datasetSourceMap.set(registryDataset.id, sourceId) this._maintainSymbolOrdering(registryDataset) diff --git a/plugins/beta/datasets/src/initialise/initialiseDatasets.js b/plugins/beta/datasets/src/initialise/initialiseDatasets.js index 18648380..ecd97f1f 100644 --- a/plugins/beta/datasets/src/initialise/initialiseDatasets.js +++ b/plugins/beta/datasets/src/initialise/initialiseDatasets.js @@ -41,30 +41,20 @@ export const initialiseDatasets = ({ }) dynamicSources.set(registryDataset.id, dynamicSource) }) + adapter.attachDynamicSources(dynamicSources) // TODO - apply dynamic source defaults here, and include in mappedDatasets dispatch({ type: 'SET_DATASETS', payload: { datasets: processedDatasets, mappedDatasets, orderedDatasets } }) eventBus.emit('datasets:ready') }) - // let currentMapStyle = mapStyle - - // Handle basemap style changes — delegate entirely to the adapter - // const onSetMapStyle = (newMapStyle) => { - // currentMapStyle = newMapStyle - // adapter.onMapStyleChange(newMapStyle, dynamicSources) - // } - const onMapSizeChange = () => { - // adapter.onMapSizeChange(currentMapStyle) adapter.onMapSizeChange() } - // eventBus.on(events.MAP_SET_STYLE, onSetMapStyle) eventBus.on(events.MAP_SIZE_CHANGE, onMapSizeChange) return { remove () { - // eventBus.off(events.MAP_SET_STYLE, onSetMapStyle) eventBus.off(events.MAP_SIZE_CHANGE, onMapSizeChange) // Clean up dynamic sources From 865e15cbe31f2d9a3d4af318389da578244fd112 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 26 Jun 2026 09:07:15 +0100 Subject: [PATCH 04/14] IM-373 initialiseDatasets.test.js updated to reflect new mapStyle method --- .../src/initialise/initialiseDatasets.test.js | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/plugins/beta/datasets/src/initialise/initialiseDatasets.test.js b/plugins/beta/datasets/src/initialise/initialiseDatasets.test.js index 46743d48..78302805 100644 --- a/plugins/beta/datasets/src/initialise/initialiseDatasets.test.js +++ b/plugins/beta/datasets/src/initialise/initialiseDatasets.test.js @@ -16,6 +16,7 @@ jest.mock('../registry/datasetRegistry.js', () => ({ const makeAdapter = (overrides = {}) => ({ init: jest.fn().mockResolvedValue(undefined), + attachDynamicSources: jest.fn(), onMapStyleChange: jest.fn(), onMapSizeChange: jest.fn(), setData: jest.fn(), @@ -42,7 +43,7 @@ const makeArgs = (overrides = {}) => { pluginStateRef: {}, mapStyle: { layers: [] }, mapProvider: { map: {} }, - events: { MAP_SET_STYLE: 'map:setStyle', MAP_SIZE_CHANGE: 'map:sizeChange' }, + events: { MAP_SIZE_CHANGE: 'map:sizeChange' }, dispatch: jest.fn(), eventBus, ...overrides @@ -94,10 +95,9 @@ describe('initialiseDatasets', () => { expect(args.dispatch).not.toHaveBeenCalledWith(expect.objectContaining({ type: 'SET_GLOBAL_STATE' })) }) - it('registers MAP_SET_STYLE and MAP_SIZE_CHANGE event listeners', () => { + it('registers MAP_SET_STYLE event listeners', () => { const args = makeArgs() initialiseDatasets(args) - expect(args.eventBus.on).toHaveBeenCalledWith('map:setStyle', expect.any(Function)) expect(args.eventBus.on).toHaveBeenCalledWith('map:sizeChange', expect.any(Function)) }) @@ -153,28 +153,11 @@ describe('initialiseDatasets', () => { // ─── event handlers ─────────────────────────────────────────────────────────── describe('event handlers', () => { - it('delegates MAP_SET_STYLE to adapter.onMapStyleChange', () => { - const args = makeArgs() - initialiseDatasets(args) - const newStyle = { layers: [{ id: 'new' }] } - args.eventBus._handlers['map:setStyle'](newStyle) - expect(args.adapter.onMapStyleChange).toHaveBeenCalledWith(newStyle, expect.any(Map)) - }) - it('delegates MAP_SIZE_CHANGE to adapter.onMapSizeChange with current mapStyle', () => { const args = makeArgs() initialiseDatasets(args) args.eventBus._handlers['map:sizeChange']() - expect(args.adapter.onMapSizeChange).toHaveBeenCalledWith(args.mapStyle) - }) - - it('uses the updated mapStyle after MAP_SET_STYLE', () => { - const args = makeArgs() - initialiseDatasets(args) - const newStyle = { layers: [{ id: 'updated' }] } - args.eventBus._handlers['map:setStyle'](newStyle) - args.eventBus._handlers['map:sizeChange']() - expect(args.adapter.onMapSizeChange).toHaveBeenCalledWith(newStyle) + expect(args.adapter.onMapSizeChange).toHaveBeenCalled() }) }) @@ -185,7 +168,6 @@ describe('returned API', () => { const args = makeArgs() const instance = initialiseDatasets(args) instance.remove() - expect(args.eventBus.off).toHaveBeenCalledWith('map:setStyle', expect.any(Function)) expect(args.eventBus.off).toHaveBeenCalledWith('map:sizeChange', expect.any(Function)) expect(args.adapter.destroy).toHaveBeenCalled() }) From 01052c5ac6e90d371d6bfb5dcae14dea39fe2ca2 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 26 Jun 2026 09:29:51 +0100 Subject: [PATCH 05/14] IM-373 fixed maplibreLayerAdapter.test.js --- .../src/adapters/maplibre/maplibreLayerAdapter.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js index 247d1b54..ab0bb242 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js @@ -42,9 +42,10 @@ const makeMapProvider = (map) => ({ const MAP_STYLE = { id: 'outdoor', layers: [] } let map, mapProvider, adapter +const dynamicSources = new Map() beforeEach(() => { - datasetRegistry.attachCreateDataset(def => new MapLibreDataset(def)) + datasetRegistry.attachMapStyle(MAP_STYLE) symbolRegistry.clear() symbolRegistry.initialise() patternRegistry.clear() @@ -53,6 +54,8 @@ beforeEach(() => { map = makeMap() mapProvider = makeMapProvider(map) adapter = new MaplibreLayerAdapter(mapProvider, symbolRegistry, patternRegistry) + adapter.attachDynamicSources(dynamicSources) + datasetRegistry.attachCreateDataset(adapter.createDataset) }) // ─── init ───────────────────────────────────────────────────────────────────── From bc651f638616d18899a32faa67919fd90d5a6858 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 26 Jun 2026 09:49:24 +0100 Subject: [PATCH 06/14] IM-373 maplibreLayerAdapter fully covered --- .../maplibre/maplibreLayerAdapter.test.js | 98 +++++++++++++------ 1 file changed, 69 insertions(+), 29 deletions(-) diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js index ab0bb242..7470c28f 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js @@ -62,26 +62,26 @@ beforeEach(() => { describe('init', () => { it('calls addPatternsAndSymbolsToMap before adding layers', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() expect(mapProvider.addPatternsToMap).toHaveBeenCalled() expect(mapProvider.addSymbolsToMap).toHaveBeenCalled() }) it('adds fill + stroke layers for existing-fields', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() expect(map.getLayer('existing-fields')).toMatchObject({ type: 'fill' }) expect(map.getLayer('existing-fields-stroke')).toMatchObject({ type: 'line' }) }) it('adds symbol layers for all historic-monuments sublayers', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() expect(map.getLayer('historic-monuments-prehistoric')).toMatchObject({ type: 'symbol' }) expect(map.getLayer('historic-monuments-roman')).toMatchObject({ type: 'symbol' }) expect(map.getLayer('historic-monuments-medieval')).toMatchObject({ type: 'symbol' }) }) it('adds fill + stroke layers for land-covers sublayers', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() expect(map.getLayer('land-covers-130-131')).toMatchObject({ type: 'fill' }) expect(map.getLayer('land-covers-130-131-stroke')).toMatchObject({ type: 'line' }) expect(map.getLayer('land-covers-332')).toMatchObject({ type: 'fill' }) @@ -90,49 +90,49 @@ describe('init', () => { }) it('adds a stroke-only layer for hedge-control', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() expect(map.getLayer('hedge-control')).toMatchObject({ type: 'line' }) }) it('adds a vector source for existing-fields', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() const ds = datasetRegistry.getDataset('existing-fields') expect(map.addSource).toHaveBeenCalledWith(ds.sourceId, expect.objectContaining({ type: 'vector' })) }) it('adds a geojson source for historic-monuments', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() const ds = datasetRegistry.getDataset('historic-monuments') expect(map.addSource).toHaveBeenCalledWith(ds.sourceId, expect.objectContaining({ type: 'geojson' })) }) it('adds a dynamic geojson source for land-covers', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() const ds = datasetRegistry.getDataset('land-covers') expect(map.addSource).toHaveBeenCalledWith(ds.sourceId, expect.objectContaining({ type: 'geojson' })) }) it('adds each source only once even when multiple sublayers share it', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() const ds = datasetRegistry.getDataset('historic-monuments') const calls = map.addSource.mock.calls.filter(([id]) => id === ds.sourceId) expect(calls).toHaveLength(1) }) it('waits for map idle before resolving', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() expect(map.once).toHaveBeenCalledWith('idle', expect.any(Function)) }) it('tracks symbol layers in _symbolLayerIds', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() expect(adapter._symbolLayerIds.has('historic-monuments-prehistoric')).toBe(true) expect(adapter._symbolLayerIds.has('historic-monuments-roman')).toBe(true) expect(adapter._symbolLayerIds.has('historic-monuments-medieval')).toBe(true) }) it('does not track fill/line layers in _symbolLayerIds', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() expect(adapter._symbolLayerIds.has('existing-fields')).toBe(false) expect(adapter._symbolLayerIds.has('land-covers-130-131')).toBe(false) expect(adapter._symbolLayerIds.has('hedge-control')).toBe(false) @@ -143,7 +143,7 @@ describe('init', () => { describe('removeLayer', () => { it('removes an existing layer from the map', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() adapter.removeLayer('existing-fields') expect(map.removeLayer).toHaveBeenCalledWith('existing-fields') expect(map.getLayer('existing-fields')).toBeNull() @@ -155,7 +155,7 @@ describe('removeLayer', () => { }) it('removes the layer id from _symbolLayerIds', async () => { - await adapter.init(MAP_STYLE) + await adapter.init() expect(adapter._symbolLayerIds.has('historic-monuments-prehistoric')).toBe(true) adapter.removeLayer('historic-monuments-prehistoric') expect(adapter._symbolLayerIds.has('historic-monuments-prehistoric')).toBe(false) @@ -165,7 +165,7 @@ describe('removeLayer', () => { // ─── destroy ────────────────────────────────────────────────────────────────── describe('destroy', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('removes all layers from the map', () => { adapter.destroy() @@ -181,6 +181,11 @@ describe('destroy', () => { adapter.destroy() expect(adapter._datasetSourceMap.size).toBe(0) }) + + it('does not throw when getStyle returns null (covers _getLayersUsingSource early return)', () => { + map.getStyle.mockReturnValue(null) + expect(() => adapter.destroy()).not.toThrow() + }) }) // ─── addDataset ─────────────────────────────────────────────────────────────── @@ -204,12 +209,18 @@ describe('addDataset', () => { expect(mapProvider.addPatternsToMap).toHaveBeenCalled() expect(mapProvider.addSymbolsToMap).toHaveBeenCalled() }) + + it('does not call moveLayer when getStyle returns no layers (covers _getFirstSymbolLayerId null branch)', async () => { + map.getStyle.mockReturnValue({}) + await adapter.addDataset('existing-fields') + expect(map.moveLayer).not.toHaveBeenCalled() + }) }) // ─── removeDataset ──────────────────────────────────────────────────────────── describe('removeDataset', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('removes all layers for existing-fields', () => { adapter.removeDataset('existing-fields') @@ -257,7 +268,7 @@ describe('removeDataset', () => { // ─── setData ────────────────────────────────────────────────────────────────── describe('setData', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('calls source.setData for a dynamic dataset (land-covers)', () => { const ds = datasetRegistry.getDataset('land-covers') @@ -280,7 +291,7 @@ describe('setData', () => { // ─── applyDatasetVisibility ─────────────────────────────────────────────────── describe('applyDatasetVisibility', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('sets visibility to "visible" for existing-fields layers', () => { adapter.applyDatasetVisibility('existing-fields') @@ -305,12 +316,19 @@ describe('applyDatasetVisibility', () => { adapter.applyDatasetVisibility('unknown') expect(map.setLayoutProperty.mock.calls.length).toBe(before) }) + + it('skips setLayoutProperty for a layer that has been removed from the map', () => { + map._layers.delete('existing-fields') + map.setLayoutProperty.mockClear() + adapter.applyDatasetVisibility('existing-fields') + expect(map.setLayoutProperty).not.toHaveBeenCalledWith('existing-fields', 'visibility', expect.any(String)) + }) }) // ─── applyGlobalVisibility ──────────────────────────────────────────────────── describe('applyGlobalVisibility', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('sets visibility on layers for all top-level datasets', () => { map.setLayoutProperty.mockClear() @@ -327,7 +345,7 @@ describe('applyGlobalVisibility', () => { // ─── applyFeatureFilter ─────────────────────────────────────────────────────── describe('applyFeatureFilter', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('sets filter on all land-covers sublayers (parent has hiddenFeatures: [42])', () => { adapter.applyFeatureFilter('land-covers') @@ -361,12 +379,19 @@ describe('applyFeatureFilter', () => { adapter.applyFeatureFilter('unknown') expect(map.setFilter).not.toHaveBeenCalled() }) + + it('skips setFilter for a sublayer whose layer has been removed from the map', () => { + map._layers.delete('land-covers-130-131') + map.setFilter.mockClear() + adapter.applyFeatureFilter('land-covers') + expect(map.setFilter).not.toHaveBeenCalledWith('land-covers-130-131', expect.anything()) + }) }) // ─── applyDatasetOpacity ────────────────────────────────────────────────────── describe('applyDatasetOpacity', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('sets fill-opacity on the fill layer for existing-fields', () => { adapter.applyDatasetOpacity('existing-fields') @@ -396,12 +421,19 @@ describe('applyDatasetOpacity', () => { adapter.applyDatasetOpacity('unknown') expect(map.setPaintProperty.mock.calls.length).toBe(before) }) + + it('does not call setPaintProperty for a layer that has been removed from the map (covers _setPaintOpacity early return)', () => { + map._layers.delete('existing-fields') + map.setPaintProperty.mockClear() + adapter.applyDatasetOpacity('existing-fields') + expect(map.setPaintProperty).not.toHaveBeenCalledWith('existing-fields', expect.any(String), expect.any(Number)) + }) }) // ─── applyGlobalOpacity ─────────────────────────────────────────────────────── describe('applyGlobalOpacity', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('sets paint opacity on layers for all datasets', () => { map.setPaintProperty.mockClear() @@ -418,7 +450,7 @@ describe('applyGlobalOpacity', () => { // ─── applyStyle ─────────────────────────────────────────────────────────────── describe('applyStyle', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('removes old fill + stroke layers then re-adds them for existing-fields', async () => { await adapter.applyStyle('existing-fields', MAP_STYLE) @@ -444,7 +476,7 @@ describe('applyStyle', () => { // ─── onMapStyleChange ───────────────────────────────────────────────────────── describe('onMapStyleChange', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('waits for map idle before proceeding', async () => { let idleCb @@ -483,11 +515,11 @@ describe('onMapStyleChange', () => { // ─── onMapSizeChange ────────────────────────────────────────────────────────── describe('onMapSizeChange', () => { - beforeEach(async () => { await adapter.init(MAP_STYLE) }) + beforeEach(async () => { await adapter.init() }) it('updates icon-image layout property for historic-monuments symbol layers', async () => { map.setLayoutProperty.mockClear() - await adapter.onMapSizeChange(MAP_STYLE) + await adapter.onMapSizeChange() const symbolUpdates = map.setLayoutProperty.mock.calls.filter(([, prop]) => prop === 'icon-image') const ids = symbolUpdates.map(([id]) => id) expect(ids).toContain('historic-monuments-prehistoric') @@ -497,7 +529,7 @@ describe('onMapSizeChange', () => { it('updates fill-pattern paint property for land-covers sublayers', async () => { map.setPaintProperty.mockClear() - await adapter.onMapSizeChange(MAP_STYLE) + await adapter.onMapSizeChange() const patternUpdates = map.setPaintProperty.mock.calls.filter(([, prop]) => prop === 'fill-pattern') const ids = patternUpdates.map(([id]) => id) expect(ids).toContain('land-covers-130-131') @@ -506,14 +538,22 @@ describe('onMapSizeChange', () => { it('does not update fill-pattern for existing-fields (no fillPattern style)', async () => { map.setPaintProperty.mockClear() - await adapter.onMapSizeChange(MAP_STYLE) + await adapter.onMapSizeChange() const patternUpdates = map.setPaintProperty.mock.calls.filter(([, prop]) => prop === 'fill-pattern') expect(patternUpdates.map(([id]) => id)).not.toContain('existing-fields') }) it('calls addPatternsAndSymbolsToMap', async () => { mapProvider.addPatternsToMap.mockClear() - await adapter.onMapSizeChange(MAP_STYLE) + await adapter.onMapSizeChange() expect(mapProvider.addPatternsToMap).toHaveBeenCalled() }) + + it('does not call setLayoutProperty for icon-image when getSymbolImageId returns null', async () => { + jest.spyOn(symbolRegistry, 'getSymbolImageId').mockReturnValue(null) + map.setLayoutProperty.mockClear() + await adapter.onMapSizeChange() + const symbolUpdates = map.setLayoutProperty.mock.calls.filter(([, prop]) => prop === 'icon-image') + expect(symbolUpdates).toHaveLength(0) + }) }) From 4552bd233771776632b4d099f0e523fd368152b1 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 26 Jun 2026 09:56:10 +0100 Subject: [PATCH 07/14] IM-373 mapLibreDataset fully covered --- .../maplibre/registry/mapLibreDataset.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugins/beta/datasets/src/adapters/maplibre/registry/mapLibreDataset.test.js b/plugins/beta/datasets/src/adapters/maplibre/registry/mapLibreDataset.test.js index d2a9c1cb..5d6adb4a 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/registry/mapLibreDataset.test.js +++ b/plugins/beta/datasets/src/adapters/maplibre/registry/mapLibreDataset.test.js @@ -309,6 +309,14 @@ describe('MapLibreDataset', () => { expect(datasetRegistry.getDataset('ds-string-with-prop').source).toEqual(expect.objectContaining({ type: 'geojson' })) expect(logger.warn).not.toHaveBeenCalled() }) + + it('adds promoteId keyed by sourceLayer when a vector source has both idProperty and sourceLayer', () => { + datasetRegistry.mockExtend({ 'ds-tiles-id-prop': { id: 'ds-tiles-id-prop', tiles: ['https://example.com/{z}/{x}/{y}'], idProperty: 'myId', sourceLayer: 'my-layer' } }) + expect(datasetRegistry.getDataset('ds-tiles-id-prop').source).toEqual(expect.objectContaining({ + type: 'vector', + promoteId: { 'my-layer': 'myId' } + })) + }) }) describe('getSymbolSource', () => { @@ -438,6 +446,10 @@ describe('MapLibreDataset', () => { expect(datasetRegistry.getDataset('land-covers')._hiddenFeaturesIdExpression).toEqual(['to-string', ['get', 'id']]) }) + it('uses get(idProperty) for a non-dynamic dataset with idProperty set', () => { + expect(datasetRegistry.getDataset('ds-no-transform')._hiddenFeaturesIdExpression).toEqual(['to-string', ['get', 'id']]) + }) + it('uses the feature id when idProperty is not set', () => { expect(datasetRegistry.getDataset('ds-bare')._hiddenFeaturesIdExpression).toEqual(['to-string', ['id']]) }) From c1c58ad86873cd7ded3128aad40b77c99df5b97f Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 26 Jun 2026 10:19:17 +0100 Subject: [PATCH 08/14] IM-373 covered layerBuilders --- .../adapters/maplibre/layerBuilders.test.js | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 plugins/beta/datasets/src/adapters/maplibre/layerBuilders.test.js diff --git a/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.test.js b/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.test.js new file mode 100644 index 00000000..6d3fdd8e --- /dev/null +++ b/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.test.js @@ -0,0 +1,83 @@ +import { addFillLayer, addStrokeLayer, addSymbolLayer } from './layerBuilders.js' + +// ─── helpers ────────────────────────────────────────────────────────────────── + +const makeMap = () => { + const layers = new Map() + return { + getLayer: jest.fn(id => layers.get(id) ?? null), + getSource: jest.fn(() => null), + addLayer: jest.fn(spec => layers.set(spec.id, spec)), + addSource: jest.fn() + } +} + +const makeDataset = (overrides = {}) => ({ + id: 'test-ds', + hasFill: false, + fillLayerId: null, + hasStroke: false, + strokeLayerId: null, + hasSymbol: false, + symbolLayerId: null, + style: {}, + opacity: 1, + getFillSource: jest.fn(paint => ({ id: 'test-ds', type: 'fill', paint })), + getStrokeSource: jest.fn(paint => ({ id: 'test-ds-stroke', type: 'line', paint })), + getSymbolSource: jest.fn((imageId, anchor) => ({ id: 'test-ds', type: 'symbol', layout: { 'icon-image': imageId } })), + ...overrides +}) + +// ─── addFillLayer ───────────────────────────────────────────────────────────── + +describe('addFillLayer', () => { + it('uses pixelRatio = 1 when the argument is omitted', () => { + const map = makeMap() + const patternRegistry = { getPatternImageId: jest.fn(() => null) } + const ds = makeDataset({ hasFill: true, fillLayerId: 'test-ds', style: { fill: '#ff0000' } }) + addFillLayer(map, ds, 'outdoor', patternRegistry) // no pixelRatio argument + expect(patternRegistry.getPatternImageId).toHaveBeenCalledWith(ds.style, 'outdoor', 1) + }) +}) + +// ─── addStrokeLayer ─────────────────────────────────────────────────────────── + +describe('addStrokeLayer', () => { + it('defaults line-width to 1 when strokeWidth is not set on the style', () => { + const map = makeMap() + const ds = makeDataset({ hasStroke: true, strokeLayerId: 'test-ds-stroke', style: { stroke: '#000000' } }) + addStrokeLayer(map, ds, 'outdoor') + expect(ds.getStrokeSource).toHaveBeenCalledWith(expect.objectContaining({ 'line-width': 1 })) + }) + + it('includes line-dasharray in the paint when strokeDashArray is set', () => { + const map = makeMap() + const ds = makeDataset({ + hasStroke: true, + strokeLayerId: 'test-ds-stroke', + style: { stroke: '#000000', strokeWidth: 2, strokeDashArray: [4, 2] } + }) + addStrokeLayer(map, ds, 'outdoor') + expect(ds.getStrokeSource).toHaveBeenCalledWith(expect.objectContaining({ 'line-dasharray': [4, 2] })) + }) +}) + +// ─── addSymbolLayer ─────────────────────────────────────────────────────────── + +describe('addSymbolLayer', () => { + it('returns early without adding a layer when symbolDef is null', () => { + const map = makeMap() + const symbolRegistry = { getSymbolDef: jest.fn(() => null), getSymbolImageId: jest.fn() } + const ds = makeDataset({ hasSymbol: true, symbolLayerId: 'test-ds', style: {} }) + addSymbolLayer(map, ds, { id: 'outdoor' }, symbolRegistry, 1) + expect(map.addLayer).not.toHaveBeenCalled() + }) + + it('returns early without adding a layer when imageId is null', () => { + const map = makeMap() + const symbolRegistry = { getSymbolDef: jest.fn(() => ({})), getSymbolImageId: jest.fn(() => null) } + const ds = makeDataset({ hasSymbol: true, symbolLayerId: 'test-ds', style: {} }) + addSymbolLayer(map, ds, { id: 'outdoor' }, symbolRegistry, 1) + expect(map.addLayer).not.toHaveBeenCalled() + }) +}) From 2576fcc1e538b4cfd3faf7e5e78ae531f5ddc246 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 26 Jun 2026 13:19:56 +0100 Subject: [PATCH 09/14] IM-373 covered esriDataset --- .../src/adapters/esri/esriLayerAdapter.js | 16 --- .../esri/registry/esriDataset.test.js | 118 ++++++++++++++++++ 2 files changed, 118 insertions(+), 16 deletions(-) create mode 100644 plugins/beta/datasets/src/adapters/esri/registry/esriDataset.test.js diff --git a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js index ebd917ea..0869642b 100644 --- a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js +++ b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js @@ -45,22 +45,6 @@ export default class EsriLayerAdapter extends LayerAdapter { // Finally show all layers that are visible based on the dataset/mapStyle visibility await Promise.all(topLevelDatasets.map(registryDataset => this.applyDatasetVisibility(registryDataset.id))) - - // Log the layers for debugging - // this.logLayers('_vectorTileLayers') - // console.log('this._vectorTileOpacityLayers', this._vectorTileOpacityLayers) - // console.log('this._groupLayers', this._groupLayers) - } - - logLayers (name) { - const obj = this[name] - console.log(`${name}:`) - Object.entries(obj).forEach(([datasetId, layer]) => { - console.log(` ${datasetId}`) - layer.styleRepository.layers.forEach(styleLayer => { - console.log(` ${styleLayer.id}`) - }) - }) } _addGroupLayer (esriGroupId) { diff --git a/plugins/beta/datasets/src/adapters/esri/registry/esriDataset.test.js b/plugins/beta/datasets/src/adapters/esri/registry/esriDataset.test.js new file mode 100644 index 00000000..c5f890ca --- /dev/null +++ b/plugins/beta/datasets/src/adapters/esri/registry/esriDataset.test.js @@ -0,0 +1,118 @@ +import { EsriDataset } from './esriDataset.js' +import { datasetRegistry } from '../../../registry/datasetRegistry.js' + +jest.mock('../../../registry/datasetRegistry.js') + +const MAP_STYLE = { id: 'outdoor' } + +describe('EsriDataset', () => { + beforeEach(() => { + datasetRegistry.attachMapStyle(MAP_STYLE) + datasetRegistry.attachCreateDataset(def => new EsriDataset(def)) + datasetRegistry.mockExtend({ + // applyLayerPaintProperties + 'esri-fill-stroke': { id: 'esri-fill-stroke', style: { fill: '#ff0000', stroke: '#0000ff' } }, + 'esri-stroke-only': { id: 'esri-stroke-only', style: { stroke: '#0000ff' } }, + 'esri-fill-only': { id: 'esri-fill-only', style: { fill: '#ff0000' } }, + 'esri-bare': { id: 'esri-bare' }, + // esriGroupId — parent owns the id, child inherits it + 'esri-group': { id: 'esri-group', esriGroupId: 'group-123', sublayerIds: ['esri-child'] }, + 'esri-child': { id: 'esri-child', parentId: 'esri-group' }, + // esriGroupId — explicitly set to null (distinct from undefined) + 'esri-group-null': { id: 'esri-group-null', esriGroupId: null }, + // useServerStyle + 'esri-server-style': { id: 'esri-server-style', esriUseServerStyle: true }, + 'esri-no-server': { id: 'esri-no-server' } + }) + }) + + // ─── applyLayerPaintProperties ────────────────────────────────────────────── + + describe('applyLayerPaintProperties', () => { + it('adds line-color when the dataset has a stroke style', () => { + const ds = datasetRegistry.getDataset('esri-stroke-only') + const paint = {} + ds.applyLayerPaintProperties(paint) + expect(paint['line-color']).toBeDefined() + }) + + it('resolves line-color against the current map style id', () => { + const ds = datasetRegistry.getDataset('esri-stroke-only') + const paint = {} + ds.applyLayerPaintProperties(paint) + expect(paint['line-color']).toBe('#0000ff') + }) + + it('adds fill-color when the dataset has a fill style', () => { + const ds = datasetRegistry.getDataset('esri-fill-only') + const paint = {} + ds.applyLayerPaintProperties(paint) + expect(paint['fill-color']).toBeDefined() + }) + + it('resolves fill-color against the current map style id', () => { + const ds = datasetRegistry.getDataset('esri-fill-only') + const paint = {} + ds.applyLayerPaintProperties(paint) + expect(paint['fill-color']).toBe('#ff0000') + }) + + it('adds both line-color and fill-color when the dataset has stroke and fill', () => { + const ds = datasetRegistry.getDataset('esri-fill-stroke') + const paint = {} + ds.applyLayerPaintProperties(paint) + expect(paint['line-color']).toBe('#0000ff') + expect(paint['fill-color']).toBe('#ff0000') + }) + + it('does not add line-color or fill-color when the dataset has neither', () => { + const ds = datasetRegistry.getDataset('esri-bare') + const paint = {} + ds.applyLayerPaintProperties(paint) + expect(paint['line-color']).toBeUndefined() + expect(paint['fill-color']).toBeUndefined() + }) + + it('returns the paint object', () => { + const ds = datasetRegistry.getDataset('esri-fill-stroke') + const paint = { existing: true } + expect(ds.applyLayerPaintProperties(paint)).toBe(paint) + }) + }) + + // ─── esriGroupId ──────────────────────────────────────────────────────────── + + describe('esriGroupId', () => { + it('returns the esriGroupId from the dataset definition when set', () => { + const ds = datasetRegistry.getDataset('esri-group') + expect(ds.esriGroupId).toBe('group-123') + }) + + it('returns null when esriGroupId is explicitly null', () => { + const ds = datasetRegistry.getDataset('esri-group-null') + expect(ds.esriGroupId).toBeNull() + }) + + it('inherits esriGroupId from the parent when own esriGroupId is undefined', () => { + const child = datasetRegistry.getDataset('esri-child') + expect(child.esriGroupId).toBe('group-123') + }) + + it('returns undefined when esriGroupId is not set and there is no parent', () => { + const ds = datasetRegistry.getDataset('esri-bare') + expect(ds.esriGroupId).toBeUndefined() + }) + }) + + // ─── useServerStyle ───────────────────────────────────────────────────────── + + describe('useServerStyle', () => { + it('returns true when esriUseServerStyle is true', () => { + expect(datasetRegistry.getDataset('esri-server-style').useServerStyle).toBe(true) + }) + + it('returns false when esriUseServerStyle is not set', () => { + expect(datasetRegistry.getDataset('esri-no-server').useServerStyle).toBe(false) + }) + }) +}) From eed82d48e7c85ce9c46d0e91e1e90c1b248ca3bb Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 26 Jun 2026 13:36:13 +0100 Subject: [PATCH 10/14] IM-373 removed some mocks in esriDataset.test.js --- .../esri/registry/esriDataset.test.js | 55 ++----------------- 1 file changed, 5 insertions(+), 50 deletions(-) diff --git a/plugins/beta/datasets/src/adapters/esri/registry/esriDataset.test.js b/plugins/beta/datasets/src/adapters/esri/registry/esriDataset.test.js index c5f890ca..206c868f 100644 --- a/plugins/beta/datasets/src/adapters/esri/registry/esriDataset.test.js +++ b/plugins/beta/datasets/src/adapters/esri/registry/esriDataset.test.js @@ -11,58 +11,24 @@ describe('EsriDataset', () => { datasetRegistry.attachCreateDataset(def => new EsriDataset(def)) datasetRegistry.mockExtend({ // applyLayerPaintProperties - 'esri-fill-stroke': { id: 'esri-fill-stroke', style: { fill: '#ff0000', stroke: '#0000ff' } }, - 'esri-stroke-only': { id: 'esri-stroke-only', style: { stroke: '#0000ff' } }, - 'esri-fill-only': { id: 'esri-fill-only', style: { fill: '#ff0000' } }, 'esri-bare': { id: 'esri-bare' }, // esriGroupId — parent owns the id, child inherits it 'esri-group': { id: 'esri-group', esriGroupId: 'group-123', sublayerIds: ['esri-child'] }, 'esri-child': { id: 'esri-child', parentId: 'esri-group' }, - // esriGroupId — explicitly set to null (distinct from undefined) - 'esri-group-null': { id: 'esri-group-null', esriGroupId: null }, // useServerStyle - 'esri-server-style': { id: 'esri-server-style', esriUseServerStyle: true }, - 'esri-no-server': { id: 'esri-no-server' } + 'esri-server-style': { id: 'esri-server-style', esriUseServerStyle: true } }) }) // ─── applyLayerPaintProperties ────────────────────────────────────────────── describe('applyLayerPaintProperties', () => { - it('adds line-color when the dataset has a stroke style', () => { - const ds = datasetRegistry.getDataset('esri-stroke-only') - const paint = {} - ds.applyLayerPaintProperties(paint) - expect(paint['line-color']).toBeDefined() - }) - - it('resolves line-color against the current map style id', () => { - const ds = datasetRegistry.getDataset('esri-stroke-only') - const paint = {} - ds.applyLayerPaintProperties(paint) - expect(paint['line-color']).toBe('#0000ff') - }) - - it('adds fill-color when the dataset has a fill style', () => { - const ds = datasetRegistry.getDataset('esri-fill-only') - const paint = {} - ds.applyLayerPaintProperties(paint) - expect(paint['fill-color']).toBeDefined() - }) - - it('resolves fill-color against the current map style id', () => { - const ds = datasetRegistry.getDataset('esri-fill-only') - const paint = {} - ds.applyLayerPaintProperties(paint) - expect(paint['fill-color']).toBe('#ff0000') - }) - it('adds both line-color and fill-color when the dataset has stroke and fill', () => { - const ds = datasetRegistry.getDataset('esri-fill-stroke') + const ds = datasetRegistry.getDataset('land-covers-other') const paint = {} ds.applyLayerPaintProperties(paint) - expect(paint['line-color']).toBe('#0000ff') - expect(paint['fill-color']).toBe('#ff0000') + expect(paint['line-color']).toBe('#1565C0') + expect(paint['fill-color']).toBe('rgba(0,0,255,0.1)') }) it('does not add line-color or fill-color when the dataset has neither', () => { @@ -72,12 +38,6 @@ describe('EsriDataset', () => { expect(paint['line-color']).toBeUndefined() expect(paint['fill-color']).toBeUndefined() }) - - it('returns the paint object', () => { - const ds = datasetRegistry.getDataset('esri-fill-stroke') - const paint = { existing: true } - expect(ds.applyLayerPaintProperties(paint)).toBe(paint) - }) }) // ─── esriGroupId ──────────────────────────────────────────────────────────── @@ -88,11 +48,6 @@ describe('EsriDataset', () => { expect(ds.esriGroupId).toBe('group-123') }) - it('returns null when esriGroupId is explicitly null', () => { - const ds = datasetRegistry.getDataset('esri-group-null') - expect(ds.esriGroupId).toBeNull() - }) - it('inherits esriGroupId from the parent when own esriGroupId is undefined', () => { const child = datasetRegistry.getDataset('esri-child') expect(child.esriGroupId).toBe('group-123') @@ -112,7 +67,7 @@ describe('EsriDataset', () => { }) it('returns false when esriUseServerStyle is not set', () => { - expect(datasetRegistry.getDataset('esri-no-server').useServerStyle).toBe(false) + expect(datasetRegistry.getDataset('land-covers-other').useServerStyle).toBe(false) }) }) }) From 7316836493afd3645e30b07dc7ea0e56610e466f Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Wed, 8 Jul 2026 11:44:00 +0100 Subject: [PATCH 11/14] IM-373 esriLayerAdapter method signatures match mapLibres --- .../src/adapters/esri/esriLayerAdapter.js | 21 +- .../adapters/esri/esriLayerAdapter.test.js | 266 ++++++++++++++++++ .../datasets/src/initialise/DatasetsInit.jsx | 2 +- .../src/reducers/__data__/esriDatasets.js | 144 ++++++++++ .../src/registry/__mocks__/datasetRegistry.js | 9 + plugins/beta/datasets/src/registry/dataset.js | 2 +- 6 files changed, 428 insertions(+), 16 deletions(-) create mode 100644 plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js create mode 100644 plugins/beta/datasets/src/reducers/__data__/esriDatasets.js diff --git a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js index 0869642b..f9303757 100644 --- a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js +++ b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.js @@ -29,16 +29,16 @@ export default class EsriLayerAdapter extends LayerAdapter { return new EsriDataset(datasetDefinition) } - async init (mapStyle) { + async init () { const topLevelDatasets = datasetRegistry.topLevelDatasets() // ensure the datasets are added in order for await (const registryDataset of topLevelDatasets) { - await this._addLayers(registryDataset, mapStyle) + await this._addLayers(registryDataset) } - // onMapStyleChange: handles showing and hiding sublayers based on the mapStyle + // onMapStyleChange: handles showing and hiding sublayers based on the current mapStyle // and updating the paint properties of the layers based on the dataset/mapStyle style - await this.onMapStyleChange(datasetRegistry.mapStyle, null) + await this.onMapStyleChange() // Apply opacity to all layers await this.applyGlobalOpacity() @@ -61,7 +61,7 @@ export default class EsriLayerAdapter extends LayerAdapter { return this._groupLayers[esriGroupId] } - async _addLayers (registryDataset, mapStyle) { + async _addLayers (registryDataset) { const { esriGroupId } = registryDataset const vectorTileParent = esriGroupId ? this._addGroupLayer(esriGroupId) : this._map const vectorTileLayer = new VectorTileLayer({ @@ -149,14 +149,7 @@ export default class EsriLayerAdapter extends LayerAdapter { console.log('TODO: applyFeatureFilter', args) } - async onMapStyleChange (newMapStyle, dynamicSources) { - if (datasetRegistry.mapStyle.id !== newMapStyle.id) { - // Ensure the datasetRegistry is aware of the new mapStyle so that the visibility and style properties of the datasets can be correctly applied - // TODO - this is a bit of a hack, we should probably have a better way to handle this - // such as having DatasetsInit listen for a mapStyle change event and then call datasetRegistry.attach with the new mapStyle - // and finally trigger this method - datasetRegistry.attach(datasetRegistry.datasets, datasetRegistry._orderedDatasets, newMapStyle) - } + async onMapStyleChange () { datasetRegistry.forEach(registryDataset => { const { id, isSublayer, esriStyleLayerId, parent } = registryDataset const vectorTileLayer = this._vectorTileLayers[isSublayer ? parent.id : id] @@ -179,5 +172,5 @@ export default class EsriLayerAdapter extends LayerAdapter { } // onMapSizeChange is not applicable to the esriLayerAdapter - async onMapSizeChange (_mapStyle) {} + async onMapSizeChange () {} } diff --git a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js new file mode 100644 index 00000000..8b40e490 --- /dev/null +++ b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js @@ -0,0 +1,266 @@ +import { EsriDataset } from './registry/esriDataset.js' +import EsriLayerAdapter from './esriLayerAdapter.js' +import { datasetRegistry } from '../../registry/datasetRegistry.js' + +jest.mock('../../registry/datasetRegistry.js') +jest.mock('@arcgis/core/layers/VectorTileLayer.js', () => + jest.fn().mockImplementation((opts = {}) => ({ + ...opts, + add: jest.fn(), + when: jest.fn().mockResolvedValue(undefined), + setStyleLayerVisibility: jest.fn(), + getPaintProperties: jest.fn().mockReturnValue({}), + setPaintProperties: jest.fn() + })) +) +jest.mock('@arcgis/core/layers/GroupLayer.js', () => + jest.fn().mockImplementation((opts = {}) => ({ + ...opts, + add: jest.fn() + })) +) + +// Uncovered: 80-88,124,145-149 + +const MAP_STYLE = { id: 'outdoor' } +const makeMap = () => { + const _added = {} + return { + add: jest.fn(({ id }) => { + // Ensure that the same layer is never added twice + expect(_added[id]).toBeUndefined() + _added[id] = true + }) + } +} +const makeMapProvider = (map) => ({ map }) + +describe('esriLayerAdapter', () => { + let map, mapProvider, adapter + + beforeEach(() => { + datasetRegistry.useEsriDatasets() + datasetRegistry.attachMapStyle(MAP_STYLE) + datasetRegistry.attachCreateDataset(def => new EsriDataset(def)) + map = makeMap() + mapProvider = makeMapProvider(map) + adapter = new EsriLayerAdapter(mapProvider, null, null) + }) + + // ─── createDataset ─────────────────────────────────────────────────────────── + + describe('createDataset', () => { + it('returns an EsriDataset instance', () => { + expect(adapter.createDataset({ id: 'test' })).toBeInstanceOf(EsriDataset) + }) + }) + + // ─── _addLayers ────────────────────────────────────────────────────────────── + + // describe('_addLayers', () => { + // it('stores the VectorTileLayer in _vectorTileLayers', async () => { + // await adapter._addLayers(datasetRegistry.getDataset('esri-standalone'), MAP_STYLE) + // expect(adapter._vectorTileLayers['esri-standalone']).toBeDefined() + // }) + + // it('adds the VectorTileLayer directly to the map when there is no esriGroupId', async () => { + // const ds = datasetRegistry.getDataset('esri-standalone') + // await adapter._addLayers(ds, MAP_STYLE) + // expect(map.add).toHaveBeenCalledWith(adapter._vectorTileLayers['esri-standalone']) + // }) + + // it('stores the VectorTileLayer as the opacity layer when standalone', async () => { + // const ds = datasetRegistry.getDataset('esri-standalone') + // await adapter._addLayers(ds, MAP_STYLE) + // expect(adapter._vectorTileOpacityLayers['esri-standalone']).toBe(adapter._vectorTileLayers['esri-standalone']) + // }) + + // it('adds the VectorTileLayer inside a GroupLayer when esriGroupId is set', async () => { + // const ds = datasetRegistry.getDataset('esri-grouped') + // await adapter._addLayers(ds, MAP_STYLE) + // const gl = adapter._groupLayers['my-group'] + // expect(gl.add).toHaveBeenCalledWith(adapter._vectorTileLayers['esri-grouped']) + // }) + + // it('stores the GroupLayer as the opacity layer when esriGroupId is set', async () => { + // const ds = datasetRegistry.getDataset('esri-grouped') + // await adapter._addLayers(ds, MAP_STYLE) + // expect(adapter._vectorTileOpacityLayers['esri-grouped']).toBe(adapter._groupLayers['my-group']) + // }) + // }) + + // ─── _applyRegistryDatasetVisibility ───────────────────────────────────────── + + describe.skip('_applyRegistryDatasetVisibility', () => { + it('calls setStyleLayerVisibility on the parent VectorTileLayer for a sublayer', async () => { + await adapter._addLayers(datasetRegistry.getDataset('esri-parent'), MAP_STYLE) + const subA = datasetRegistry.getDataset('esri-parent-sub-a') + adapter._applyRegistryDatasetVisibility(subA) + expect(adapter._vectorTileLayers['esri-parent'].setStyleLayerVisibility) + .toHaveBeenCalledWith('style-sub-a', subA.visibility) + }) + + it('sets vectorTileLayer.visible to true for a visible top-level dataset', async () => { + const ds = datasetRegistry.getDataset('esri-standalone') + await adapter._addLayers(ds, MAP_STYLE) + adapter._applyRegistryDatasetVisibility(ds) + expect(adapter._vectorTileLayers['esri-standalone'].visible).toBe(true) + }) + + it('sets vectorTileLayer.visible to false for a hidden top-level dataset', async () => { + const ds = datasetRegistry.getDataset('esri-grouped') + await adapter._addLayers(ds, MAP_STYLE) + adapter._applyRegistryDatasetVisibility(ds) + expect(adapter._vectorTileLayers['esri-grouped'].visible).toBe(false) + }) + + it('applies sublayer style visibility when the top-level dataset is visible', async () => { + await adapter._addLayers(datasetRegistry.getDataset('esri-parent'), MAP_STYLE) + const parent = datasetRegistry.getDataset('esri-parent') + adapter._applyRegistryDatasetVisibility(parent) + const vtl = adapter._vectorTileLayers['esri-parent'] + expect(vtl.setStyleLayerVisibility).toHaveBeenCalledWith('style-sub-a', 'visible') + expect(vtl.setStyleLayerVisibility).toHaveBeenCalledWith('style-sub-b', 'none') + }) + + it('skips sublayer style visibility when the top-level dataset is not visible', async () => { + const ds = datasetRegistry.getDataset('esri-grouped') + await adapter._addLayers(ds, MAP_STYLE) + adapter._applyRegistryDatasetVisibility(ds) + expect(adapter._vectorTileLayers['esri-grouped'].setStyleLayerVisibility).not.toHaveBeenCalled() + }) + }) + + // ─── applyDatasetVisibility ────────────────────────────────────────────────── + + describe('applyDatasetVisibility', () => { + beforeEach(async () => { + const standAlone = datasetRegistry.getDataset('esri-standalone') + await adapter._addLayers(standAlone, MAP_STYLE) + }) + + it('applies visibility for a known dataset', async () => { + await adapter.applyDatasetVisibility('esri-standalone') + expect(adapter._vectorTileLayers['esri-standalone'].visible).toBe(true) + }) + + it('does nothing for an unknown dataset', async () => { + await expect(adapter.applyDatasetVisibility('unknown')).resolves.not.toThrow() + }) + }) + + // ─── applyGlobalVisibility ─────────────────────────────────────────────────── + + describe('applyGlobalVisibility', () => { + it.skip('applies visibility to all known datasets', async () => { + await adapter._addLayers(datasetRegistry.getDataset('esri-standalone'), MAP_STYLE) + await adapter.applyGlobalVisibility() + expect(adapter._vectorTileLayers['esri-standalone'].visible).toBe(true) + }) + }) + + // ─── applyDatasetOpacity ───────────────────────────────────────────────────── + + describe('applyDatasetOpacity', () => { + it('sets opacity on the opacity layer for a known dataset', async () => { + await adapter._addLayers(datasetRegistry.getDataset('esri-standalone'), MAP_STYLE) + await adapter.applyDatasetOpacity('esri-standalone') + expect(adapter._vectorTileOpacityLayers['esri-standalone'].opacity) + .toBe(datasetRegistry.getDataset('esri-standalone').opacity) + }) + + it('does nothing when the dataset has no opacity layer', async () => { + await expect(adapter.applyDatasetOpacity('esri-standalone')).resolves.not.toThrow() + }) + + it('does nothing for an unknown dataset', async () => { + await adapter._addLayers(datasetRegistry.getDataset('esri-standalone'), MAP_STYLE) + await expect(adapter.applyDatasetOpacity('unknown')).resolves.not.toThrow() + }) + }) + + // ─── applyGlobalOpacity ────────────────────────────────────────────────────── + + describe('applyGlobalOpacity', () => { + it('sets opacity on all opacity layers', async () => { + await adapter._addLayers(datasetRegistry.getDataset('esri-standalone'), MAP_STYLE) + await adapter.applyGlobalOpacity() + expect(adapter._vectorTileOpacityLayers['esri-standalone'].opacity) + .toBe(datasetRegistry.getDataset('esri-standalone').opacity) + }) + + it('skips entries whose dataset is not in the registry', async () => { + adapter._vectorTileOpacityLayers['ghost-id'] = { opacity: 99 } + await expect(adapter.applyGlobalOpacity()).resolves.not.toThrow() + expect(adapter._vectorTileOpacityLayers['ghost-id'].opacity).toBe(99) + }) + }) + + // ─── onMapStyleChange ──────────────────────────────────────────────────────── + + describe('onMapStyleChange', () => { + beforeEach(async () => { + await adapter._addLayers(datasetRegistry.getDataset('esri-standalone'), MAP_STYLE) + await adapter._addLayers(datasetRegistry.getDataset('esri-server'), MAP_STYLE) + }) + + it('calls setStyleLayerVisibility for datasets with esriStyleLayerId', async () => { + await adapter.onMapStyleChange(MAP_STYLE, null) + expect(adapter._vectorTileLayers['esri-standalone'].setStyleLayerVisibility) + .toHaveBeenCalledWith('standalone-style', expect.any(String)) + }) + + it('calls setPaintProperties for datasets not using server style', async () => { + await adapter.onMapStyleChange(MAP_STYLE, null) + expect(adapter._vectorTileLayers['esri-standalone'].setPaintProperties) + .toHaveBeenCalledWith('standalone-style', expect.any(Object)) + }) + + it('does not call setPaintProperties when useServerStyle is true', async () => { + await adapter.onMapStyleChange(MAP_STYLE, null) + expect(adapter._vectorTileLayers['esri-server'].setPaintProperties).not.toHaveBeenCalled() + }) + + // it('does not call setPaintProperties when getPaintProperties returns null', async () => { + // const vtl = adapter._vectorTileLayers['esri-standalone'] + // vtl.getPaintProperties.mockReturnValueOnce(null) + // await adapter.onMapStyleChange(MAP_STYLE, null) + // expect(vtl.setPaintProperties).not.toHaveBeenCalled() + // }) + }) + + // ─── init ──────────────────────────────────────────────────────────────────── + + describe('init', () => { + beforeEach(async () => { + await adapter.init() + }) + + it('adds VectorTileLayers for all top-level datasets', async () => { + expect(adapter._vectorTileLayers['flood-zones-cc']).toBeDefined() + expect(adapter._vectorTileLayers['flood-zones']).toBeDefined() + }) + + it('applies dataset visibility after adding layers', async () => { + expect(adapter._groupLayers['flood-zones-group'].visible).toBe(true) + expect(adapter._vectorTileLayers['flood-zones-cc'].visible).toBe(true) + expect(adapter._vectorTileLayers['flood-zones'].visible).toBe(false) + }) + + it('adds GroupLayers for datasets with esriGroupId', async () => { + expect(map.add).toHaveBeenCalledWith(expect.objectContaining({ id: 'flood-zones-group' })) + }) + + it('calls map.add for each top-level dataset', async () => { + expect(map.add.mock.calls.length).toEqual(3) + }) + }) + + // ─── onMapSizeChange ───────────────────────────────────────────────────────── + + describe('onMapSizeChange', () => { + it('resolves without doing anything', async () => { + await expect(adapter.onMapSizeChange()).resolves.toBeUndefined() + }) + }) +}) diff --git a/plugins/beta/datasets/src/initialise/DatasetsInit.jsx b/plugins/beta/datasets/src/initialise/DatasetsInit.jsx index 7f4fb53b..36964be1 100755 --- a/plugins/beta/datasets/src/initialise/DatasetsInit.jsx +++ b/plugins/beta/datasets/src/initialise/DatasetsInit.jsx @@ -55,7 +55,7 @@ export function DatasetsInit ({ pluginConfig, pluginState, appState, mapState, m useEffect(() => { datasetRegistry.attachMapStyle(mapState.mapStyle) if (layerAdapter?.onMapStyleChange) { - layerAdapter.onMapStyleChange(mapState.mapStyle) + layerAdapter.onMapStyleChange() } }, [mapState.mapStyle]) diff --git a/plugins/beta/datasets/src/reducers/__data__/esriDatasets.js b/plugins/beta/datasets/src/reducers/__data__/esriDatasets.js new file mode 100644 index 00000000..56407d4d --- /dev/null +++ b/plugins/beta/datasets/src/reducers/__data__/esriDatasets.js @@ -0,0 +1,144 @@ +export const datasets = [ + { + id: 'flood-zones-cc', + label: 'Flood Zones Climate Change', + groupLabel: 'Datasets', + esriGroupId: 'flood-zones-group', + tiles: 'https://tiles.arcgis.com/tiles/JZM7qJpmv7vJ0Hzx/arcgis/rest/services/Flood_Zones_2_and_3_Rivers_and_Sea_CCP1_NON_PRODUCTION/VectorTileServer', + showInKey: true, + showInMenu: true, + visible: true, + sourceLayer: 'Flood Zones 2 and 3 Rivers and Sea CCP1', + sublayers: [ + { + id: 'climate-change', + label: 'Climate change (2070 to 2125)', + esriStyleLayerId: 'Flood Zones 2 and 3 Rivers and Sea CCP1/Flood Zones plus climate change/1', + showInKey: true, + showInMenu: false, + style: { + fill: { outdoor: '#F4A582', dark: '#BF3D4A' }, + stroke: 'none' + } + }, + { + id: 'data-unavailable', + label: 'Climate change data unavailable', + style: { // This is used just for the key - so that it renders the pattern correctly. + fillPattern: 'dot', + fillPatternForegroundColor: { outdoor: '#000000', dark: '#ffffff' }, + stroke: { outdoor: '#000000', dark: '#FFFFFF' } + }, + showInKey: true, + showInMenu: false + }, + { + id: 'data-unavailable-outline', + style: { + stroke: { outdoor: '#000000', dark: '#FFFFFF' } + }, + esriStyleLayerId: 'Flood Zones 2 and 3 Rivers and Sea CCP1/Unavailable/0', + showInKey: false, + showInMenu: false + }, + { + id: 'data-unavailable-light', + visibleWhen: { mapStyleId: ['outdoor', 'black-and-white'] }, + esriStyleLayerId: 'Flood Zones 2 and 3 Rivers and Sea CCP1/Unavailable/1', + esriUseServerStyle: true, + showInKey: false, + showInMenu: false + }, + { + id: 'data-unavailable-dark', + visibleWhen: { mapStyleId: ['dark'] }, + esriStyleLayerId: 'Flood Zones 2 and 3 Rivers and Sea CCP1/Unavailable/2', + esriUseServerStyle: true, + showInKey: false, + showInMenu: false + } + ] + }, + { + id: 'flood-zones', + label: 'Flood Zones', + groupLabel: 'Datasets', + esriGroupId: 'flood-zones-group', + tiles: 'https://tiles.arcgis.com/tiles/JZM7qJpmv7vJ0Hzx/arcgis/rest/services/Flood_Zones_2_and_3_Rivers_and_Sea_NON_PRODUCTION/VectorTileServer', + showInKey: true, + visible: false, + // showInMenu: true, + sourceLayer: 'Flood Zones 2 and 3 Rivers and Sea', + sublayers: [ + { + id: 'flood-zone-2', + label: 'Flood Zone 2', + esriStyleLayerId: 'Flood Zones 2 and 3 Rivers and Sea/Flood Zone 2/1', + showInMenu: true, + style: { + fill: { outdoor: '#1d70b8', dark: '#7fcdbb' }, + stroke: 'none' + } + }, + { + id: 'flood-zone-3', + label: 'Flood Zone 3', + esriStyleLayerId: 'Flood Zones 2 and 3 Rivers and Sea/Flood Zone 3/1', + showInMenu: true, + style: { + fill: { outdoor: '#003078', dark: '#e5f5e0' }, + stroke: 'none' + } + } + ] + }, + { + id: 'esri-standalone', + label: 'Standalone Layer', + tiles: 'https://example.com/vtl/standalone', + visible: true, + esriStyleLayerId: 'standalone-style', + style: { fill: '#ff0000', stroke: '#000000' } + }, + // Grouped: belongs to a GroupLayer, not visible (covers the visible=false branch) + { + id: 'esri-grouped', + label: 'Grouped Layer', + tiles: 'https://example.com/vtl/grouped', + visible: false, + esriGroupId: 'my-group' + }, + // Server style: setPaintProperties should be skipped + { + id: 'esri-server', + label: 'Server Style Layer', + tiles: 'https://example.com/vtl/server', + visible: true, + esriStyleLayerId: 'server-style-layer', + esriUseServerStyle: true, + style: {} + }, + // Parent with sublayers that have esriStyleLayerId + { + id: 'esri-parent', + label: 'Parent With Sublayers', + tiles: 'https://example.com/vtl/parent', + visible: true, + sublayers: [ + { + id: 'sub-a', + label: 'Sub A', + esriStyleLayerId: 'style-sub-a', + visible: true, + style: { fill: '#00ff00' } + }, + { + id: 'sub-b', + label: 'Sub B', + esriStyleLayerId: 'style-sub-b', + visible: false, + style: {} + } + ] + } +] diff --git a/plugins/beta/datasets/src/registry/__mocks__/datasetRegistry.js b/plugins/beta/datasets/src/registry/__mocks__/datasetRegistry.js index fbb2717b..02eb2e5b 100644 --- a/plugins/beta/datasets/src/registry/__mocks__/datasetRegistry.js +++ b/plugins/beta/datasets/src/registry/__mocks__/datasetRegistry.js @@ -1,5 +1,6 @@ import { mappedDatasetsReducer } from '../../reducers/mappedDatasetsReducer.js' import { datasets as datasetDefinitions } from '../../reducers/__data__/demoDatasets.js' +import { datasets as esriDatasetDefinitions } from '../../reducers/__data__/esriDatasets.js' import { attachGlobalState } from '../globalDataset.js' const { datasetRegistry } = jest.requireActual('../datasetRegistry.js') const { mappedDatasets, orderedDatasets } = mappedDatasetsReducer({ datasets: datasetDefinitions }) @@ -26,4 +27,12 @@ datasetRegistry.mockExtend = (extraDatasets) => datasetRegistry.attach( [...orderedDatasets, ...Object.keys(extraDatasets)] ) +datasetRegistry.useEsriDatasets = (extraDatasets = {}) => { + const { mappedDatasets, orderedDatasets } = mappedDatasetsReducer({ datasets: esriDatasetDefinitions }) + datasetRegistry.attach( + { ...mappedDatasets, ...extraDatasets }, + [...orderedDatasets, ...Object.keys(extraDatasets)] + ) +} + export { datasetRegistry } diff --git a/plugins/beta/datasets/src/registry/dataset.js b/plugins/beta/datasets/src/registry/dataset.js index dce6f78f..a8bd7423 100644 --- a/plugins/beta/datasets/src/registry/dataset.js +++ b/plugins/beta/datasets/src/registry/dataset.js @@ -137,7 +137,7 @@ export class Dataset { if (sublayerIds) { return sublayerIds.map(id => datasetRegistry.getDataset(id)) } - return undefined + return [] } get parent () { From f0b4f611acc7cb89c707c4781e0e3f36c1496d5b Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Wed, 8 Jul 2026 12:29:58 +0100 Subject: [PATCH 12/14] IM-373 fixed a couple of tests --- .../beta/datasets/src/adapters/esri/esriLayerAdapter.test.js | 2 +- plugins/beta/datasets/src/registry/dataset.test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js index 8b40e490..88426002 100644 --- a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js +++ b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js @@ -252,7 +252,7 @@ describe('esriLayerAdapter', () => { }) it('calls map.add for each top-level dataset', async () => { - expect(map.add.mock.calls.length).toEqual(3) + expect(map.add.mock.calls.length).toEqual(5) // 3 top-level datasets + 2 group layers }) }) diff --git a/plugins/beta/datasets/src/registry/dataset.test.js b/plugins/beta/datasets/src/registry/dataset.test.js index d88fb69d..c7bf00c1 100644 --- a/plugins/beta/datasets/src/registry/dataset.test.js +++ b/plugins/beta/datasets/src/registry/dataset.test.js @@ -41,9 +41,9 @@ describe('Dataset class', () => { }) describe('sublayers', () => { - it('returns undefined for a dataset with no sublayerIds', () => { + it('returns an empty array for a dataset with no sublayerIds', () => { const dataset = datasetRegistry.getDataset('hedge-control') - expect(dataset.sublayers).toBeUndefined() + expect(dataset.sublayers).toEqual([]) }) it('returns a Dataset instance for each sublayer', () => { From 2d37f6a56cebd28fd030abad202328b4dc6229c9 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Wed, 8 Jul 2026 16:55:19 +0100 Subject: [PATCH 13/14] IM-373 removed MapLibreDataset from maplibreLayerAdapter.test.js --- .../datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js index 7470c28f..66ad72fd 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js @@ -1,5 +1,4 @@ import MaplibreLayerAdapter from './maplibreLayerAdapter.js' -import { MapLibreDataset } from './registry/mapLibreDataset.js' import { datasetRegistry } from '../../registry/datasetRegistry.js' import { symbolRegistry } from '../../../../../../src/services/symbolRegistry.js' import { patternRegistry } from '../../../../../../src/services/patternRegistry.js' From 30ac614634c97f220120a7797b103d6f879265cd Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Wed, 8 Jul 2026 17:09:28 +0100 Subject: [PATCH 14/14] IM-373 removed commented tests --- .../adapters/esri/esriLayerAdapter.test.js | 86 ------------------- 1 file changed, 86 deletions(-) diff --git a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js index 88426002..bb43d16c 100644 --- a/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js +++ b/plugins/beta/datasets/src/adapters/esri/esriLayerAdapter.test.js @@ -55,82 +55,6 @@ describe('esriLayerAdapter', () => { }) }) - // ─── _addLayers ────────────────────────────────────────────────────────────── - - // describe('_addLayers', () => { - // it('stores the VectorTileLayer in _vectorTileLayers', async () => { - // await adapter._addLayers(datasetRegistry.getDataset('esri-standalone'), MAP_STYLE) - // expect(adapter._vectorTileLayers['esri-standalone']).toBeDefined() - // }) - - // it('adds the VectorTileLayer directly to the map when there is no esriGroupId', async () => { - // const ds = datasetRegistry.getDataset('esri-standalone') - // await adapter._addLayers(ds, MAP_STYLE) - // expect(map.add).toHaveBeenCalledWith(adapter._vectorTileLayers['esri-standalone']) - // }) - - // it('stores the VectorTileLayer as the opacity layer when standalone', async () => { - // const ds = datasetRegistry.getDataset('esri-standalone') - // await adapter._addLayers(ds, MAP_STYLE) - // expect(adapter._vectorTileOpacityLayers['esri-standalone']).toBe(adapter._vectorTileLayers['esri-standalone']) - // }) - - // it('adds the VectorTileLayer inside a GroupLayer when esriGroupId is set', async () => { - // const ds = datasetRegistry.getDataset('esri-grouped') - // await adapter._addLayers(ds, MAP_STYLE) - // const gl = adapter._groupLayers['my-group'] - // expect(gl.add).toHaveBeenCalledWith(adapter._vectorTileLayers['esri-grouped']) - // }) - - // it('stores the GroupLayer as the opacity layer when esriGroupId is set', async () => { - // const ds = datasetRegistry.getDataset('esri-grouped') - // await adapter._addLayers(ds, MAP_STYLE) - // expect(adapter._vectorTileOpacityLayers['esri-grouped']).toBe(adapter._groupLayers['my-group']) - // }) - // }) - - // ─── _applyRegistryDatasetVisibility ───────────────────────────────────────── - - describe.skip('_applyRegistryDatasetVisibility', () => { - it('calls setStyleLayerVisibility on the parent VectorTileLayer for a sublayer', async () => { - await adapter._addLayers(datasetRegistry.getDataset('esri-parent'), MAP_STYLE) - const subA = datasetRegistry.getDataset('esri-parent-sub-a') - adapter._applyRegistryDatasetVisibility(subA) - expect(adapter._vectorTileLayers['esri-parent'].setStyleLayerVisibility) - .toHaveBeenCalledWith('style-sub-a', subA.visibility) - }) - - it('sets vectorTileLayer.visible to true for a visible top-level dataset', async () => { - const ds = datasetRegistry.getDataset('esri-standalone') - await adapter._addLayers(ds, MAP_STYLE) - adapter._applyRegistryDatasetVisibility(ds) - expect(adapter._vectorTileLayers['esri-standalone'].visible).toBe(true) - }) - - it('sets vectorTileLayer.visible to false for a hidden top-level dataset', async () => { - const ds = datasetRegistry.getDataset('esri-grouped') - await adapter._addLayers(ds, MAP_STYLE) - adapter._applyRegistryDatasetVisibility(ds) - expect(adapter._vectorTileLayers['esri-grouped'].visible).toBe(false) - }) - - it('applies sublayer style visibility when the top-level dataset is visible', async () => { - await adapter._addLayers(datasetRegistry.getDataset('esri-parent'), MAP_STYLE) - const parent = datasetRegistry.getDataset('esri-parent') - adapter._applyRegistryDatasetVisibility(parent) - const vtl = adapter._vectorTileLayers['esri-parent'] - expect(vtl.setStyleLayerVisibility).toHaveBeenCalledWith('style-sub-a', 'visible') - expect(vtl.setStyleLayerVisibility).toHaveBeenCalledWith('style-sub-b', 'none') - }) - - it('skips sublayer style visibility when the top-level dataset is not visible', async () => { - const ds = datasetRegistry.getDataset('esri-grouped') - await adapter._addLayers(ds, MAP_STYLE) - adapter._applyRegistryDatasetVisibility(ds) - expect(adapter._vectorTileLayers['esri-grouped'].setStyleLayerVisibility).not.toHaveBeenCalled() - }) - }) - // ─── applyDatasetVisibility ────────────────────────────────────────────────── describe('applyDatasetVisibility', () => { @@ -149,16 +73,6 @@ describe('esriLayerAdapter', () => { }) }) - // ─── applyGlobalVisibility ─────────────────────────────────────────────────── - - describe('applyGlobalVisibility', () => { - it.skip('applies visibility to all known datasets', async () => { - await adapter._addLayers(datasetRegistry.getDataset('esri-standalone'), MAP_STYLE) - await adapter.applyGlobalVisibility() - expect(adapter._vectorTileLayers['esri-standalone'].visible).toBe(true) - }) - }) - // ─── applyDatasetOpacity ───────────────────────────────────────────────────── describe('applyDatasetOpacity', () => {