IM-286 dataset visibility simplified

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(() => {

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) {

plugins/beta/datasets/src/adapters/maplibre/maplibreLayerAdapter.test.js

@@ -2,7 +2,7 @@
 
 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('./layerBuilders.js', () => ({
-  addDatasetLayers: jest.fn(() => 'source-ds'),
-  addSublayerLayers: jest.fn()
+  addDatasetLayers: jest.fn(() => 'source-ds')
 }))
 
 jest.mock('./patternImages.js', () => ({
@@ -119,7 +118,7 @@
 
 // ─── init ─────────────────────────────────────────────────────────────────────
 
 describe.skip('init', () => {
   it('registers patterns and symbols before adding layers', async () => {
     const { adapter, mapProvider } = makeAdapter()
     await adapter.init([dataset], mapStyle)
@@ -151,7 +150,7 @@
 // ─── destroy ──────────────────────────────────────────────────────────────────
 
 describe('destroy', () => {
   it.skip('removes all layers for the dataset', async () => {
     const { adapter, map } = makeAdapter({ 'ds-fill': 'fill', 'ds-stroke': 'line' })
     map.getStyle.mockReturnValue({
       layers: [
@@ -165,7 +164,7 @@
     expect(map.removeLayer).toHaveBeenCalledWith('ds-stroke')
   })
 
   it.skip('removes the source after removing layers', async () => {
     const { adapter, map } = makeAdapter()
     getSourceId.mockReturnValue('source-ds')
     map.getStyle.mockReturnValue({ layers: [{ id: 'ds-fill', source: 'source-ds' }] })
@@ -174,7 +173,7 @@
     expect(map.removeSource).toHaveBeenCalledWith('source-ds')
   })
 
   it.skip('clears the datasetSourceMap', async () => {
     const { adapter } = makeAdapter()
     await adapter.init([dataset], mapStyle)
     adapter.destroy([dataset])
@@ -191,7 +190,7 @@
 
 // ─── addDataset ───────────────────────────────────────────────────────────────
 
 describe.skip('addDataset', () => {
   it('calls addDatasetLayers and stores the sourceId', () => {
     const { adapter } = makeAdapter()
     adapter.addDataset(dataset, mapStyle)
@@ -235,75 +234,6 @@
   })
 })
 
-// ─── 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', () => {
@@ -337,7 +267,7 @@
 
 // ─── setStyle ─────────────────────────────────────────────────────────────────
 
 describe.skip('setStyle', () => {
   it('removes existing layers before re-adding', async () => {
     const { adapter, map } = makeAdapter({ 'ds-fill': 'fill', 'ds-stroke': 'line' })
     await adapter.setStyle(dataset, mapStyle)
@@ -434,7 +364,7 @@
 
 // ─── onStyleChange ────────────────────────────────────────────────────────────
 
 describe.skip('onStyleChange', () => {
   it('waits for map idle before adding layers', async () => {
     const { adapter, map } = makeAdapter()
     await adapter.onStyleChange([dataset], mapStyle, {}, new Map())
@@ -462,7 +392,7 @@
     expect(reapply).toHaveBeenCalled()
   })
 
   it.skip('reapplies hidden feature filters', async () => {
     const { adapter } = makeAdapter({ 'ds-fill': 'fill' })
     const hiddenFeatures = { ds: { idProperty: 'id', ids: [1, 2] } }
     await adapter.onStyleChange([dataset], mapStyle, hiddenFeatures, new Map())
@@ -480,7 +410,7 @@
     expect(mapProvider.addPatternsToMap).toHaveBeenCalled()
   })
 
   it.skip('updates icon-image on symbol layers when imageId resolves', async () => {
     const { adapter, map } = makeAdapter({ 'ds-fill': 'symbol' })
     adapter._symbolLayerIds.add('ds-fill')
     getSymbolImageId.mockReturnValue('new-img')
@@ -488,7 +418,7 @@
     expect(map.setLayoutProperty).toHaveBeenCalledWith('ds-fill', 'icon-image', 'new-img')
   })
 
   it.skip('updates fill-pattern on pattern layers when imageId resolves', async () => {
     const { adapter, map } = makeAdapter({ 'ds-fill': 'fill' })
     hasPattern.mockReturnValue(true)
     getPatternImageId.mockReturnValue('pattern-img')
@@ -497,7 +427,7 @@
     expect(map.setPaintProperty).toHaveBeenCalledWith('ds-fill', 'fill-pattern', 'pattern-img')
   })
 
   it.skip('updates sublayer symbol layers on size change', async () => {
     const { adapter, map } = makeAdapter({ 'ds-sl-symbol': 'symbol' })
     const ds = { ...dataset, sublayers: [{ id: 'sl' }] }
     mergeSublayer.mockReturnValue({ symbol: 'marker' })
@@ -537,7 +467,7 @@
 
 // ─── onStyleChange: hiddenFeatures dataset not found (line 108) ───────────────
 
 describe.skip('onStyleChange — hiddenFeatures skips missing dataset', () => {
   it('does not apply filter when hiddenFeatures references a datasetId not in the list', async () => {
     const { adapter } = makeAdapter()
     const hiddenFeatures = { 'nonexistent-id': { idProperty: 'id', ids: [1] } }
@@ -571,7 +501,7 @@
 
 // ─── _maintainSymbolOrdering (lines 389, 398-400) ────────────────────────────
 
 describe.skip('_maintainSymbolOrdering', () => {
   it('adds a symbol-type layer id to _symbolLayerIds (line 389)', () => {
     const { adapter, map } = makeAdapter()
     getAllLayerIds.mockReturnValue(['ds-symbol'])

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 } })
 }

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 })
 }

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 || {}

plugins/beta/datasets/src/defaults.test.js

@@ -7,7 +7,7 @@ describe('datasetDefaults', () => {
       maxZoom: 24,
       showInKey: false,
       showInMenu: false,
-      visibility: 'visible'
+      visible: true
     })
   })
 

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,

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
   }