From 8afc2a71df4db697bc37f1a0413febe0a183f613 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Mon, 10 Aug 2026 12:08:51 +0100 Subject: [PATCH 1/2] IM-425 Dataset keys are now grouped by groupLabel correctly --- demo/js/esri-datasets.js | 20 +++---- demo/js/ml-datasets.js | 9 +++- plugins/datasets/src/registry/dataset.js | 4 +- .../datasets/src/registry/datasetRegistry.js | 54 ++++++++++--------- 4 files changed, 46 insertions(+), 41 deletions(-) diff --git a/demo/js/esri-datasets.js b/demo/js/esri-datasets.js index ddfcc39c..c068c643 100644 --- a/demo/js/esri-datasets.js +++ b/demo/js/esri-datasets.js @@ -40,8 +40,8 @@ const nonFloodZoneDepthBandsDark = [COLOURS.depthOver2300.dark, COLOURS.depth230 const datasetFloodZonesCC = { id: 'floodzonescc', - label: 'Flood Zones Climate Change', - groupLabel: 'Datasets', + label: 'Flood zones', + groupLabel: 'Flood zones', esriGroupId: 'floodzones-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, @@ -116,7 +116,7 @@ const datasetFloodZonesCC = { const datasetFloodZones = { id: 'floodzones', label: 'Flood Zones', - groupLabel: 'Datasets', + groupLabel: 'Flood zones', esriGroupId: 'floodzones-group', tiles: `https://tiles.arcgis.com/tiles/JZM7qJpmv7vJ0Hzx/arcgis/rest/services/Flood_Zones_2_and_3_Rivers_and_Sea_NON_PRODUCTION/VectorTileServer`, showInKey: true, @@ -151,7 +151,7 @@ const surfaceWaterDatasetGenerator = ({id, tileName, sourceLayer, timeframe, aep const extentsDataset = { id: `${id}-extents`, label: 'Surface Water', - groupLabel: 'Datasets', + groupLabel: 'Surface Water', tiles: `https://tiles.arcgis.com/tiles/JZM7qJpmv7vJ0Hzx/arcgis/rest/services/${tileName}/VectorTileServer`, showInKey: true, sourceLayer, @@ -206,7 +206,7 @@ const surfaceWaterDatasetGenerator = ({id, tileName, sourceLayer, timeframe, aep const depthDataset = { id: `${id}-depths`, label: 'Surface Water Depth All', - groupLabel: 'Datasets', + groupLabel: 'Surface Water', tiles: `https://tiles.arcgis.com/tiles/JZM7qJpmv7vJ0Hzx/arcgis/rest/services/${tileName}/VectorTileServer`, showInKey: true, sourceLayer, @@ -276,7 +276,7 @@ const surfaceWaterDatasetGenerator = ({id, tileName, sourceLayer, timeframe, aep const surfaceWaterExtentsKey = { id: 'surfacewater-extents-key', label: 'Surface Water', - groupLabel: 'Datasets', + groupLabel: 'Surface Water', showInKey: true, style: { stroke: { outdoor: nonFloodZoneLight, dark: nonFloodZoneDark }, @@ -605,14 +605,6 @@ interactiveMap.on('map:ready', function ({ map, view, mapStyleId, mapSize, crs } console.log('map:ready', { map, view, mapStyleId, mapSize, crs }) mapState.map = map mapState.view = view - - interactiveMap.addPanel('help-banner', { - label: 'Click on the flood zones for information', - html: 'Alert:', - mobile: { slot: 'banner', dismissible: true }, - tablet: { slot: 'banner', dismissible: true, width: '372px' }, - desktop: { slot: 'banner', dismissible: true, width: '372px' } - }) }) let visibleLayers = null diff --git a/demo/js/ml-datasets.js b/demo/js/ml-datasets.js index 5601ea42..4d2e79e7 100644 --- a/demo/js/ml-datasets.js +++ b/demo/js/ml-datasets.js @@ -109,6 +109,7 @@ const framePlugin = createFramePlugin({ aspectRatio: 1.5 }) const landCoversDataset = { id: 'land-covers', label: 'Land covers', + groupLabel: 'Land covers and hedge control', dynamicGeoJSON: { idProperty: 'id', // required - the ID that identifies individual features url: `${process.env.FARMING_API_URL}/api/collections/parcels/items?sbi=106325052`, // required @@ -140,6 +141,8 @@ const landCoversDataset = { sublayers: [{ id: '130-131', label: 'Permanent grassland', + showInKey: true, + groupLabel: 'Grassland covers', filter: ['in', ['get', 'dominant_land_cover'], ['literal', ['130', '131']]], // 'dominant_land_cover = "130"' showInMenu: true, style: { @@ -151,6 +154,8 @@ const landCoversDataset = { }, { id: 'permanent-grassland-2', label: 'Permanent grassland 2', + showInKey: true, + groupLabel: 'Grassland covers', filter: ['in', ['get', 'dominant_land_cover'], ['literal', ['130', '131']]], // 'dominant_land_cover = "130"' showInMenu: true, visible: false, @@ -211,7 +216,7 @@ const landCoversDataset = { const existingFieldsDataset = { id: 'existing-fields', label: 'Existing fields', - groupLabel: 'Test group', + // groupLabel: 'Land covers and hedge control', filter: ['all', ['==', ['get', 'sbi'], '106223377'], ['==', ['get', 'is_dominant_land_cover'], true]], tiles: ['https://farming-tiles-702a60f45633.herokuapp.com/field_parcels_with_hedges/{z}/{x}/{y}'], sourceLayer: 'field_parcels_filtered', @@ -280,7 +285,7 @@ const historicMonumentsDataset = { const hedgeControlDataset = { id: 'hedge-control', label: 'Hedge control', - groupLabel: 'Test group', + groupLabel: 'Land covers and hedge control', tiles: ['https://farming-tiles-702a60f45633.herokuapp.com/field_parcels_with_hedges/{z}/{x}/{y}'], sourceLayer: 'hedge_control', minZoom: 10, diff --git a/plugins/datasets/src/registry/dataset.js b/plugins/datasets/src/registry/dataset.js index cb1e178f..95f7c9dd 100644 --- a/plugins/datasets/src/registry/dataset.js +++ b/plugins/datasets/src/registry/dataset.js @@ -41,7 +41,9 @@ export class Dataset { return this._datasetDefinition.visibleWhen } - get groupLabel () { return this._datasetDefinition.groupLabel } + // A sublayer effectively has a groupLabel, even if it or its parent doesn't, + // as it will be grouped with its siblings under the parents label if it doesn't. + get groupLabel () { return this._datasetDefinition.groupLabel || this.parent?.groupLabel || this.parent?.label } get opacity () { const myOpacity = this.style?.opacity diff --git a/plugins/datasets/src/registry/datasetRegistry.js b/plugins/datasets/src/registry/datasetRegistry.js index c6c99373..29b7074a 100644 --- a/plugins/datasets/src/registry/datasetRegistry.js +++ b/plugins/datasets/src/registry/datasetRegistry.js @@ -92,35 +92,41 @@ const datasetRegistry = { return this._lastKeyItems } this._lastKeyItemsDatasets = this.datasets - const items = [] - const seenGroups = new Set() - let hasGroups = false - this.forEachDataset((dataset) => { + const _items = [] + const groups = new Map() + + const getOrCreateGroup = (groupLabel) => { + if (groups.has(groupLabel)) { + return groups.get(groupLabel) + } + const groupObject = { + type: 'group', + groupLabel, + datasets: [] + } + groups.set(groupLabel, groupObject) + _items.push(groupObject) + return groupObject + } + + this.forEach((dataset) => { if (!(dataset.showInKey && dataset.keyVisibility)) { return } - if (dataset.hasSublayers) { - const sublayers = dataset.sublayers.filter(sublayer => sublayer.keyVisibility) - if (sublayers.length) { - hasGroups = true - items.push({ type: 'sublayers', dataset, sublayers }) - } - } else if (dataset.groupLabel) { - if (seenGroups.has(dataset.groupLabel)) { - return - } - seenGroups.add(dataset.groupLabel) - hasGroups = true - items.push({ - type: 'group', - groupLabel: dataset.groupLabel, - datasets: this.topLevelDatasets() - .filter(groupDataset => (groupDataset.groupLabel === dataset.groupLabel && !groupDataset.hasSublayers && groupDataset.keyVisibility)) - }) - } else { - items.push({ type: 'flat', dataset }) + const isGroup = dataset.hasSublayers || dataset.groupLabel + if (!isGroup) { + _items.push({ type: 'flat', dataset }) + return + } + + const groupLabel = dataset.groupLabel || dataset.label + const groupObject = getOrCreateGroup(groupLabel) + if (!dataset.hasSublayers) { + groupObject.datasets.push(dataset) } }) + const items = _items.filter((item) => item.type === 'flat' || Boolean(item.datasets?.length)) + const hasGroups = items.some(item => item.type === 'group') this._lastKeyItems = { items, hasGroups } return this._lastKeyItems } From 59c47116a5675c33e44bde9114ed9019c1ffc038 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Mon, 10 Aug 2026 13:12:22 +0100 Subject: [PATCH 2/2] IM-425 existing tests now passing --- .../src/registry/datasetRegistry.test.js | 43 +++++++++++-------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/plugins/datasets/src/registry/datasetRegistry.test.js b/plugins/datasets/src/registry/datasetRegistry.test.js index e9ca189c..22c71943 100644 --- a/plugins/datasets/src/registry/datasetRegistry.test.js +++ b/plugins/datasets/src/registry/datasetRegistry.test.js @@ -121,10 +121,10 @@ describe('datasetRegistry', () => { expect(typeof hasGroups).toBe('boolean') }) - it('includes a sublayers entry for each dataset that has sublayers', () => { + it('includes a group entry for each dataset that has sublayers', () => { const { items } = datasetRegistry.keyItems() - const sublayerItems = items.filter(item => item.type === 'sublayers') - expect(sublayerItems).toHaveLength(2) // land-covers and historic-monuments + const groupItems = items.filter(item => item.type === 'group') + expect(groupItems).toHaveLength(2) // land-covers and historic-monuments }) it('includes a flat entry for datasets without sublayers and no groupLabel', () => { @@ -139,13 +139,13 @@ describe('datasetRegistry', () => { }) it('sets hasGroups to false when all items are flat with no groupLabel', () => { - datasetRegistry.attach({ simple: { id: 'simple', showInKey: true, visible: true, style: {} } }) + datasetRegistry.attach({ simple: { id: 'simple', showInKey: true, visible: true, style: {} } }, ['simple']) const { hasGroups } = datasetRegistry.keyItems() expect(hasGroups).toBe(false) }) it('sets hasGroups to false when a dataset has only one sublayer and it is not visible', () => { - const { mappedDatasets: singleHiddenSublayer } = mappedDatasetsReducer({ + const { mappedDatasets: singleHiddenSublayer, orderedDatasets } = mappedDatasetsReducer({ datasets: [{ id: 'single-sublayer-dataset', showInKey: true, @@ -154,38 +154,38 @@ describe('datasetRegistry', () => { sublayers: [{ id: 'sub', visible: false, style: {} }] }] }) - datasetRegistry.attach(singleHiddenSublayer) + datasetRegistry.attach(singleHiddenSublayer, orderedDatasets) const { hasGroups } = datasetRegistry.keyItems() expect(hasGroups).toBe(false) }) - it('only includes visible sublayers in sublayers items', () => { + it('only includes visible sublayers in group items', () => { const { items } = datasetRegistry.keyItems() - const landCoversItem = items.find(item => item.type === 'sublayers' && item.dataset.id === 'land-covers') - const sublayerIds = landCoversItem.sublayers.map(s => s.id) + const landCoversItem = items.find(item => item.type === 'group' && item.groupLabel === 'Land covers') + const sublayerIds = landCoversItem.datasets.map(s => s.id) expect(sublayerIds).not.toContain('land-covers-379') // land-covers-379 has visible: false }) it('includes a group entry for datasets sharing a groupLabel', () => { - const { mappedDatasets: grouped } = mappedDatasetsReducer({ datasets: datasetsWithGroups }) - datasetRegistry.attach(grouped) + const { mappedDatasets: grouped, orderedDatasets } = mappedDatasetsReducer({ datasets: datasetsWithGroups }) + datasetRegistry.attach(grouped, orderedDatasets) const { items } = datasetRegistry.keyItems() const groupItems = items.filter(item => item.type === 'group') - expect(groupItems).toHaveLength(1) - expect(groupItems[0].groupLabel).toBe('Test group') + expect(groupItems).toHaveLength(3) + expect(groupItems[1].groupLabel).toBe('Test group') }) it('only adds one group entry per unique groupLabel', () => { - const { mappedDatasets: grouped } = mappedDatasetsReducer({ datasets: datasetsWithGroups }) - datasetRegistry.attach(grouped) + const { mappedDatasets: grouped, orderedDatasets } = mappedDatasetsReducer({ datasets: datasetsWithGroups }) + datasetRegistry.attach(grouped, orderedDatasets) const { items } = datasetRegistry.keyItems() const groupLabels = items.filter(i => i.type === 'group').map(i => i.groupLabel) expect(groupLabels.length).toBe(new Set(groupLabels).size) }) it('populates group item datasets with matching showInKey datasets', () => { - const { mappedDatasets: grouped } = mappedDatasetsReducer({ datasets: datasetsWithGroups }) - datasetRegistry.attach(grouped) + const { mappedDatasets: grouped, orderedDatasets } = mappedDatasetsReducer({ datasets: datasetsWithGroups }) + datasetRegistry.attach(grouped, orderedDatasets) const { items } = datasetRegistry.keyItems() const groupItem = items.find(item => item.type === 'group' && item.groupLabel === 'Test group') const ids = groupItem.datasets.map(d => d.id) @@ -199,9 +199,16 @@ describe('datasetRegistry', () => { expect(result1).toBe(result2) }) + it('recalculates caches the cached result when invalidateKeyItems is called', () => { + const result1 = datasetRegistry.keyItems() + datasetRegistry.invalidateKeyItems() + const result2 = datasetRegistry.keyItems() + expect(result1).not.toBe(result2) + }) + it('recomputes when the datasets ref changes', () => { const result1 = datasetRegistry.keyItems() - datasetRegistry.attach({ simple: { id: 'simple', showInKey: true, visible: true, style: {} } }) + datasetRegistry.attach({ simple: { id: 'simple', showInKey: true, visible: true, style: {} } }, ['simple']) const result2 = datasetRegistry.keyItems() expect(result1).not.toBe(result2) })