From 2fba69a25268910d2d83124559905ec76240bc15 Mon Sep 17 00:00:00 2001 From: Mark Fee Date: Tue, 19 May 2026 17:48:33 +0100 Subject: [PATCH] IM-286 dataset visibility simplified --- plugins/beta/datasets/src/DatasetsInit.jsx | 2 +- .../adapters/maplibre/maplibreLayerAdapter.js | 66 ++++------------- .../maplibre/maplibreLayerAdapter.test.js | 74 +------------------ .../datasets/src/api/setDatasetVisibility.js | 34 +-------- plugins/beta/datasets/src/api/setStyle.js | 7 +- plugins/beta/datasets/src/defaults.js | 6 +- plugins/beta/datasets/src/defaults.test.js | 2 +- plugins/beta/datasets/src/reducer.js | 32 ++------ plugins/beta/datasets/src/registry/dataset.js | 16 +++- 9 files changed, 52 insertions(+), 187 deletions(-) diff --git a/plugins/beta/datasets/src/DatasetsInit.jsx b/plugins/beta/datasets/src/DatasetsInit.jsx index 77a10164..19fcd3e9 100755 --- a/plugins/beta/datasets/src/DatasetsInit.jsx +++ b/plugins/beta/datasets/src/DatasetsInit.jsx @@ -8,7 +8,6 @@ const useLayerAdapterActions = (methodName, dispatch, pluginState, dependencies) useEffect(() => { const methodParameters = pluginState.layerAdapterActions?.[methodName] || [] const method = pluginState.layerAdapter?.[methodName] - console.log('useEffect:', ...methodParameters.map((params) => `${params[0]},`)) if (method && methodParameters.length) { methodParameters.forEach((parameters) => { console.log(`calling ${methodName} with ${parameters[0]}`) @@ -82,6 +81,7 @@ export function DatasetsInit ({ pluginConfig, pluginState, appState, mapState, m datasetsRef.current = pluginState.mappedDatasets useEffect(() => datasetRegistry.attach(datasetsRef.current), [pluginState.mappedDatasets]) useLayerAdapterActions('setStyle', dispatch, pluginState, [pluginState.layerAdapterActions.setStyle]) + useLayerAdapterActions('setDatasetVisibility', dispatch, pluginState, [pluginState.layerAdapterActions.setDatasetVisibility]) // Cleanup only on unmount useEffect(() => { diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js index c64e2096..275bb1b5 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.js @@ -221,50 +221,6 @@ export default class MaplibreLayerAdapter { this._datasetSourceMap.delete(dataset.id) } - /** - * Make a dataset's layers visible. - * @param {string} datasetId - */ - showDataset (datasetId) { - this._setDatasetVisibility(datasetId, 'visible') - } - - /** - * Hide a dataset's layers. - * @param {string} datasetId - */ - hideDataset (datasetId) { - this._setDatasetVisibility(datasetId, 'none') - } - - /** - * Make a single sublayer's layers visible. - * @param {string} datasetId - * @param {string} sublayerId - */ - showSublayer (datasetId, sublayerId) { - const { fillLayerId, strokeLayerId, symbolLayerId } = getSublayerLayerIds(datasetId, sublayerId) - ;[fillLayerId, strokeLayerId, symbolLayerId].forEach(layerId => { - if (this._map.getLayer(layerId)) { - this._map.setLayoutProperty(layerId, 'visibility', 'visible') - } - }) - } - - /** - * Hide a single sublayer's layers. - * @param {string} datasetId - * @param {string} sublayerId - */ - hideSublayer (datasetId, sublayerId) { - const { fillLayerId, strokeLayerId, symbolLayerId } = getSublayerLayerIds(datasetId, sublayerId) - ;[fillLayerId, strokeLayerId, symbolLayerId].forEach(layerId => { - if (this._map.getLayer(layerId)) { - this._map.setLayoutProperty(layerId, 'visibility', 'none') - } - }) - } - // ─── Feature operations ───────────────────────────────────────────────────── /** @@ -388,19 +344,27 @@ export default class MaplibreLayerAdapter { }) } - _setDatasetVisibility (datasetId, visibility) { + setDatasetVisibility (datasetId) { + const registryDataset = datasetRegistry.getDataset(datasetId) const style = this._map.getStyle() if (!style?.layers) { return } // Covers base fill layer (datasetId) and all suffixed layers // (-stroke, -${sublayerId}, -${sublayerId}-stroke) without needing the dataset object. - style.layers - .filter(layer => - layer.id === datasetId || - layer.id.startsWith(`${datasetId}-`) - ) - .forEach(layer => this._map.setLayoutProperty(layer.id, 'visibility', visibility)) + if (registryDataset.hasSublayers) { + const { sublayerIds } = registryDataset + sublayerIds.forEach(sublayerId => { this.setDatasetVisibility(sublayerId) }) + } else { + const { visible } = registryDataset + const datasetId = registryDataset.id + style.layers.filter(layer => + layer.id === datasetId || layer.id.startsWith(`${datasetId}-`) + ).forEach(layer => { + const visibility = visible ? 'visible' : 'none' + this._map.setLayoutProperty(layer.id, 'visibility', visibility) + }) + } } _applyFeatureFilter (dataset, idProperty, excludeIds) { diff --git a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js index 17da54e1..c5fcd546 100644 --- a/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js +++ b/plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js @@ -2,7 +2,7 @@ import MaplibreLayerAdapter from './maplibreLayerAdapter' import { applyExclusionFilter } from '../../utils/filters.js' import { getSourceId, getLayerIds, getSublayerLayerIds, getAllLayerIds } from './layerIds.js' -import { addDatasetLayers, addSublayerLayers } from './layerBuilders.js' +import { addDatasetLayers } from './layerBuilders.js' import { hasPattern, getPatternImageId } from './patternImages.js' import { getSymbolImageId } from './symbolImages.js' import { mergeSublayer } from '../../utils/mergeSublayer.js' @@ -27,8 +27,7 @@ jest.mock('./layerIds.js', () => ({ })) jest.mock('./layerBuilders.js', () => ({ - addDatasetLayers: jest.fn(() => 'source-ds'), - addSublayerLayers: jest.fn() + addDatasetLayers: jest.fn(() => 'source-ds') })) jest.mock('./patternImages.js', () => ({ @@ -235,75 +234,6 @@ describe('removeDataset', () => { }) }) -// ─── showDataset / hideDataset ──────────────────────────────────────────────── - -describe('showDataset', () => { - it('sets visibility to visible on all matching layers', () => { - const { adapter, map } = makeAdapter() - map.getStyle.mockReturnValue({ - layers: [{ id: 'ds', type: 'fill' }, { id: 'ds-stroke', type: 'line' }] - }) - adapter.showDataset('ds') - expect(map.setLayoutProperty).toHaveBeenCalledWith('ds', 'visibility', 'visible') - expect(map.setLayoutProperty).toHaveBeenCalledWith('ds-stroke', 'visibility', 'visible') - }) - - it('does nothing when map style has no layers', () => { - const { adapter, map } = makeAdapter() - map.getStyle.mockReturnValue(null) - expect(() => adapter.showDataset('ds')).not.toThrow() - expect(map.setLayoutProperty).not.toHaveBeenCalled() - }) -}) - -describe('hideDataset', () => { - it('sets visibility to none on all matching layers', () => { - const { adapter, map } = makeAdapter() - map.getStyle.mockReturnValue({ - layers: [{ id: 'ds', type: 'fill' }, { id: 'ds-stroke', type: 'line' }] - }) - adapter.hideDataset('ds') - expect(map.setLayoutProperty).toHaveBeenCalledWith('ds', 'visibility', 'none') - expect(map.setLayoutProperty).toHaveBeenCalledWith('ds-stroke', 'visibility', 'none') - }) - - it('does not affect layers belonging to a different dataset', () => { - const { adapter, map } = makeAdapter() - map.getStyle.mockReturnValue({ - layers: [{ id: 'other', type: 'fill' }, { id: 'other-stroke', type: 'line' }] - }) - adapter.hideDataset('ds') - expect(map.setLayoutProperty).not.toHaveBeenCalled() - }) -}) - -// ─── showSublayer / hideSublayer ────────────────────────────────────────────── - -describe('showSublayer', () => { - it('sets visibility to visible for all three sublayer layer ids that exist', () => { - const { adapter, map } = makeAdapter({ 'ds-sl': 'fill', 'ds-sl-stroke': 'line', 'ds-sl-symbol': 'symbol' }) - adapter.showSublayer('ds', 'sl') - expect(map.setLayoutProperty).toHaveBeenCalledWith('ds-sl', 'visibility', 'visible') - expect(map.setLayoutProperty).toHaveBeenCalledWith('ds-sl-stroke', 'visibility', 'visible') - expect(map.setLayoutProperty).toHaveBeenCalledWith('ds-sl-symbol', 'visibility', 'visible') - }) - - it('skips layer ids that do not exist on the map', () => { - const { adapter, map } = makeAdapter({ 'ds-sl': 'fill' }) - adapter.showSublayer('ds', 'sl') - expect(map.setLayoutProperty).toHaveBeenCalledTimes(1) - }) -}) - -describe('hideSublayer', () => { - it('sets visibility to none for all three sublayer layer ids that exist', () => { - const { adapter, map } = makeAdapter({ 'ds-sl': 'fill', 'ds-sl-stroke': 'line' }) - adapter.hideSublayer('ds', 'sl') - expect(map.setLayoutProperty).toHaveBeenCalledWith('ds-sl', 'visibility', 'none') - expect(map.setLayoutProperty).toHaveBeenCalledWith('ds-sl-stroke', 'visibility', 'none') - }) -}) - // ─── showFeatures / hideFeatures ────────────────────────────────────────────── describe('showFeatures / hideFeatures', () => { diff --git a/plugins/beta/datasets/src/api/setDatasetVisibility.js b/plugins/beta/datasets/src/api/setDatasetVisibility.js index d3be96a5..1c90d030 100644 --- a/plugins/beta/datasets/src/api/setDatasetVisibility.js +++ b/plugins/beta/datasets/src/api/setDatasetVisibility.js @@ -1,37 +1,11 @@ -export const setDatasetVisibility = ({ pluginState }, visible, options = {}) => { - const { datasetId, sublayerId } = options - - if (sublayerId) { - const visibility = visible ? 'visible' : 'hidden' - pluginState.layerAdapter?.[visible ? 'showSublayer' : 'hideSublayer'](datasetId, sublayerId) - pluginState.dispatch({ type: 'SET_SUBLAYER_VISIBILITY', payload: { datasetId, sublayerId, visibility } }) - return - } +export const setDatasetVisibility = ({ pluginState }, visible, { datasetId, sublayerId }) => { + datasetId = sublayerId ? `${datasetId}-${sublayerId}` : datasetId if (datasetId) { - pluginState.layerAdapter?.[visible ? 'showDataset' : 'hideDataset'](datasetId) - pluginState.dispatch({ type: 'SET_DATASET_VISIBILITY', payload: { id: datasetId, visibility: visible ? 'visible' : 'hidden' } }) - if (visible) { - const dataset = pluginState.datasets?.find(d => d.id === datasetId) - Object.entries(dataset?.sublayerVisibility || {}).forEach(([subId, vis]) => { - if (vis === 'hidden') { - pluginState.layerAdapter?.hideSublayer(datasetId, subId) - } - }) - } + pluginState.dispatch({ type: 'SET_DATASET_VISIBILITY', payload: { datasetId, visible } }) return } // Global - pluginState.dispatch({ type: 'SET_GLOBAL_VISIBILITY', payload: { visibility: visible ? 'visible' : 'hidden' } }) - pluginState.datasets?.forEach(dataset => { - pluginState.layerAdapter?.[visible ? 'showDataset' : 'hideDataset'](dataset.id) - if (visible) { - Object.entries(dataset.sublayerVisibility || {}).forEach(([subId, vis]) => { - if (vis === 'hidden') { - pluginState.layerAdapter?.hideSublayer(dataset.id, subId) - } - }) - } - }) + pluginState.dispatch({ type: 'SET_GLOBAL_VISIBILITY', payload: { visible } }) } diff --git a/plugins/beta/datasets/src/api/setStyle.js b/plugins/beta/datasets/src/api/setStyle.js index 010a572c..58529738 100644 --- a/plugins/beta/datasets/src/api/setStyle.js +++ b/plugins/beta/datasets/src/api/setStyle.js @@ -3,9 +3,8 @@ export const setStyle = ({ pluginState, mapState }, styleChanges, { datasetId, s if (!dataset) { return } + datasetId = sublayerId ? `${datasetId}-${sublayerId}` : datasetId const mapStyle = mapState.mapStyle - // const type = sublayerId ? 'SET_SUBLAYER_STYLE' : 'SET_DATASET_STYLE' - const type = 'SET_DATASET_STYLE' - const payload = { datasetId: sublayerId ? `${datasetId}-${sublayerId}` : datasetId, styleChanges, mapStyle, sublayerId } - pluginState.dispatch({ type, payload }) + const payload = { datasetId, styleChanges, mapStyle, sublayerId } + pluginState.dispatch({ type: 'SET_DATASET_STYLE', payload }) } diff --git a/plugins/beta/datasets/src/defaults.js b/plugins/beta/datasets/src/defaults.js index 19149a41..7153f2dd 100644 --- a/plugins/beta/datasets/src/defaults.js +++ b/plugins/beta/datasets/src/defaults.js @@ -3,7 +3,7 @@ const datasetDefaults = { maxZoom: 24, showInKey: false, showInMenu: false, - visibility: 'visible', + visible: true, style: { stroke: '#d4351c', strokeWidth: 2, @@ -49,6 +49,10 @@ const applyDatasetDefaults = (dataset, defaults) => { } const applyDatasetDefaultsWithoutFlattening = (dataset) => { + // Allow for existing configs that use visibility instead of visible, but default to visible if neither is set + if (dataset.visible !== true && dataset.visible !== false) { + dataset.visible = dataset.visibility !== 'hidden' + } const datasetWithDefaults = { ...datasetDefaults, ...dataset, style: { ...datasetDefaults.style, ...dataset.style } } const style = dataset.style || {} diff --git a/plugins/beta/datasets/src/defaults.test.js b/plugins/beta/datasets/src/defaults.test.js index 21ab5ea1..626add11 100644 --- a/plugins/beta/datasets/src/defaults.test.js +++ b/plugins/beta/datasets/src/defaults.test.js @@ -7,7 +7,7 @@ describe('datasetDefaults', () => { maxZoom: 24, showInKey: false, showInMenu: false, - visibility: 'visible' + visible: true }) }) diff --git a/plugins/beta/datasets/src/reducer.js b/plugins/beta/datasets/src/reducer.js index 950f370c..14b0a342 100755 --- a/plugins/beta/datasets/src/reducer.js +++ b/plugins/beta/datasets/src/reducer.js @@ -20,7 +20,8 @@ const initialState = { hiddenFeatures: {}, // { [layerId]: { idProperty: string, ids: string[] } } layerAdapter: null, layerAdapterActions: { - setStyle: [] + setStyle: [], + setDatasetVisibility: [] } } @@ -70,13 +71,15 @@ const removeDataset = (state, payload) => { } const setDatasetVisibility = (state, payload) => { - const { id, visibility } = payload + const { datasetId, visible } = payload + const setDatasetVisibility = [...state.layerAdapterActions.setDatasetVisibility, [datasetId, visible]] return { ...state, + layerAdapterActions: { ...state.layerAdapterActions, setDatasetVisibility }, datasets: state.datasets?.map(dataset => - dataset.id === id ? { ...dataset, visibility } : dataset + dataset.id === datasetId ? { ...dataset, visible } : dataset ), - mappedDatasets: { ...state.mappedDatasets, [id]: { ...state.mappedDatasets[id], visibility } } + mappedDatasets: { ...state.mappedDatasets, [datasetId]: { ...state.mappedDatasets[datasetId], visible } } } } @@ -128,26 +131,6 @@ const showFeatures = (state, payload) => { } } -const setSublayerVisibility = (state, payload) => { - const { datasetId, sublayerId, visibility } = payload - const id = `${datasetId}-${sublayerId}` - return { - ...state, - mappedDatasets: { ...state.mappedDatasets, [id]: { ...state.mappedDatasets[id], visibility } }, - datasets: state.datasets?.map(dataset => { - if (dataset.id !== datasetId) { - return dataset - } - return { - ...dataset, - sublayerVisibility: { - ...dataset.sublayerVisibility, - [sublayerId]: visibility - } - } - }) - } -} const setLayerAdapterActions = (state, payload) => ({ ...state, layerAdapterActions: { ...state.layerAdapterActions, ...payload } }) const setDatasetStyle = (state, payload) => { @@ -213,7 +196,6 @@ const actions = { REMOVE_DATASET: removeDataset, SET_DATASET_VISIBILITY: setDatasetVisibility, SET_GLOBAL_VISIBILITY: setGlobalVisibility, - SET_SUBLAYER_VISIBILITY: setSublayerVisibility, SET_DATASET_STYLE: setDatasetStyle, SET_OPACITY: setOpacity, SET_GLOBAL_OPACITY: setGlobalOpacity, diff --git a/plugins/beta/datasets/src/registry/dataset.js b/plugins/beta/datasets/src/registry/dataset.js index e3fbe530..599945c3 100644 --- a/plugins/beta/datasets/src/registry/dataset.js +++ b/plugins/beta/datasets/src/registry/dataset.js @@ -23,6 +23,13 @@ export class Dataset { get maxZoom () { return this._datasetDefinition.maxZoom || this.parent?.maxZoom } get filter () { return this._datasetDefinition.filter || this.parent?.filter } + get visible () { + if (this.isSublayer) { + return this._datasetDefinition.visible && (this.parent?.visible) + } + return this._datasetDefinition.visible + } + get symbolAnchor () { if (this.style?.symbolAnchor) { return this.style.symbolAnchor @@ -48,9 +55,14 @@ export class Dataset { return this._datasetDefinition.sublayerIds?.length > 0 } + get sublayerIds () { + return this._datasetDefinition.sublayerIds + } + get sublayers () { - if (this._datasetDefinition.sublayerIds) { - return this._datasetDefinition.sublayerIds.map(id => datasetRegistry.getDataset(id)) + const { sublayerIds } = this + if (sublayerIds) { + return sublayerIds.map(id => datasetRegistry.getDataset(id)) } return undefined }