diff --git a/demo/js/index.js b/demo/js/index.js index cebe9211..47e29c4a 100755 --- a/demo/js/index.js +++ b/demo/js/index.js @@ -314,7 +314,17 @@ interactiveMap.on('map:ready', function (e) { interactiveMap.on('datasets:ready', function () { // setTimeout(() => datasetsPlugin.setFeatureVisibility(false, [55], { datasetId: 'land-covers', idProperty: null }), 2000) // setTimeout(() => datasetsPlugin.setFeatureVisibility(true, [55], { datasetId: 'land-covers', idProperty: null }), 4000) - // setTimeout(() => datasetsPlugin.setStyle({ stroke: { outdoor: '#ff0000', dark: '#ffffff' }, fillPattern: 'horizontal-hatch', fillPatternForegroundColor: { outdoor: '#ff0000', dark: '#ffffff' } }, { datasetId: 'land-covers', sublayerId: '130' }), 2000) + + setTimeout(() => datasetsPlugin.setStyle( + { + stroke: { outdoor: '#ff0000', dark: '#ffffff' }, + fillPattern: 'horizontal-hatch', + fillPatternForegroundColor: { outdoor: '#ff0000', dark: '#ffffff' }, + fillPatternBackgroundColor: 'transparent' + }, + { datasetId: 'land-covers', sublayerId: '130-131' }), 2000) + // setTimeout(() => datasetsPlugin.setDatasetVisibility(true, { datasetId: 'hedge-control' }), 500) + // setTimeout(() => datasetsPlugin.setStyle({ stroke: { outdoor: '#ffff00' }, }, { datasetId: 'hedge-control' }), 1000) }) // Ref to the selected features diff --git a/plugins/beta/datasets/src/DatasetsInit.jsx b/plugins/beta/datasets/src/DatasetsInit.jsx index b792e71b..b62be658 100755 --- a/plugins/beta/datasets/src/DatasetsInit.jsx +++ b/plugins/beta/datasets/src/DatasetsInit.jsx @@ -1,7 +1,6 @@ // src/plugins/datasets/datasetsInit.jsx import { useEffect, useRef } from 'react' import { EVENTS } from '../../../../src/config/events.js' -import { datasetDefaults } from './defaults.js' import { createDatasets } from './datasets.js' export function DatasetsInit ({ pluginConfig, pluginState, appState, mapState, mapProvider, services }) { @@ -56,7 +55,6 @@ export function DatasetsInit ({ pluginConfig, pluginState, appState, mapState, m }, [isMapStyleReady, appState.mode]) useEffect(() => { - // dispatch({ type: 'BUILD_MAPPED_DATASETS', payload: null }) dispatch({ type: 'BUILD_KEY_GROUPS', payload: null }) }, [pluginState.datasets]) diff --git a/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.js b/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.js index bdd99ed6..3f731777 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.js +++ b/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.js @@ -108,7 +108,7 @@ export const addSymbolLayer = (map, dataset, layerId, sourceId, sourceLayer, vis export const addSublayerLayers = (map, dataset, sublayer, sourceId, sourceLayer, { mapStyle, symbolRegistry, patternRegistry, pixelRatio }) => { const mapStyleId = mapStyle.id const merged = mergeSublayer(dataset, sublayer) - const { fillLayerId, strokeLayerId, symbolLayerId } = getSublayerLayerIds(dataset.id, sublayer.id) + const { fillLayerId, strokeLayerId, symbolLayerId } = getSublayerLayerIds(dataset.id, sublayer.sublayerId ? sublayer.sublayerId : sublayer.id) const parentHidden = dataset.visibility === 'hidden' const sublayerHidden = dataset.sublayerVisibility?.[sublayer.id] === 'hidden' const visibility = (parentHidden || sublayerHidden) ? 'none' : 'visible' diff --git a/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.test.js b/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.test.js index 0b973a2a..61843233 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.test.js +++ b/plugins/beta/datasets/src/adapters/maplibre/layerBuilders.test.js @@ -386,7 +386,7 @@ describe('addSublayerLayers', () => { getSymbolDef.mockReturnValue({ id: 'marker' }) getSymbolImageId.mockReturnValue('marker-img') const dataset = { id: 'ds', visibility: 'visible' } - const sublayer = { id: 'sl' } + const sublayer = { id: 'ds-sl', sublayerId: 'sl' } mergeSublayer.mockReturnValue({ symbol: 'marker' }) addSublayerLayers(map, dataset, sublayer, 'source-id', undefined, { mapStyle, symbolRegistry, patternRegistry, pixelRatio: 1 }) expect(map.addLayer).toHaveBeenCalledWith(expect.objectContaining({ type: 'symbol' })) diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js index 961c6b64..d2f49d83 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js @@ -292,23 +292,22 @@ export default class MaplibreLayerAdapter { * @param {Object} mapStyle * @returns {Promise} */ - async setSublayerStyle (dataset, sublayerId, mapStyle) { + async setSublayerStyle (dataset, sublayer, mapStyle) { + if (!sublayer) { + return + } const mapStyleId = mapStyle.id const pixelRatio = this._pixelRatio - const { fillLayerId, strokeLayerId, symbolLayerId } = getSublayerLayerIds(dataset.id, sublayerId) + const { fillLayerId, strokeLayerId, symbolLayerId } = getSublayerLayerIds(dataset.id, sublayer.sublayerId) ;[fillLayerId, strokeLayerId, symbolLayerId].forEach(layerId => { if (this._map.getLayer(layerId)) { this._map.removeLayer(layerId) } this._symbolLayerIds.delete(layerId) }) - const sublayer = dataset.sublayers?.find(s => s.id === sublayerId) - if (!sublayer) { - return - } - await Promise.all([ - this._mapProvider.registerPatterns(getPatternConfigs([dataset], this._patternRegistry), mapStyleId, this._patternRegistry), - this._mapProvider.registerSymbols(getSymbolConfigs([dataset]), mapStyle, this._symbolRegistry) + await Promise.all([ // Add pattern and symbol images to the map before re-adding layers, so they're available for use in the new style. + this._mapProvider.registerPatterns([sublayer.style], mapStyleId, this._patternRegistry), + this._mapProvider.registerSymbols([sublayer.style], mapStyle, this._symbolRegistry) ]) const sourceId = this._datasetSourceMap.get(dataset.id) const sourceLayer = dataset.tiles?.length ? dataset.sourceLayer : undefined diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js index a415f683..d579fec1 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js @@ -365,8 +365,8 @@ describe('setSublayerStyle', () => { it('removes existing sublayer layers before re-adding', async () => { const { adapter, map } = makeAdapter({ 'ds-sl': 'fill', 'ds-sl-stroke': 'line', 'ds-sl-symbol': 'symbol' }) adapter._datasetSourceMap.set('ds', 'source-ds') - const ds = { ...dataset, sublayers: [{ id: 'sl' }] } - await adapter.setSublayerStyle(ds, 'sl', mapStyle) + const sublayer = { id: 'ds-sl', sublayerId: 'sl' } + await adapter.setSublayerStyle(dataset, sublayer, mapStyle) expect(map.removeLayer).toHaveBeenCalledWith('ds-sl') expect(map.removeLayer).toHaveBeenCalledWith('ds-sl-stroke') expect(map.removeLayer).toHaveBeenCalledWith('ds-sl-symbol') @@ -380,10 +380,9 @@ describe('setSublayerStyle', () => { expect(addSublayerLayers).toHaveBeenCalled() }) - it('does nothing if sublayer is not found', async () => { + it('does nothing if sublayer does not exist', async () => { const { adapter } = makeAdapter() - const ds = { ...dataset, sublayers: [] } - await adapter.setSublayerStyle(ds, 'missing', mapStyle) + await adapter.setSublayerStyle(dataset, null, mapStyle) expect(addSublayerLayers).not.toHaveBeenCalled() }) }) @@ -743,9 +742,10 @@ describe('setSublayerStyle — tiled dataset', () => { const { adapter } = makeAdapter() const tiledDataset = { ...dataset, tiles: ['https://tiles/{z}/{x}/{y}'], sourceLayer: 'buildings', sublayers: [{ id: 'sl' }] } adapter._datasetSourceMap.set('ds', 'source-ds') - await adapter.setSublayerStyle(tiledDataset, 'sl', mapStyle) + const sublayer = { id: 'ds-sl', sublayerId: 'sl' } + await adapter.setSublayerStyle(tiledDataset, sublayer, mapStyle) expect(addSublayerLayers).toHaveBeenCalledWith( - adapter._map, tiledDataset, { id: 'sl' }, 'source-ds', 'buildings', expect.any(Object) + adapter._map, tiledDataset, sublayer, 'source-ds', 'buildings', expect.any(Object) ) }) }) diff --git a/plugins/beta/datasets/src/api/setStyle.js b/plugins/beta/datasets/src/api/setStyle.js index 7da65bd6..0753ed3c 100644 --- a/plugins/beta/datasets/src/api/setStyle.js +++ b/plugins/beta/datasets/src/api/setStyle.js @@ -1,22 +1,10 @@ -export const setStyle = ({ pluginState, mapState }, style, { datasetId, sublayerId } = {}) => { +export const setStyle = ({ pluginState, mapState }, styleChanges, { datasetId, sublayerId } = {}) => { const dataset = pluginState.datasets?.find(d => d.id === datasetId) if (!dataset) { return } - - if (sublayerId) { - pluginState.dispatch({ type: 'SET_SUBLAYER_STYLE', payload: { datasetId, sublayerId, styleChanges: style } }) - const updatedSublayerDataset = { - ...dataset, - sublayers: dataset.sublayers?.map(sublayer => - sublayer.id === sublayerId ? { ...sublayer, style: { ...sublayer.style, ...style } } : sublayer - ) - } - pluginState.layerAdapter?.setSublayerStyle(updatedSublayerDataset, sublayerId, mapState.mapStyle) - return - } - - pluginState.dispatch({ type: 'SET_DATASET_STYLE', payload: { datasetId, styleChanges: style } }) - const updatedDataset = { ...dataset, ...style } - pluginState.layerAdapter?.setStyle(updatedDataset, mapState.mapStyle) + const mapStyle = mapState.mapStyle + const type = sublayerId ? 'SET_SUBLAYER_STYLE' : 'SET_DATASET_STYLE' + const payload = { datasetId, styleChanges, mapStyle, sublayerId } + pluginState.dispatch({ type, payload }) } diff --git a/plugins/beta/datasets/src/panels/Layers.jsx b/plugins/beta/datasets/src/panels/Layers.jsx index c5273e5d..8091674c 100755 --- a/plugins/beta/datasets/src/panels/Layers.jsx +++ b/plugins/beta/datasets/src/panels/Layers.jsx @@ -1,4 +1,4 @@ -import React from 'react' +import React, { useEffect } from 'react' import { setDatasetVisibility } from '../api/setDatasetVisibility' const CHECKBOX_LABEL_CLASS = 'im-c-datasets-layers__item-label govuk-label govuk-checkboxes__label' @@ -70,6 +70,13 @@ export const Layers = ({ pluginState }) => { ) } + // TODO - remove this useEffect - it's useful while refactoring the state + useEffect(() => { + // console.log('menu:', pluginState.menu) + console.log('datasets:', pluginState.datasets) + console.log('mappedDatasets:', pluginState.mappedDatasets) + // console.log('orderedDatasets:', pluginState.orderedDatasets) + }, [pluginState.datasets, pluginState.mappedDatasets]) const visibleDatasets = (pluginState.datasets || []) .filter(dataset => dataset.showInMenu || hasToggleableSublayers(dataset)) diff --git a/plugins/beta/datasets/src/reducer.js b/plugins/beta/datasets/src/reducer.js index addf7410..919e11d3 100755 --- a/plugins/beta/datasets/src/reducer.js +++ b/plugins/beta/datasets/src/reducer.js @@ -137,9 +137,14 @@ const setSublayerVisibility = (state, payload) => { } const setDatasetStyle = (state, payload) => { - const { datasetId, styleChanges } = payload + const { datasetId, styleChanges, mapStyle } = payload + const { layerAdapter } = state + const dataset = { ...state.mappedDatasets[datasetId], ...styleChanges } + // TODO - handle this side effect better + layerAdapter?.setStyle(dataset, mapStyle) return { ...state, + mappedDatasets: { ...state.mappedDatasets, [datasetId]: dataset }, datasets: state.datasets?.map(dataset => dataset.id === datasetId ? { ...dataset, ...styleChanges } : dataset ) @@ -147,9 +152,17 @@ const setDatasetStyle = (state, payload) => { } const setSublayerStyle = (state, payload) => { - const { datasetId, sublayerId, styleChanges } = payload + const { datasetId, sublayerId, styleChanges, mapStyle } = payload + const { layerAdapter } = state + const id = `${datasetId}-${sublayerId}` + const dataset = state.mappedDatasets[datasetId] + const style = { ...state.mappedDatasets[id].style, ...styleChanges } + const subLayer = { ...state.mappedDatasets[id], style } + // TODO - handle this side effect better + layerAdapter.setSublayerStyle(dataset, subLayer, mapStyle) return { ...state, + mappedDatasets: { ...state.mappedDatasets, [id]: subLayer }, datasets: state.datasets?.map(dataset => { if (dataset.id !== datasetId) { return dataset @@ -207,7 +220,6 @@ const setSublayerOpacity = (state, payload) => { const setLayerAdapter = (state, payload) => ({ ...state, layerAdapter: payload }) const actions = { - BUILD_MAPPED_DATASETS: mappedDatasetsReducer, BUILD_KEY_GROUPS: keyReducer, SET_DATASETS: setDatasets, ADD_DATASET: addDataset, diff --git a/plugins/beta/datasets/src/reducers/mappedDatasetsReducer.js b/plugins/beta/datasets/src/reducers/mappedDatasetsReducer.js index 42defca6..6666b1d4 100644 --- a/plugins/beta/datasets/src/reducers/mappedDatasetsReducer.js +++ b/plugins/beta/datasets/src/reducers/mappedDatasetsReducer.js @@ -1,6 +1,9 @@ const flattenSublayer = (parentId, sublayer) => { const id = `${parentId}-${sublayer.id}` const sublayerId = sublayer.id + if (sublayer.visibility !== 'hidden') { + sublayer.visibility = 'visible' + } return { ...sublayer, id, parentId, sublayerId } } @@ -13,7 +16,6 @@ const reduceDatasets = (acc, dataset) => { if (flattenedSublayers?.length) { const sublayerIds = flattenedSublayers?.map(sublayer => sublayer.id) const sublayers = flattenedSublayers?.reduce(reduceDatasets, { orderedDatasets }) - // orderedDatasets.push(...sublayerIds) acc[id].sublayerIds = sublayerIds delete acc[id].sublayers return { ...acc, ...sublayers, orderedDatasets } diff --git a/plugins/beta/datasets/src/reducers/mappedDatasetsReducer.test.js b/plugins/beta/datasets/src/reducers/mappedDatasetsReducer.test.js new file mode 100644 index 00000000..116be00f --- /dev/null +++ b/plugins/beta/datasets/src/reducers/mappedDatasetsReducer.test.js @@ -0,0 +1,116 @@ +import { mappedDatasetsReducer } from './mappedDatasetsReducer' + +describe('mappedDatasetsReducer', () => { + it('handles empty datasets', () => { + const result = mappedDatasetsReducer({ datasets: [] }) + expect(result.mappedDatasets).toEqual({}) + expect(result.orderedDatasets).toEqual([]) + }) + + it('handles missing datasets', () => { + const result = mappedDatasetsReducer({}) + expect(result.mappedDatasets).toEqual({}) + expect(result.orderedDatasets).toEqual([]) + }) + + it('maps a single dataset with no sublayers', () => { + const state = { + datasets: [{ id: 'roads', label: 'Roads', minZoom: 10 }] + } + const result = mappedDatasetsReducer(state) + expect(result.mappedDatasets).toEqual({ + roads: { id: 'roads', label: 'Roads', minZoom: 10 } + }) + expect(result.orderedDatasets).toEqual(['roads']) + }) + + it('maps multiple datasets preserving order', () => { + const state = { + datasets: [ + { id: 'alpha', label: 'Alpha' }, + { id: 'beta', label: 'Beta' }, + { id: 'gamma', label: 'Gamma' } + ] + } + const result = mappedDatasetsReducer(state) + expect(result.orderedDatasets).toEqual(['alpha', 'beta', 'gamma']) + expect(Object.keys(result.mappedDatasets)).toEqual(['alpha', 'beta', 'gamma']) + }) + + it('preserves other state properties', () => { + const state = { + datasets: [], + someOtherProp: 'value', + nested: { foo: 'bar' } + } + const result = mappedDatasetsReducer(state) + expect(result.someOtherProp).toBe('value') + expect(result.nested).toEqual({ foo: 'bar' }) + }) + + describe('with sublayers', () => { + const state = { + datasets: [{ + id: 'land-covers', + label: 'Land covers', + sublayers: [ + { id: 'grassland', label: 'Grassland' }, + { id: 'woodland', label: 'Woodland', visibility: 'hidden' } + ] + }] + } + + it('removes the sublayers property from the parent dataset', () => { + const result = mappedDatasetsReducer(state) + expect(result.mappedDatasets['land-covers'].sublayers).toBeUndefined() + }) + + it('adds sublayerIds to the parent dataset', () => { + const result = mappedDatasetsReducer(state) + expect(result.mappedDatasets['land-covers'].sublayerIds).toEqual([ + 'land-covers-grassland', + 'land-covers-woodland' + ]) + }) + + it('adds flattened sublayers to mappedDatasets', () => { + const result = mappedDatasetsReducer(state) + expect(result.mappedDatasets['land-covers-grassland']).toBeDefined() + expect(result.mappedDatasets['land-covers-woodland']).toBeDefined() + }) + + it('sets compound id on flattened sublayer', () => { + const result = mappedDatasetsReducer(state) + expect(result.mappedDatasets['land-covers-grassland'].id).toBe('land-covers-grassland') + }) + + it('sets parentId on flattened sublayer', () => { + const result = mappedDatasetsReducer(state) + expect(result.mappedDatasets['land-covers-grassland'].parentId).toBe('land-covers') + }) + + it('sets sublayerId to the original sublayer id', () => { + const result = mappedDatasetsReducer(state) + expect(result.mappedDatasets['land-covers-grassland'].sublayerId).toBe('grassland') + }) + + it('sets visibility to visible for sublayers without explicit visibility', () => { + const result = mappedDatasetsReducer(state) + expect(result.mappedDatasets['land-covers-grassland'].visibility).toBe('visible') + }) + + it('preserves hidden visibility on sublayers', () => { + const result = mappedDatasetsReducer(state) + expect(result.mappedDatasets['land-covers-woodland'].visibility).toBe('hidden') + }) + + it('includes sublayers in orderedDatasets after their parent', () => { + const result = mappedDatasetsReducer(state) + expect(result.orderedDatasets).toEqual([ + 'land-covers', + 'land-covers-grassland', + 'land-covers-woodland' + ]) + }) + }) +})