From 8de1161e47b7b47e31ea7b2a4846cdc930ac8343 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Tue, 14 Jul 2026 13:02:26 +0100 Subject: [PATCH 1/7] IM-401 addressed some Sonar Issues --- .../src/adapters/esri/esriLayerAdapter.js | 10 ++++---- .../adapters/maplibre/maplibreLayerAdapter.js | 6 +++-- plugins/datasets/src/api/getStyle.js | 2 +- plugins/datasets/src/api/setData.js | 2 +- plugins/datasets/src/components/Key/Key.jsx | 1 - .../components/LayersMenu/LayersMenuRadio.jsx | 2 +- .../LayersMenu/LayersRadioGroupWrapper.jsx | 7 +++--- .../src/initialise/initialiseDatasets.js | 1 - .../src/reducers/__data__/demoDatasets.js | 23 +++++++++---------- .../src/registry/datasetDefinitionCache.js | 4 ++-- .../datasets/src/registry/datasetRegistry.js | 2 +- plugins/datasets/src/utils/bbox.js | 8 +++---- 12 files changed, 33 insertions(+), 35 deletions(-) diff --git a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js index 3cc304b5e..6be8f18b7 100644 --- a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js +++ b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js @@ -6,11 +6,10 @@ import { EsriDataset } from './registry/esriDataset.js' import { logger } from '../../../../../src/services/logger.js' export default class EsriLayerAdapter extends LayerAdapter { - constructor (mapProvider, symbolRegistry, patternRegistry) { + constructor (mapProvider) { super() this._mapProvider = mapProvider this._map = mapProvider.map - // TODO: Implement symbolRegistry and patternRegistry usage in the adapter // _vectorTileLayers is a map of datasetId to VectorTileLayer instances // it includes stand alone vectorTileLayers and vectorTileLayers that are part of a groupLayer @@ -33,7 +32,7 @@ export default class EsriLayerAdapter extends LayerAdapter { async init () { const topLevelDatasets = datasetRegistry.topLevelDatasets() // ensure the datasets are added in order - for await (const registryDataset of topLevelDatasets) { + for (const registryDataset of topLevelDatasets) { await this._addLayers(registryDataset) } @@ -111,7 +110,7 @@ export default class EsriLayerAdapter extends LayerAdapter { } // If the group layer has no more sublayers, we need to also remove the group layer from the map - if (groupLayer && groupLayer.layers.length === 0) { + if (groupLayer?.layers.length === 0) { this._map.remove(groupLayer) delete this._groupLayers[esriGroupId] } @@ -127,7 +126,8 @@ export default class EsriLayerAdapter extends LayerAdapter { this._applyStyleLayerVisibility(registryDataset, vectorTileLayer) // Don't apply the visibility change to the parent, since the parent may have other sublayers that are visible return - } else if (visible) { + } + if (visible) { // No need to apply style layer visibility for datasets that are hidden registryDataset.sublayers.forEach(sublayer => this._applyStyleLayerVisibility(sublayer, vectorTileLayer)) } diff --git a/plugins/datasets/src/adapters/maplibre/maplibreLayerAdapter.js b/plugins/datasets/src/adapters/maplibre/maplibreLayerAdapter.js index dc214ad45..d33011fca 100644 --- a/plugins/datasets/src/adapters/maplibre/maplibreLayerAdapter.js +++ b/plugins/datasets/src/adapters/maplibre/maplibreLayerAdapter.js @@ -125,7 +125,9 @@ export default class MaplibreLayerAdapter extends LayerAdapter { if (imageId) { this._map.setLayoutProperty(symbolLayerId, 'icon-image', imageId) } - } else if (fillLayerId && this._map.getLayer(fillLayerId)) { + return + } + if (fillLayerId && this._map.getLayer(fillLayerId)) { const imageId = this._patternRegistry.getPatternImageId(registryDataset.style, mapStyle.id, this._pixelRatio) if (imageId) { this._map.setPaintProperty(fillLayerId, 'fill-pattern', imageId) @@ -163,7 +165,7 @@ export default class MaplibreLayerAdapter extends LayerAdapter { // Remove source if no other dataset is using it const sourceIsShared = datasetRegistry.topLevelDatasets() - .filter(registryDataset => registryDataset.id !== datasetId && registryDataset.sourceId === sourceId) + .filter(dataset => dataset.id !== datasetId && dataset.sourceId === sourceId) .length > 0 if (!sourceIsShared && this._map.getSource(sourceId)) { diff --git a/plugins/datasets/src/api/getStyle.js b/plugins/datasets/src/api/getStyle.js index 6ecfc39af..0f26d3401 100644 --- a/plugins/datasets/src/api/getStyle.js +++ b/plugins/datasets/src/api/getStyle.js @@ -1,7 +1,7 @@ import { logger } from '../../../../src/services/logger.js' import { datasetRegistry } from '../registry/datasetRegistry.js' -export const getStyle = ({ pluginState }, { datasetId, sublayerId } = {}) => { +export const getStyle = ({ _pluginState }, { datasetId, sublayerId } = {}) => { datasetId = sublayerId ? `${datasetId}-${sublayerId}` : datasetId const registryDataset = datasetRegistry.getDataset(datasetId) if (!registryDataset) { diff --git a/plugins/datasets/src/api/setData.js b/plugins/datasets/src/api/setData.js index f5485fc87..a63f4f640 100644 --- a/plugins/datasets/src/api/setData.js +++ b/plugins/datasets/src/api/setData.js @@ -2,7 +2,7 @@ import { logger } from '../../../../src/services/logger.js' import { datasetRegistry } from '../registry/datasetRegistry.js' import { layerAdapter } from '../adapters/loadLayerAdapter.js' -export const setData = ({ pluginState }, geojson, { datasetId }) => { +export const setData = ({ _pluginState }, geojson, { datasetId }) => { const registryDataset = datasetRegistry.getDataset(datasetId) if (!registryDataset) { logger.warn(`setData: Dataset with id ${datasetId} not found`) diff --git a/plugins/datasets/src/components/Key/Key.jsx b/plugins/datasets/src/components/Key/Key.jsx index 40d9d3ff6..2b3582d5f 100755 --- a/plugins/datasets/src/components/Key/Key.jsx +++ b/plugins/datasets/src/components/Key/Key.jsx @@ -7,7 +7,6 @@ import { datasetRegistry } from '../../registry/datasetRegistry.js' export const Key = ({ pluginConfig: { noKeyItemText }, mapState: { mapStyle }, - pluginState: { mappedDatasets }, services: { symbolRegistry, patternRegistry } }) => { const { items: keyGroups, hasGroups } = datasetRegistry.keyItems() diff --git a/plugins/datasets/src/components/LayersMenu/LayersMenuRadio.jsx b/plugins/datasets/src/components/LayersMenu/LayersMenuRadio.jsx index 2b74090ea..0ffefb89c 100644 --- a/plugins/datasets/src/components/LayersMenu/LayersMenuRadio.jsx +++ b/plugins/datasets/src/components/LayersMenu/LayersMenuRadio.jsx @@ -1,6 +1,6 @@ import { isVisibleWhen } from '../../registry/isVisibleWhen.js' -export const LayersMenuRadio = ({ menuState, menuGroupItem, checked, name, onChange }) => { +export const LayersMenuRadio = ({ menuGroupItem, name, checked, onChange }) => { const itemClass = 'im-c-datasets-layers__item govuk-radios govuk-radios--small"' const { visibleWhen } = menuGroupItem const visible = visibleWhen ? isVisibleWhen(visibleWhen) : true diff --git a/plugins/datasets/src/components/LayersMenu/LayersRadioGroupWrapper.jsx b/plugins/datasets/src/components/LayersMenu/LayersRadioGroupWrapper.jsx index 493d51430..9f12b17b6 100644 --- a/plugins/datasets/src/components/LayersMenu/LayersRadioGroupWrapper.jsx +++ b/plugins/datasets/src/components/LayersMenu/LayersRadioGroupWrapper.jsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react' +import React from 'react' import { isVisibleWhen } from '../../registry/isVisibleWhen.js' import { LayersMenuRadio } from './LayersMenuRadio.jsx' @@ -10,9 +10,8 @@ export const LayersRadioGroupWrapper = ({ pluginState, menuGroup }) => { } const { menuState, dispatch } = pluginState - const [value, setValue] = useState(menuState[id]) + const value = menuState[id] const handleChange = (event) => { - setValue(event.target.value) dispatch({ type: 'UPDATE_MENU_STATE', payload: { [id]: event.target.value } }) } @@ -23,7 +22,7 @@ export const LayersRadioGroupWrapper = ({ pluginState, menuGroup }) => { {menuGroup.label} -
+
{items.map((menuGroupItem) => { - const existingDefinition = this.idToDefinitionMap.get(id) - this.definitionToInstanceMap.delete(existingDefinition) + const definition = this.idToDefinitionMap.get(id) + this.definitionToInstanceMap.delete(definition) this.idToDefinitionMap.delete(id) }) } diff --git a/plugins/datasets/src/registry/datasetRegistry.js b/plugins/datasets/src/registry/datasetRegistry.js index aebdc2d4f..8b9a2e88f 100644 --- a/plugins/datasets/src/registry/datasetRegistry.js +++ b/plugins/datasets/src/registry/datasetRegistry.js @@ -25,7 +25,7 @@ const datasetRegistry = { // createDataset defaults to a generic dataset factory function, but can be overridden by calling // attachCreateDataset, which allows the layer adapter to provide its own createDataset function, - attachCreateDataset (createDataset) { this._createDataset = createDataset }, + attachCreateDataset (newCreateDatasetFunction) { this._createDataset = newCreateDatasetFunction }, _createDataset: (datasetDefinition) => createDataset(datasetDefinition), attachMapStyle (mapStyle) { diff --git a/plugins/datasets/src/utils/bbox.js b/plugins/datasets/src/utils/bbox.js index bcb0d55e7..7fa337638 100755 --- a/plugins/datasets/src/utils/bbox.js +++ b/plugins/datasets/src/utils/bbox.js @@ -103,10 +103,10 @@ export const getGeometryBbox = (geometry) => { case 'GeometryCollection': geometry.geometries.forEach(g => { const b = getGeometryBbox(g) - minX = Math.min(minX, b[0]) - minY = Math.min(minY, b[1]) - maxX = Math.max(maxX, b[2]) - maxY = Math.max(maxY, b[3]) + minX = Math.min(minX, b[0]) // west + minY = Math.min(minY, b[1]) // south + maxX = Math.max(maxX, b[2]) // east + maxY = Math.max(maxY, b[3]) // NOSONAR north }) break default: From dda7db2fa9d45b6c61ac7cf4ee6b31e0bdc51407 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Tue, 14 Jul 2026 13:41:48 +0100 Subject: [PATCH 2/7] IM-401 more sonar fixes --- .../src/reducers/__data__/demoDatasets.js | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/plugins/datasets/src/reducers/__data__/demoDatasets.js b/plugins/datasets/src/reducers/__data__/demoDatasets.js index aeddec4d4..64cf922f8 100644 --- a/plugins/datasets/src/reducers/__data__/demoDatasets.js +++ b/plugins/datasets/src/reducers/__data__/demoDatasets.js @@ -3,16 +3,16 @@ const pointData = { features: [{ type: 'Feature', properties: { category: 'prehistoric' }, - geometry: { coordinates: [-2.4558622, 54.5617135], type: 'Point' } + geometry: { coordinates: [-2.4558622, 54.5617135], type: 'Point' } // NOSONAR }, { type: 'Feature', properties: { category: 'roman' }, - geometry: { coordinates: [-2.439823, 54.5525437], type: 'Point' } + geometry: { coordinates: [-2.439823, 54.5525437], type: 'Point' } // NOSONAR }, { type: 'Feature', properties: { category: 'medieval' }, - geometry: { coordinates: [-2.4481939, 54.5575261], type: 'Point' } + geometry: { coordinates: [-2.4481939, 54.5575261], type: 'Point' } // NOSONAR }] } export const datasets = [ @@ -25,7 +25,7 @@ export const datasets = [ transformRequest: (url) => url + 'TRANSFORMED', // Required maxFeatures: 50000 // Optional: evict distant features when exceeded }, - hiddenFeatures: [42], + hiddenFeatures: [42], // NOSONAR query: {}, maxFeatures: 50000, // Optional: evict distant features when exceeded minZoom: 10, @@ -219,14 +219,8 @@ export const expectedDatasetsMenuConfig = [ } ] -const landCovers = datasets[0] -const existingFields = datasets[1] -const historicMonuments = datasets[2] -const hedgeControl = datasets[3] -const landCoversMenuItem = expectedDatasetsMenuConfig[0] -const existingFieldsMenuItem = expectedDatasetsMenuConfig[1] -const historicMonumentsMenuItem = expectedDatasetsMenuConfig[2] -const hedgeControlMenuItem = expectedDatasetsMenuConfig[3] +const [landCovers, existingFields, historicMonuments, hedgeControl] = datasets +const [landCoversMenuItem, existingFieldsMenuItem, historicMonumentsMenuItem, hedgeControlMenuItem] = expectedDatasetsMenuConfig const testGroupLabel = 'Test group' export const datasetsWithGroups = [ From 8e2ec3e053a9cd70f23e75df4208e0ba0255a0d3 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Tue, 14 Jul 2026 16:49:10 +0100 Subject: [PATCH 3/7] IM-399 fixed umd build issue with isVisibleWhen setMenuState --- plugins/datasets/src/registry/isVisibleWhen.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/datasets/src/registry/isVisibleWhen.js b/plugins/datasets/src/registry/isVisibleWhen.js index b2fd1cb97..f9d32ece0 100644 --- a/plugins/datasets/src/registry/isVisibleWhen.js +++ b/plugins/datasets/src/registry/isVisibleWhen.js @@ -1,7 +1,7 @@ import { datasetRegistry } from './datasetRegistry.js' let _menuState = {} -export const setMenuState = (menuState) => (_menuState = menuState) +export const setMenuState = (menuState) => { _menuState = menuState } const _isVisibleWhenMenuCheck = (menuVisibleWhen) => { for (const [key, valueArray] of Object.entries(menuVisibleWhen)) { From f83badfd39edf74067a89f1f3b24d249d0e4ff1e Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Tue, 14 Jul 2026 16:54:37 +0100 Subject: [PATCH 4/7] IM-399 featureLayers working - but no styles --- demo/js/esri-datasets.js | 66 +++++++++++++++++-- demo/js/ml-datasets.js | 18 ++--- .../src/adapters/esri/esriLayerAdapter.js | 28 +++++++- plugins/datasets/src/registry/dataset.js | 1 + 4 files changed, 96 insertions(+), 17 deletions(-) diff --git a/demo/js/esri-datasets.js b/demo/js/esri-datasets.js index 54e3535be..3de2d1d95 100644 --- a/demo/js/esri-datasets.js +++ b/demo/js/esri-datasets.js @@ -9,6 +9,8 @@ import { transformGeocodeRequest, transformVtsRequest3857, setupEsriConfig } fro const nonFloodZoneLight = '#2b8cbe' const nonFloodZoneDark = '#7fcdbb' +const white = '#ffffff' +const darkTeal = '#12393d' const COLOURS = { // floodExtents: { default: nonFloodZoneLight, dark: nonFloodZoneDark }, @@ -36,8 +38,6 @@ const nonFloodZoneDepthBandsLight = [COLOURS.depthOver2300.default, COLOURS.dept // GREENS dark tones > 2300 to < 150 const nonFloodZoneDepthBandsDark = [COLOURS.depthOver2300.dark, COLOURS.depth2300.dark, COLOURS.depth1200.dark, COLOURS.depth900.dark, COLOURS.depth600.dark, COLOURS.depth300.dark, COLOURS.depth150.dark] - - const datasetFloodZonesCC = { id: 'floodzonescc', label: 'Flood Zones Climate Change', @@ -326,8 +326,60 @@ const surfaceWaterDepthAllDataset = { ] } +const datasetMainRivers = { + id: 'mainrivers', + label: 'Main Rivers', + groupLabel: 'Map features', + type: 'FeatureService', + tiles: 'https://services1.arcgis.com/JZM7qJpmv7vJ0Hzx/arcgis/rest/services/Statutory_Main_River_Map/FeatureServer', + showInKey: true, + showInMenu: true, + sourceLayer: 'Statutory_Main_River_Map', + visible: false, + style: { + stroke: { outdoor: darkTeal, dark: white }, + strokeWidth: 3 + } +} + +const datasetWaterStorageAreas = { + id: 'waterstorage', + label: 'Water Storage', + groupLabel: 'Map features', + type: 'FeatureService', + tiles: 'https://services1.arcgis.com/JZM7qJpmv7vJ0Hzx/arcgis/rest/services/Flood_Storage_Areas_NON_PRODUCTION/FeatureServer', + showInKey: true, + showInMenu: true, + sourceLayer: 'Flood_Storage_Areas', + visible: false, + style: { + stroke: { outdoor: darkTeal, dark: white }, + strokeWidth: 1, + fillPattern: 'diagonal-cross-hatch', + fillPatternForegroundColor: { outdoor: darkTeal, dark: white }, + fillPatternBackgroundColor: 'transparent' + } +} + +const datasetFloodDefences = { + id: 'flooddefence', + label: 'Flood Defence', + groupLabel: 'Map features', + type: 'FeatureService', + tiles: 'https://services1.arcgis.com/JZM7qJpmv7vJ0Hzx/arcgis/rest/services/Defences_NON_PRODUCTION/FeatureServer', + showInKey: true, + showInMenu: true, + sourceLayer: 'Defences', + visible: false, + style: { + stroke: { outdoor: '#f47738', dark: '#f47738' }, + strokeWidth: 3 + } +} + const datasets = [ - datasetFloodZonesCC, datasetFloodZones, surfaceWaterDataset, surfaceWaterDepthAllDataset + datasetFloodZonesCC, datasetFloodZones, surfaceWaterDataset, surfaceWaterDepthAllDataset, + datasetWaterStorageAreas, datasetFloodDefences, datasetMainRivers ] const menu = [ @@ -391,14 +443,14 @@ const menu = [ ] }, { id: 'features', - label: 'Map features', + groupLabel: 'Map features', urlKey: 'features', type: 'checkbox', visibleWhen: true, items: [ - { id: 'water-storage', label: 'Water storage', checked: false }, - { id: 'flood-defence', label: 'Flood defence', checked: false }, - { id: 'main-rivers', label: 'Main rivers', checked: false }, + { id: 'waterstorage', label: 'Water storage' }, + { id: 'flooddefence', label: 'Flood defence' }, + { id: 'mainrivers', label: 'Main rivers' }, ] } ] diff --git a/demo/js/ml-datasets.js b/demo/js/ml-datasets.js index 378f54039..ec3d79034 100644 --- a/demo/js/ml-datasets.js +++ b/demo/js/ml-datasets.js @@ -467,15 +467,15 @@ const testSetData = () => { } interactiveMap.on('datasets:ready', function () { - testGetters() - testInvalidApiCalls() - testFeatureVisibility() - testSetOpacity() - testSetStyle() - testVisibility() - testGlobalVisibility() - testRemoveAndAddDataset() - testSetData() + // testGetters() + // testInvalidApiCalls() + // testFeatureVisibility() + // testSetOpacity() + // testSetStyle() + // testVisibility() + // testGlobalVisibility() + // testRemoveAndAddDataset() + // testSetData() }) // Ref to the selected features diff --git a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js index 6be8f18b7..58cc0e469 100644 --- a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js +++ b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js @@ -1,4 +1,5 @@ import VectorTileLayer from '@arcgis/core/layers/VectorTileLayer.js' +import FeatureLayer from '@arcgis/core/layers/FeatureLayer.js' import GroupLayer from '@arcgis/core/layers/GroupLayer.js' import { LayerAdapter } from '../layerAdapter.js' import { datasetRegistry } from '../../registry/datasetRegistry.js' @@ -61,8 +62,30 @@ export default class EsriLayerAdapter extends LayerAdapter { return this._groupLayers[esriGroupId] } + async _addFeatureLayers (registryDataset) { + const featureLayer = new FeatureLayer({ + id: registryDataset.id, + url: registryDataset.tiles, + opacity: 1, + visible: false + }) + this._vectorTileLayers[registryDataset.id] = featureLayer + this._vectorTileOpacityLayers[registryDataset.id] = featureLayer + try { + this._map.add(featureLayer) + return featureLayer.when() + } catch (error) { + logger.error(`Error adding FeatureLayer for dataset ${registryDataset.id}:`, error) + } + } + async _addLayers (registryDataset) { - const { esriGroupId } = registryDataset + const { type, esriGroupId } = registryDataset + + if (type === 'FeatureService') { + return this._addFeatureLayers(registryDataset) + } + const vectorTileParent = esriGroupId ? this._addGroupLayer(esriGroupId) : this._map const vectorTileLayer = new VectorTileLayer({ id: registryDataset.id, @@ -121,6 +144,9 @@ export default class EsriLayerAdapter extends LayerAdapter { // if this is a top level dataset, we need to apply the visibility to the vectorTileLayer/ groupLayer itself const { id, isSublayer, visible, parentId } = registryDataset const vectorTileLayer = this._vectorTileLayers[isSublayer ? parentId : id] + if (!vectorTileLayer) { + return + } if (isSublayer) { this._applyStyleLayerVisibility(registryDataset, vectorTileLayer) diff --git a/plugins/datasets/src/registry/dataset.js b/plugins/datasets/src/registry/dataset.js index 9d14997d9..77237ea20 100644 --- a/plugins/datasets/src/registry/dataset.js +++ b/plugins/datasets/src/registry/dataset.js @@ -23,6 +23,7 @@ export class Dataset { get parentId () { return this._datasetDefinition.parentId } get minZoom () { return this._datasetDefinition.minZoom || this.parent?.minZoom } get maxZoom () { return this._datasetDefinition.maxZoom || this.parent?.maxZoom } + get type () { return this._datasetDefinition.type || this.parent?.type } get showInKey () { const own = this._datasetDefinition.showInKey From c32915db05d6b73ec48f9155e1295dc09f472869 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Tue, 14 Jul 2026 20:56:54 +0100 Subject: [PATCH 5/7] IM-399 featureLayers styles working --- demo/js/esri-datasets.js | 30 +++++++++- .../src/adapters/esri/esriLayerAdapter.js | 46 +++++++++------ .../adapters/esri/esriLayerAdapter.test.js | 58 +++++++++---------- .../src/adapters/esri/registry/esriDataset.js | 19 ++++++ 4 files changed, 104 insertions(+), 49 deletions(-) diff --git a/demo/js/esri-datasets.js b/demo/js/esri-datasets.js index 3de2d1d95..60c5b1ffe 100644 --- a/demo/js/esri-datasets.js +++ b/demo/js/esri-datasets.js @@ -337,6 +337,14 @@ const datasetMainRivers = { sourceLayer: 'Statutory_Main_River_Map', visible: false, style: { + renderer: { + type: 'simple', + symbol: { + type: 'simple-line', + width: '3px', + color: { outdoor: darkTeal, dark: white }, + } + }, stroke: { outdoor: darkTeal, dark: white }, strokeWidth: 3 } @@ -353,6 +361,18 @@ const datasetWaterStorageAreas = { sourceLayer: 'Flood_Storage_Areas', visible: false, style: { + renderer: { + type: 'simple', + symbol: { + type: 'simple-fill', + style: 'diagonal-cross', + color: { outdoor: darkTeal, dark: white }, + outline: { + color: { outdoor: darkTeal, dark: white }, + width: 1 + } + } + }, stroke: { outdoor: darkTeal, dark: white }, strokeWidth: 1, fillPattern: 'diagonal-cross-hatch', @@ -372,7 +392,15 @@ const datasetFloodDefences = { sourceLayer: 'Defences', visible: false, style: { - stroke: { outdoor: '#f47738', dark: '#f47738' }, + renderer: { + type: 'simple', + symbol: { + type: 'simple-line', + width: '3px', + color: '#f47738', + } + }, + stroke: '#f47738', strokeWidth: 3 } } diff --git a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js index 58cc0e469..ab80862b8 100644 --- a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js +++ b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js @@ -12,15 +12,15 @@ export default class EsriLayerAdapter extends LayerAdapter { this._mapProvider = mapProvider this._map = mapProvider.map - // _vectorTileLayers is a map of datasetId to VectorTileLayer instances + // _mapVisibilityLayers is a map of datasetId to VectorTileLayer or FeatureLayer instances // it includes stand alone vectorTileLayers and vectorTileLayers that are part of a groupLayer // but does not include group layers themselves, which are tracked in _groupLayers - this._vectorTileLayers = {} + this._mapVisibilityLayers = {} - // _vectorTileOpacityLayers is a map of datasetId to VectorTileLayer/GroupLayer where opacity is applied - // it includes stand alone vectorTileLayers and groupLayers that contain vectorTileLayers + // _mapOpacityLayers is a map of datasetId to mapLayers where opacity is applied + // it includes featureLayers, vectorTileLayers and groupLayers // but does not include vectorTileLayers that are part of a groupLayer - this._vectorTileOpacityLayers = {} + this._mapOpacityLayers = {} // _groupLayers is a map of esriGroupId to GroupLayer this._groupLayers = {} @@ -66,11 +66,12 @@ export default class EsriLayerAdapter extends LayerAdapter { const featureLayer = new FeatureLayer({ id: registryDataset.id, url: registryDataset.tiles, + renderer: registryDataset.renderer, opacity: 1, visible: false }) - this._vectorTileLayers[registryDataset.id] = featureLayer - this._vectorTileOpacityLayers[registryDataset.id] = featureLayer + this._mapVisibilityLayers[registryDataset.id] = featureLayer + this._mapOpacityLayers[registryDataset.id] = featureLayer try { this._map.add(featureLayer) return featureLayer.when() @@ -93,8 +94,8 @@ export default class EsriLayerAdapter extends LayerAdapter { opacity: 1, visible: false }) - this._vectorTileLayers[registryDataset.id] = vectorTileLayer - this._vectorTileOpacityLayers[registryDataset.id] = esriGroupId ? vectorTileParent : vectorTileLayer + this._mapVisibilityLayers[registryDataset.id] = vectorTileLayer + this._mapOpacityLayers[registryDataset.id] = esriGroupId ? vectorTileParent : vectorTileLayer vectorTileParent.add(vectorTileLayer) return vectorTileLayer.when() } @@ -107,7 +108,7 @@ export default class EsriLayerAdapter extends LayerAdapter { } await this._addLayers(registryDataset) const { parentId } = registryDataset - const vectorTileLayer = this._vectorTileLayers[parentId || datasetId] + const vectorTileLayer = this._mapVisibilityLayers[parentId || datasetId] this.applyDatasetOpacity(datasetId) this._applyStyleLayerPaintProperties(registryDataset, vectorTileLayer) this.applyDatasetVisibility(datasetId) @@ -119,7 +120,7 @@ export default class EsriLayerAdapter extends LayerAdapter { return } const { esriGroupId } = registryDataset - const vectorTileLayer = this._vectorTileLayers[datasetId] + const vectorTileLayer = this._mapVisibilityLayers[datasetId] // If the dataset is part of a group layer, we need to remove it from the group layer const groupLayer = esriGroupId ? this._groupLayers[esriGroupId] : null const vectorTileParent = groupLayer || this._map @@ -128,8 +129,8 @@ export default class EsriLayerAdapter extends LayerAdapter { // Remove the vectorTileLayer from the map or group layer vectorTileParent.remove(vectorTileLayer) // And remove the vectorTileLayer from the adapter's internal state - delete this._vectorTileLayers[datasetId] - delete this._vectorTileOpacityLayers[datasetId] + delete this._mapVisibilityLayers[datasetId] + delete this._mapOpacityLayers[datasetId] } // If the group layer has no more sublayers, we need to also remove the group layer from the map @@ -143,7 +144,7 @@ export default class EsriLayerAdapter extends LayerAdapter { // if this is a sublayer, we need to apply the visibility to the vectorTileLayers style sheet // if this is a top level dataset, we need to apply the visibility to the vectorTileLayer/ groupLayer itself const { id, isSublayer, visible, parentId } = registryDataset - const vectorTileLayer = this._vectorTileLayers[isSublayer ? parentId : id] + const vectorTileLayer = this._mapVisibilityLayers[isSublayer ? parentId : id] if (!vectorTileLayer) { return } @@ -172,7 +173,7 @@ export default class EsriLayerAdapter extends LayerAdapter { } async applyDatasetOpacity (datasetId) { - const vectorTileLayer = this._vectorTileOpacityLayers[datasetId] + const vectorTileLayer = this._mapOpacityLayers[datasetId] const registryDataset = datasetRegistry.getDataset(datasetId) if (vectorTileLayer && registryDataset) { vectorTileLayer.opacity = registryDataset.opacity @@ -180,7 +181,7 @@ export default class EsriLayerAdapter extends LayerAdapter { } async applyGlobalOpacity () { - Object.entries(this._vectorTileOpacityLayers).forEach(([datasetId, vectorTileLayer]) => { + Object.entries(this._mapOpacityLayers).forEach(([datasetId, vectorTileLayer]) => { const registryDataset = datasetRegistry.getDataset(datasetId) if (registryDataset) { vectorTileLayer.opacity = registryDataset.opacity @@ -211,9 +212,16 @@ export default class EsriLayerAdapter extends LayerAdapter { async onMapStyleChange () { datasetRegistry.forEach(registryDataset => { const { id, isSublayer, parent } = registryDataset - const vectorTileLayer = this._vectorTileLayers[isSublayer ? parent.id : id] - this._applyStyleLayerVisibility(registryDataset, vectorTileLayer) - this._applyStyleLayerPaintProperties(registryDataset, vectorTileLayer) + + // mapLayer could be a VectorTileLayer or a FeatureLayer, depending on the dataset type + const mapLayer = this._mapVisibilityLayers[isSublayer ? parent.id : id] + if (registryDataset.type === 'FeatureService') { + // FeatureLayers don't have style layers, so we don't need to apply style layer visibility or paint properties + mapLayer.renderer = registryDataset.renderer + } else { + this._applyStyleLayerVisibility(registryDataset, mapLayer) + this._applyStyleLayerPaintProperties(registryDataset, mapLayer) + } }) // TODO - handle dynamic sources } diff --git a/plugins/datasets/src/adapters/esri/esriLayerAdapter.test.js b/plugins/datasets/src/adapters/esri/esriLayerAdapter.test.js index 9194185d8..172ef854a 100644 --- a/plugins/datasets/src/adapters/esri/esriLayerAdapter.test.js +++ b/plugins/datasets/src/adapters/esri/esriLayerAdapter.test.js @@ -71,38 +71,38 @@ describe('esriLayerAdapter', () => { describe('addDataset', () => { it('copes and returns when the dataset is not in the registry', async () => { await adapter.addDataset('unknown') - expect(adapter._vectorTileLayers.unknown).toBeUndefined() + expect(adapter._mapVisibilityLayers.unknown).toBeUndefined() }) it('adds a standalone dataset to the map and populates internal state', async () => { await adapter.addDataset('esri-standalone') - expect(adapter._vectorTileLayers['esri-standalone']).toBeDefined() - expect(adapter._vectorTileOpacityLayers['esri-standalone']).toBeDefined() + expect(adapter._mapVisibilityLayers['esri-standalone']).toBeDefined() + expect(adapter._mapOpacityLayers['esri-standalone']).toBeDefined() }) it('applies opacity and visibility after adding layers', async () => { await adapter.addDataset('esri-standalone') - const vtl = adapter._vectorTileLayers['esri-standalone'] + const vtl = adapter._mapVisibilityLayers['esri-standalone'] expect(vtl.visible).toBe(true) - expect(adapter._vectorTileOpacityLayers['esri-standalone'].opacity) + expect(adapter._mapOpacityLayers['esri-standalone'].opacity) .toBe(datasetRegistry.getDataset('esri-standalone').opacity) }) it('applies paint properties for datasets with esriStyleLayerId', async () => { await adapter.addDataset('esri-standalone') - const vtl = adapter._vectorTileLayers['esri-standalone'] + const vtl = adapter._mapVisibilityLayers['esri-standalone'] expect(vtl.setPaintProperties).toHaveBeenCalledWith('standalone-style', expect.any(Object)) }) it('does not apply paint properties for server-style datasets', async () => { await adapter.addDataset('esri-server') - const vtl = adapter._vectorTileLayers['esri-server'] + const vtl = adapter._mapVisibilityLayers['esri-server'] expect(vtl.setPaintProperties).not.toHaveBeenCalled() }) it('creates a group layer when adding a grouped dataset', async () => { await adapter.addDataset('esri-grouped') - expect(adapter._vectorTileLayers['esri-grouped']).toBeDefined() + expect(adapter._mapVisibilityLayers['esri-grouped']).toBeDefined() expect(adapter._groupLayers['my-group']).toBeDefined() }) }) @@ -122,36 +122,36 @@ describe('esriLayerAdapter', () => { it('removes a standalone vectorTileLayer from the map and clears internal state', async () => { await adapter._addLayers(datasetRegistry.getDataset('esri-standalone')) - const vtl = adapter._vectorTileLayers['esri-standalone'] + const vtl = adapter._mapVisibilityLayers['esri-standalone'] await adapter.removeDataset('esri-standalone') expect(map.remove).toHaveBeenCalledWith(vtl) - expect(adapter._vectorTileLayers['esri-standalone']).toBeUndefined() - expect(adapter._vectorTileOpacityLayers['esri-standalone']).toBeUndefined() + expect(adapter._mapVisibilityLayers['esri-standalone']).toBeUndefined() + expect(adapter._mapOpacityLayers['esri-standalone']).toBeUndefined() }) it('removes the vectorTileLayer from its group layer but keeps the group when other layers remain', async () => { await adapter._addLayers(datasetRegistry.getDataset('flood-zones-cc')) await adapter._addLayers(datasetRegistry.getDataset('flood-zones')) - const vtl = adapter._vectorTileLayers['flood-zones-cc'] + const vtl = adapter._mapVisibilityLayers['flood-zones-cc'] const groupLayer = adapter._groupLayers['flood-zones-group'] await adapter.removeDataset('flood-zones-cc') expect(groupLayer.remove).toHaveBeenCalledWith(vtl) expect(map.remove).not.toHaveBeenCalledWith(groupLayer) expect(adapter._groupLayers['flood-zones-group']).toBeDefined() - expect(adapter._vectorTileLayers['flood-zones-cc']).toBeUndefined() - expect(adapter._vectorTileOpacityLayers['flood-zones-cc']).toBeUndefined() + expect(adapter._mapVisibilityLayers['flood-zones-cc']).toBeUndefined() + expect(adapter._mapOpacityLayers['flood-zones-cc']).toBeUndefined() }) it('removes the group layer from the map when its last vectorTileLayer is removed', async () => { await adapter._addLayers(datasetRegistry.getDataset('esri-grouped')) - const vtl = adapter._vectorTileLayers['esri-grouped'] + const vtl = adapter._mapVisibilityLayers['esri-grouped'] const groupLayer = adapter._groupLayers['my-group'] await adapter.removeDataset('esri-grouped') expect(groupLayer.remove).toHaveBeenCalledWith(vtl) expect(map.remove).toHaveBeenCalledWith(groupLayer) expect(adapter._groupLayers['my-group']).toBeUndefined() - expect(adapter._vectorTileLayers['esri-grouped']).toBeUndefined() - expect(adapter._vectorTileOpacityLayers['esri-grouped']).toBeUndefined() + expect(adapter._mapVisibilityLayers['esri-grouped']).toBeUndefined() + expect(adapter._mapOpacityLayers['esri-grouped']).toBeUndefined() }) }) @@ -165,7 +165,7 @@ describe('esriLayerAdapter', () => { it('applies visibility for a known dataset', async () => { await adapter.applyDatasetVisibility('esri-standalone') - expect(adapter._vectorTileLayers['esri-standalone'].visible).toBe(true) + expect(adapter._mapVisibilityLayers['esri-standalone'].visible).toBe(true) }) it('does nothing for an unknown dataset', async () => { @@ -186,7 +186,7 @@ describe('esriLayerAdapter', () => { it('sets opacity on the opacity layer for a known dataset', async () => { await adapter._addLayers(datasetRegistry.getDataset('esri-standalone')) await adapter.applyDatasetOpacity('esri-standalone') - expect(adapter._vectorTileOpacityLayers['esri-standalone'].opacity) + expect(adapter._mapOpacityLayers['esri-standalone'].opacity) .toBe(datasetRegistry.getDataset('esri-standalone').opacity) }) @@ -206,14 +206,14 @@ describe('esriLayerAdapter', () => { it('sets opacity on all opacity layers', async () => { await adapter._addLayers(datasetRegistry.getDataset('esri-standalone')) await adapter.applyGlobalOpacity() - expect(adapter._vectorTileOpacityLayers['esri-standalone'].opacity) + expect(adapter._mapOpacityLayers['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 } + adapter._mapOpacityLayers['ghost-id'] = { opacity: 99 } await expect(adapter.applyGlobalOpacity()).resolves.not.toThrow() - expect(adapter._vectorTileOpacityLayers['ghost-id'].opacity).toBe(99) + expect(adapter._mapOpacityLayers['ghost-id'].opacity).toBe(99) }) }) @@ -227,19 +227,19 @@ describe('esriLayerAdapter', () => { it('calls setStyleLayerVisibility for datasets with esriStyleLayerId', async () => { await adapter.onMapStyleChange() - expect(adapter._vectorTileLayers['esri-standalone'].setStyleLayerVisibility) + expect(adapter._mapVisibilityLayers['esri-standalone'].setStyleLayerVisibility) .toHaveBeenCalledWith('standalone-style', expect.any(String)) }) it('calls setPaintProperties for datasets not using server style', async () => { await adapter.onMapStyleChange() - expect(adapter._vectorTileLayers['esri-standalone'].setPaintProperties) + expect(adapter._mapVisibilityLayers['esri-standalone'].setPaintProperties) .toHaveBeenCalledWith('standalone-style', expect.any(Object)) }) it('does not call setPaintProperties when useServerStyle is true', async () => { await adapter.onMapStyleChange() - expect(adapter._vectorTileLayers['esri-server'].setPaintProperties).not.toHaveBeenCalled() + expect(adapter._mapVisibilityLayers['esri-server'].setPaintProperties).not.toHaveBeenCalled() }) }) @@ -251,14 +251,14 @@ describe('esriLayerAdapter', () => { }) it('adds VectorTileLayers for all top-level datasets', async () => { - expect(adapter._vectorTileLayers['flood-zones-cc']).toBeDefined() - expect(adapter._vectorTileLayers['flood-zones']).toBeDefined() + expect(adapter._mapVisibilityLayers['flood-zones-cc']).toBeDefined() + expect(adapter._mapVisibilityLayers['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) + expect(adapter._mapVisibilityLayers['flood-zones-cc'].visible).toBe(true) + expect(adapter._mapVisibilityLayers['flood-zones'].visible).toBe(false) }) it('adds GroupLayers for datasets with esriGroupId', async () => { diff --git a/plugins/datasets/src/adapters/esri/registry/esriDataset.js b/plugins/datasets/src/adapters/esri/registry/esriDataset.js index 322b4318f..9a7f2be81 100644 --- a/plugins/datasets/src/adapters/esri/registry/esriDataset.js +++ b/plugins/datasets/src/adapters/esri/registry/esriDataset.js @@ -24,4 +24,23 @@ export class EsriDataset extends Dataset { get useServerStyle () { return Boolean(this._datasetDefinition.esriUseServerStyle) } + + get renderer () { + if (this.type !== 'FeatureService') { + return undefined + } + const rendererDefinition = this._datasetDefinition.style?.renderer || this.parent?.renderer + if (!rendererDefinition) { + return undefined + } + const { mapStyle } = datasetRegistry + const renderer = JSON.parse(JSON.stringify(rendererDefinition)) + if (renderer.symbol?.color) { + renderer.symbol.color = getValueForStyle(renderer.symbol.color, mapStyle.id) + } + if (renderer.symbol?.outline?.color) { + renderer.symbol.outline.color = getValueForStyle(renderer.symbol.outline.color, mapStyle.id) + } + return renderer + } } From a9a02484bd29bf88e54a7a58700d27956495bd66 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 17 Jul 2026 08:34:05 +0100 Subject: [PATCH 6/7] IM-399 ignore __data__ for coverage --- jest.config.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/jest.config.mjs b/jest.config.mjs index 02f1cd4dd..871620da9 100755 --- a/jest.config.mjs +++ b/jest.config.mjs @@ -22,6 +22,7 @@ export default { testPathIgnorePatterns: ['/src/test-utils.js'], coveragePathIgnorePatterns: [ '/__mocks__/', + '/__data__/', '/src/index.umd.js', '/stylelint.config.js', '/coverage', From a334730fdccf9a336e8c0f67b91e52942dac864e Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Fri, 17 Jul 2026 08:48:51 +0100 Subject: [PATCH 7/7] IM-399 Sonar issue handled --- jest.config.mjs | 2 +- plugins/datasets/src/adapters/esri/esriLayerAdapter.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/jest.config.mjs b/jest.config.mjs index 871620da9..c2981305b 100755 --- a/jest.config.mjs +++ b/jest.config.mjs @@ -28,7 +28,7 @@ export default { '/coverage', '/demo', '/src/test-utils.js', - '/plugins/datasets/', + // '/plugins/datasets/', '/providers/beta/', '/plugins/beta/draw-es', '/plugins/beta/draw-ml', diff --git a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js index ab80862b8..76341baaa 100644 --- a/plugins/datasets/src/adapters/esri/esriLayerAdapter.js +++ b/plugins/datasets/src/adapters/esri/esriLayerAdapter.js @@ -78,6 +78,7 @@ export default class EsriLayerAdapter extends LayerAdapter { } catch (error) { logger.error(`Error adding FeatureLayer for dataset ${registryDataset.id}:`, error) } + return Promise.resolve() // Return a resolved promise to avoid unhandled promise rejection } async _addLayers (registryDataset) {