From dcf21ccac4de5140c08f36ddaedaee231b17daf4 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Fri, 1 May 2026 12:34:49 +0200 Subject: [PATCH 1/3] feat: add per-layer visibility toggle in plugin legend panel [DHIS2-20287] --- i18n/en.pot | 10 ++++++-- src/components/plugin/Legend.jsx | 9 +++++-- src/components/plugin/LegendLayer.jsx | 32 +++++++++++++++++++++---- src/components/plugin/Map.jsx | 27 ++++++++++++++++++--- src/components/plugin/styles/Legend.css | 23 ++++++++++++++++++ 5 files changed, 90 insertions(+), 11 deletions(-) diff --git a/i18n/en.pot b/i18n/en.pot index 3c604cc9a..3c8b20ac0 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -5,8 +5,8 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2026-04-28T15:36:36.629Z\n" -"PO-Revision-Date: 2026-04-28T15:36:36.629Z\n" +"POT-Creation-Date: 2026-05-01T10:18:47.999Z\n" +"PO-Revision-Date: 2026-05-01T10:18:48.000Z\n" msgid "2020" msgstr "2020" @@ -988,6 +988,12 @@ msgstr "Click to unpin legend" msgid "Click to pin legend" msgstr "Click to pin legend" +msgid "Hide layer" +msgstr "Hide layer" + +msgid "Show layer" +msgstr "Show layer" + msgid "No program" msgstr "No program" diff --git a/src/components/plugin/Legend.jsx b/src/components/plugin/Legend.jsx index 83c1dbb31..d1c57a8bf 100644 --- a/src/components/plugin/Legend.jsx +++ b/src/components/plugin/Legend.jsx @@ -6,7 +6,7 @@ import LegendLayer from './LegendLayer.jsx' import './styles/Legend.css' // Renders a legend for all map layers -const Legend = ({ layers }) => { +const Legend = ({ layers, toggleLayerVisibility }) => { const [isOpen, setIsOpen] = useState(false) const [isPinned, setIsPinned] = useState(false) @@ -30,7 +30,11 @@ const Legend = ({ layers }) => { onClick={() => setIsPinned(!isPinned)} > {legendLayers.map((layer) => ( - + ))} @@ -47,6 +51,7 @@ const Legend = ({ layers }) => { Legend.propTypes = { layers: PropTypes.array.isRequired, + toggleLayerVisibility: PropTypes.func, } export default Legend diff --git a/src/components/plugin/LegendLayer.jsx b/src/components/plugin/LegendLayer.jsx index 5753bdcbf..5b41f6017 100644 --- a/src/components/plugin/LegendLayer.jsx +++ b/src/components/plugin/LegendLayer.jsx @@ -1,3 +1,5 @@ +import i18n from '@dhis2/d2-i18n' +import { IconView24, IconViewOff24 } from '@dhis2/ui' import PropTypes from 'prop-types' import React, { Fragment } from 'react' import { getRenderingLabel } from '../../util/legend.js' @@ -11,16 +13,36 @@ const LegendLayer = ({ legend, renderingStrategy, alerts = DEFAULT_NO_ALERTS, + isVisible = true, + toggleLayerVisibility, }) => (
{legend && (

- {legend.title} - - {legend.period} - {getRenderingLabel(renderingStrategy)} + + {legend.title} + + {legend.period} + {getRenderingLabel(renderingStrategy)} + + {toggleLayerVisibility && ( + + )}

@@ -37,10 +59,12 @@ LegendLayer.propTypes = { id: PropTypes.string.isRequired, alerts: PropTypes.array, data: PropTypes.array, + isVisible: PropTypes.bool, layer: PropTypes.string, legend: PropTypes.object, renderingStrategy: PropTypes.string, serverCluster: PropTypes.bool, + toggleLayerVisibility: PropTypes.func, } export default LegendLayer diff --git a/src/components/plugin/Map.jsx b/src/components/plugin/Map.jsx index 4a899774b..06a2a3cd6 100644 --- a/src/components/plugin/Map.jsx +++ b/src/components/plugin/Map.jsx @@ -35,17 +35,28 @@ const Map = forwardRef((props, ref) => { useEffect(() => { if (didViewsChange(layers.current, mapViews)) { layers.current = mapViews.map((v) => ({ ...v, isLoaded: false })) - + setVisibilityOverrides({}) setMapIsLoaded(false) } }, [mapViews]) const [mapIsLoaded, setMapIsLoaded] = useState(mapViews.length === 0) const [contextMenu, setContextMenu] = useState() + const [visibilityOverrides, setVisibilityOverrides] = useState({}) const [resizeCount, setResizeCount] = useState(0) const onResize = () => setResizeCount((state) => state + 1) + const toggleLayerVisibility = useCallback((id) => { + setVisibilityOverrides((prev) => { + const current = + prev[id] ?? + layers.current.find((l) => l.id === id)?.isVisible ?? + true + return { ...prev, [id]: !current } + }) + }, []) + const onLayerLoad = useCallback((layer) => { layers.current = layers.current.map((l) => layer.id === l.id ? layer : l @@ -126,6 +137,11 @@ const Map = forwardRef((props, ref) => { ) } + const layersWithVisibility = layers.current.map((l) => ({ + ...l, + isVisible: visibilityOverrides[l.id] ?? l.isVisible ?? true, + })) + return (
@@ -134,13 +150,18 @@ const Map = forwardRef((props, ref) => { isPlugin={true} isFullscreen={false} basemap={basemap} - layers={layers.current} + layers={layersWithVisibility} controls={controls} bounds={defaultBounds} openContextMenu={setContextMenu} resizeCount={resizeCount} /> - {mapViews.length > 0 && } + {mapViews.length > 0 && ( + + )} {contextMenu && ( Date: Fri, 1 May 2026 19:02:29 +0200 Subject: [PATCH 2/3] feat: persist layer visibility and basemap opacity on map save [DHIS2-19078] --- src/components/app/FileMenu.jsx | 6 +- src/components/map/layers/EventLayer.jsx | 1 + src/components/map/layers/ExternalLayer.js | 1 + src/components/map/layers/FacilityLayer.jsx | 1 + src/components/map/layers/GeoJsonLayer.js | 1 + src/components/map/layers/Layer.js | 1 + src/components/map/layers/OrgUnitLayer.jsx | 1 + src/components/map/layers/ThematicLayer.jsx | 1 + .../map/layers/TrackedEntityLayer.jsx | 1 + .../layers/earthEngine/EarthEngineLayer.jsx | 1 + src/loaders/earthEngineLoader.js | 1 - src/loaders/eventLoader.js | 1 - src/loaders/externalLoader.js | 3 +- src/loaders/facilityLoader.js | 1 - src/loaders/geoJsonUrlLoader.js | 1 - src/loaders/orgUnitLoader.js | 1 - src/loaders/thematicLoader.js | 2 - src/loaders/trackedEntityLoader.js | 1 - src/reducers/map.js | 1 + src/util/__tests__/external.spec.js | 8 -- src/util/__tests__/favorites.spec.js | 19 ++-- .../__tests__/getMigratedMapConfig.spec.js | 89 ++++++++++++++----- src/util/external.js | 2 - src/util/favorites.js | 40 ++++++--- src/util/getMigratedMapConfig.js | 74 ++++++++------- src/util/helpers.js | 1 + 26 files changed, 171 insertions(+), 89 deletions(-) diff --git a/src/components/app/FileMenu.jsx b/src/components/app/FileMenu.jsx index 5f3bc9f26..cfd1b6c18 100644 --- a/src/components/app/FileMenu.jsx +++ b/src/components/app/FileMenu.jsx @@ -4,7 +4,7 @@ import { preparePayloadForSaveAs, VIS_TYPE_MAP, } from '@dhis2/analytics' -import { useDataMutation, useDataEngine } from '@dhis2/app-runtime' +import { useDataMutation, useDataEngine, useConfig } from '@dhis2/app-runtime' import { useAlert } from '@dhis2/app-service-alerts' import i18n from '@dhis2/d2-i18n' import PropTypes from 'prop-types' @@ -67,6 +67,7 @@ const FileMenu = ({ onFileMenuAction }) => { const map = useSelector((state) => state.map) const dispatch = useDispatch() const engine = useDataEngine() + const { serverVersion } = useConfig() const { systemSettings, currentUser } = useCachedData() const defaultBasemap = systemSettings.keyDefaultBaseMap //alerts @@ -119,6 +120,7 @@ const FileMenu = ({ onFileMenuAction }) => { const cleanedMap = cleanMapConfig({ config: map, defaultBasemapId: defaultBasemap, + serverVersion, }) const config = preparePayloadForSave({ @@ -160,6 +162,7 @@ const FileMenu = ({ onFileMenuAction }) => { config: latestMap, defaultBasemapId: defaultBasemap, cleanMapviewConfig: false, + serverVersion, }) const config = preparePayloadForSave({ @@ -189,6 +192,7 @@ const FileMenu = ({ onFileMenuAction }) => { const cleanedMap = cleanMapConfig({ config: map, defaultBasemapId: defaultBasemap, + serverVersion, }) const config = preparePayloadForSaveAs({ diff --git a/src/components/map/layers/EventLayer.jsx b/src/components/map/layers/EventLayer.jsx index fd9103b82..36d2cd5d0 100644 --- a/src/components/map/layers/EventLayer.jsx +++ b/src/components/map/layers/EventLayer.jsx @@ -146,6 +146,7 @@ class EventLayer extends Layer { this.layer = map.createLayer(config) map.addLayer(this.layer) + this.setLayerVisibility() // Fit map to layer bounds once (when first created) this.fitBoundsOnce() diff --git a/src/components/map/layers/ExternalLayer.js b/src/components/map/layers/ExternalLayer.js index cc7ea39e1..839e70fb5 100644 --- a/src/components/map/layers/ExternalLayer.js +++ b/src/components/map/layers/ExternalLayer.js @@ -14,5 +14,6 @@ export default class ExternalLayer extends Layer { }) map.addLayer(this.layer) + this.setLayerVisibility() } } diff --git a/src/components/map/layers/FacilityLayer.jsx b/src/components/map/layers/FacilityLayer.jsx index 749272621..3ff48172b 100644 --- a/src/components/map/layers/FacilityLayer.jsx +++ b/src/components/map/layers/FacilityLayer.jsx @@ -97,6 +97,7 @@ class FacilityLayer extends Layer { group.addLayer(config) this.layer = group map.addLayer(this.layer).catch(this.onError.bind(this)) + this.setLayerVisibility() // Fit map to layer bounds once (when first created) this.fitBoundsOnce() diff --git a/src/components/map/layers/GeoJsonLayer.js b/src/components/map/layers/GeoJsonLayer.js index 321248e1a..b837f48a0 100644 --- a/src/components/map/layers/GeoJsonLayer.js +++ b/src/components/map/layers/GeoJsonLayer.js @@ -48,6 +48,7 @@ class GeoJsonLayer extends Layer { }) map.addLayer(this.layer) + this.setLayerVisibility() // Fit map to layer bounds once (when first created) this.fitBoundsOnce() diff --git a/src/components/map/layers/Layer.js b/src/components/map/layers/Layer.js index 0b2998bef..c8bff1363 100644 --- a/src/components/map/layers/Layer.js +++ b/src/components/map/layers/Layer.js @@ -106,6 +106,7 @@ class Layer extends PureComponent { }) await map.addLayer(this.layer) + this.setLayerVisibility() } async updateLayer() { diff --git a/src/components/map/layers/OrgUnitLayer.jsx b/src/components/map/layers/OrgUnitLayer.jsx index 76fa7ee71..5e1247876 100644 --- a/src/components/map/layers/OrgUnitLayer.jsx +++ b/src/components/map/layers/OrgUnitLayer.jsx @@ -60,6 +60,7 @@ export default class OrgUnitLayer extends Layer { this.layer = map.createLayer(config) map.addLayer(this.layer) + this.setLayerVisibility() // Fit map to layer bounds once (when first created) this.fitBoundsOnce() diff --git a/src/components/map/layers/ThematicLayer.jsx b/src/components/map/layers/ThematicLayer.jsx index 6518ad2da..adf4bfbfe 100644 --- a/src/components/map/layers/ThematicLayer.jsx +++ b/src/components/map/layers/ThematicLayer.jsx @@ -105,6 +105,7 @@ class ThematicLayer extends Layer { } map.addLayer(this.layer) + this.setLayerVisibility() const options = {} if (renderingStrategy === RENDERING_STRATEGY_TIMELINE) { diff --git a/src/components/map/layers/TrackedEntityLayer.jsx b/src/components/map/layers/TrackedEntityLayer.jsx index 966f8b4ac..96f3f0841 100644 --- a/src/components/map/layers/TrackedEntityLayer.jsx +++ b/src/components/map/layers/TrackedEntityLayer.jsx @@ -134,6 +134,7 @@ class TrackedEntityLayer extends Layer { this.layer = group map.addLayer(this.layer) + this.setLayerVisibility() // Fit map to layer bounds once (when first created) this.fitBoundsOnce() diff --git a/src/components/map/layers/earthEngine/EarthEngineLayer.jsx b/src/components/map/layers/earthEngine/EarthEngineLayer.jsx index 02b47d492..8a97a6f23 100644 --- a/src/components/map/layers/earthEngine/EarthEngineLayer.jsx +++ b/src/components/map/layers/earthEngine/EarthEngineLayer.jsx @@ -143,6 +143,7 @@ export default class EarthEngineLayer extends Layer { try { this.layer = map.createLayer(config) await map.addLayer(this.layer) + this.setLayerVisibility() } catch (error) { this.onError(error) } diff --git a/src/loaders/earthEngineLoader.js b/src/loaders/earthEngineLoader.js index b767ec8c9..32690d5dc 100644 --- a/src/loaders/earthEngineLoader.js +++ b/src/loaders/earthEngineLoader.js @@ -222,7 +222,6 @@ const earthEngineLoader = async ({ isLoaded: true, isLoading: false, isExpanded: true, - isVisible: true, loadError, } } diff --git a/src/loaders/eventLoader.js b/src/loaders/eventLoader.js index d49d5ce3d..81c75b2ab 100644 --- a/src/loaders/eventLoader.js +++ b/src/loaders/eventLoader.js @@ -89,7 +89,6 @@ const eventLoader = async ({ config.isLoaded = true config.isLoading = false config.isExpanded = true - config.isVisible = true return config } diff --git a/src/loaders/externalLoader.js b/src/loaders/externalLoader.js index 678fbb83e..0e031c9dd 100644 --- a/src/loaders/externalLoader.js +++ b/src/loaders/externalLoader.js @@ -31,13 +31,12 @@ const externalLoader = async ({ config: layer, engine }) => { return { ...layer, layer: EXTERNAL_LAYER, - name: config.name, + name: config.name, // Overrides layer.name from spread — redundant on 2.42+ (DHIS2-16088), remove when 2.41 support is dropped legend, config, isLoaded: true, isLoading: false, isExpanded: true, - isVisible: true, } } diff --git a/src/loaders/facilityLoader.js b/src/loaders/facilityLoader.js index ec2eeff76..93b6e3a57 100644 --- a/src/loaders/facilityLoader.js +++ b/src/loaders/facilityLoader.js @@ -154,7 +154,6 @@ const facilityLoader = async ({ isLoaded: true, isLoading: false, isExpanded: true, - isVisible: true, loadError, } } diff --git a/src/loaders/geoJsonUrlLoader.js b/src/loaders/geoJsonUrlLoader.js index 43e832316..55dffe115 100644 --- a/src/loaders/geoJsonUrlLoader.js +++ b/src/loaders/geoJsonUrlLoader.js @@ -128,7 +128,6 @@ const geoJsonUrlLoader = async ({ isLoaded: true, isLoading: false, isExpanded: true, - isVisible: true, loadError, } } diff --git a/src/loaders/orgUnitLoader.js b/src/loaders/orgUnitLoader.js index 0c0c8b34f..982eedfad 100644 --- a/src/loaders/orgUnitLoader.js +++ b/src/loaders/orgUnitLoader.js @@ -140,7 +140,6 @@ const orgUnitLoader = async ({ isLoaded: true, isLoading: false, isExpanded: true, - isVisible: true, loadError, } } diff --git a/src/loaders/thematicLoader.js b/src/loaders/thematicLoader.js index 9a1b5c1b5..1751fa31e 100644 --- a/src/loaders/thematicLoader.js +++ b/src/loaders/thematicLoader.js @@ -163,7 +163,6 @@ const thematicLoader = async ({ legend: null, isLoaded: true, isLoading: false, - isVisible: true, loadError, } } @@ -503,7 +502,6 @@ const thematicLoader = async ({ isLoaded: true, isLoading: false, isExpanded: true, - isVisible: true, loadError, } } diff --git a/src/loaders/trackedEntityLoader.js b/src/loaders/trackedEntityLoader.js index 706b52fe5..17e907eae 100644 --- a/src/loaders/trackedEntityLoader.js +++ b/src/loaders/trackedEntityLoader.js @@ -357,7 +357,6 @@ const trackedEntityLoader = async ({ isLoaded: true, isLoading: false, isExpanded: true, - isVisible: true, } } diff --git a/src/reducers/map.js b/src/reducers/map.js index 0f3b835e9..832b94eaf 100644 --- a/src/reducers/map.js +++ b/src/reducers/map.js @@ -242,6 +242,7 @@ const map = (state = defaultState, action) => { { ...action.payload, id: generateUid(), + isVisible: action.payload.isVisible ?? true, }, ], } diff --git a/src/util/__tests__/external.spec.js b/src/util/__tests__/external.spec.js index 5f7c37f64..3089e956e 100644 --- a/src/util/__tests__/external.spec.js +++ b/src/util/__tests__/external.spec.js @@ -29,7 +29,6 @@ describe('createExternalBasemapLayer', () => { layer: EXTERNAL_LAYER, id, name, - opacity: 1, config: { id, type: VECTOR_STYLE, @@ -58,7 +57,6 @@ describe('createExternalBasemapLayer', () => { layer: EXTERNAL_LAYER, id, name, - opacity: 1, config: { id, type: TILE_LAYER, @@ -87,7 +85,6 @@ describe('createExternalBasemapLayer', () => { layer: EXTERNAL_LAYER, id, name, - opacity: 1, config: { id, type: TILE_LAYER, @@ -123,7 +120,6 @@ describe('createExternalBasemapLayer', () => { layer: EXTERNAL_LAYER, id, name, - opacity: 1, config: { id, type: WMS_LAYER, @@ -154,7 +150,6 @@ describe('createExternalOverlayLayer', () => { expect(createExternalOverlayLayer(model)).toMatchObject({ layer: EXTERNAL_LAYER, name, - opacity: 1, img: 'images/featurelayer.png', config: { id, @@ -183,7 +178,6 @@ describe('createExternalOverlayLayer', () => { expect(createExternalOverlayLayer(model)).toMatchObject({ layer: EXTERNAL_LAYER, name, - opacity: 1, img: 'images/featurelayer.png', config: { id, @@ -219,7 +213,6 @@ describe('createExternalOverlayLayer', () => { expect(createExternalOverlayLayer(model)).toMatchObject({ layer: EXTERNAL_LAYER, name, - opacity: 1, img: 'images/featurelayer.png', config: { id, @@ -249,7 +242,6 @@ describe('createExternalOverlayLayer', () => { expect(createExternalOverlayLayer(model)).toMatchObject({ layer: GEOJSON_URL_LAYER, name, - opacity: 1, img: 'images/featurelayer.png', config: { id, diff --git a/src/util/__tests__/favorites.spec.js b/src/util/__tests__/favorites.spec.js index 902544404..f3461d70d 100644 --- a/src/util/__tests__/favorites.spec.js +++ b/src/util/__tests__/favorites.spec.js @@ -16,8 +16,9 @@ describe('cleanMapConfig', () => { }) expect(cleanedConfig).toEqual( expect.objectContaining({ - basemap: 'thedefaultBasemap', - mapViews: [{ layer: 'layer1' }], + basemap: + '{"id":"thedefaultBasemap","opacity":0.9,"hidden":false}', + mapViews: [{ hidden: false, layer: 'layer1' }], name: 'my new map', }) ) @@ -48,8 +49,9 @@ describe('cleanMapConfig', () => { }) expect(cleanedConfig).toEqual( expect.objectContaining({ - basemap: 'myUniqueBasemap', - mapViews: [{ layer: 'layer1' }], + basemap: + '{"id":"myUniqueBasemap","opacity":0.9,"hidden":false}', + mapViews: [{ hidden: false, layer: 'layer1' }], name: 'my new map', }) ) @@ -176,10 +178,11 @@ describe('cleanMapConfig', () => { }) expect(cleanedConfig).toEqual({ - basemap: 'thedefaultBasemap', + basemap: '{"id":"thedefaultBasemap","opacity":1,"hidden":false}', mapViews: [ { areaRadius: 5000, + hidden: false, layer: 'earthEngine', name: 'Population', opacity: 0.9, @@ -274,10 +277,11 @@ describe('cleanMapConfig', () => { }) expect(cleanedConfig).toEqual({ - basemap: 'thedefaultBasemap', + basemap: '{"id":"thedefaultBasemap","opacity":1,"hidden":false}', mapViews: [ { config: '{"id":"CSYRWeK81E7","type":"geoJson","url":"https://debug.dhis2.org/analytics-dev/api/routes/aaa11122233/run","name":"Bo catchment areas","tms":false,"format":"image/png","featureStyle":{"color":"transparent","strokeColor":"#333333","weight":1,"pointSize":5}}', + hidden: false, layer: 'geoJsonUrl', name: 'Bo catchment areas', opacity: 1, @@ -593,11 +597,12 @@ describe('cleanMapConfig', () => { }) expect(cleanedConfig).toEqual({ - basemap: 'thedefaultBasemap', + basemap: '{"id":"thedefaultBasemap","opacity":1,"hidden":false}', mapViews: [ { startDate: '2018-02-19', endDate: '2024-02-19', + hidden: false, layer: 'trackedEntity', name: 'Tracked entity', opacity: 0.5, diff --git a/src/util/__tests__/getMigratedMapConfig.spec.js b/src/util/__tests__/getMigratedMapConfig.spec.js index dbd1c465f..2363c3db1 100644 --- a/src/util/__tests__/getMigratedMapConfig.spec.js +++ b/src/util/__tests__/getMigratedMapConfig.spec.js @@ -30,6 +30,8 @@ test('getMigratedMapConfig when basemap in mapViews', () => { id: 'Basemap id', mapLayerPosition: 'BASEMAP', name: 'Basemap name', + opacity: 1, + isVisible: true, }, mapViews: [ { @@ -39,6 +41,7 @@ test('getMigratedMapConfig when basemap in mapViews', () => { config: { mapLayerPosition: 'OVERLAY', }, + isVisible: true, }, ], }) @@ -60,10 +63,18 @@ test('getMigratedMapConfig when basemap is a string but not "none"', () => { expect.objectContaining({ id: 'mapId', name: 'map name', - basemap: { id: 'TheRainbowBasemap' }, + basemap: { id: 'TheRainbowBasemap', opacity: 1, isVisible: true }, mapViews: [ - { layer: 'thematic', name: 'All the pretty colors' }, - { layer: 'facilities', name: 'All the facilities' }, + { + layer: 'thematic', + name: 'All the pretty colors', + isVisible: true, + }, + { + layer: 'facilities', + name: 'All the facilities', + isVisible: true, + }, ], }) ) @@ -84,13 +95,18 @@ test('getMigratedMapConfig when basemap is string "none"', () => { expect.objectContaining({ id: 'mapId', name: 'map name', - basemap: { - id: defaultBasemapId, - isVisible: false, - }, + basemap: { id: defaultBasemapId, opacity: 1, isVisible: false }, mapViews: [ - { layer: 'thematic', name: 'All the pretty colors' }, - { layer: 'facilities', name: 'All the facilities' }, + { + layer: 'thematic', + name: 'All the pretty colors', + isVisible: true, + }, + { + layer: 'facilities', + name: 'All the facilities', + isVisible: true, + }, ], }) ) @@ -114,10 +130,23 @@ test('getMigratedMapConfig when basemap is an object', () => { expect.objectContaining({ id: 'mapId', name: 'map name', - basemap: { id: 'osmStreet', displayName: 'Basemap name' }, + basemap: { + id: 'osmStreet', + displayName: 'Basemap name', + opacity: 1, + isVisible: true, + }, mapViews: [ - { layer: 'thematic', name: 'All the pretty colors' }, - { layer: 'facilities', name: 'All the facilities' }, + { + layer: 'thematic', + name: 'All the pretty colors', + isVisible: true, + }, + { + layer: 'facilities', + name: 'All the facilities', + isVisible: true, + }, ], }) ) @@ -137,10 +166,18 @@ test('getMigratedMapConfig when no basemap in config', () => { expect.objectContaining({ id: 'mapId', name: 'map name', - basemap: { id: defaultBasemapId }, + basemap: { id: defaultBasemapId, opacity: 1, isVisible: true }, mapViews: [ - { layer: 'thematic', name: 'All the pretty colors' }, - { layer: 'facilities', name: 'All the facilities' }, + { + layer: 'thematic', + name: 'All the pretty colors', + isVisible: true, + }, + { + layer: 'facilities', + name: 'All the facilities', + isVisible: true, + }, ], }) ) @@ -163,11 +200,19 @@ test('getMigratedMapConfig with old GIS app format and Boundary layer', () => { expect.objectContaining({ id: 'mapId', name: 'map name', - basemap: { id: 'osmStreet' }, + basemap: { id: 'osmStreet', opacity: 1, isVisible: true }, mapViews: [ - { layer: 'thematic', name: 'Thematic layer 2' }, - { layer: 'thematic', name: 'Thematic layer 1' }, - { layer: 'orgUnit', name: 'Boundary layer' }, + { + layer: 'thematic', + name: 'Thematic layer 2', + isVisible: true, + }, + { + layer: 'thematic', + name: 'Thematic layer 1', + isVisible: true, + }, + { layer: 'orgUnit', name: 'Boundary layer', isVisible: true }, ], }) ) @@ -191,7 +236,7 @@ test('getMigratedMapConfig with colorScale with multiple values converted to an expect.objectContaining({ id: 'mapId', name: 'map name', - basemap: { id: 'osmStreet' }, + basemap: { id: 'osmStreet', opacity: 1, isVisible: true }, mapViews: [ { layer: 'thematic', @@ -204,6 +249,7 @@ test('getMigratedMapConfig with colorScale with multiple values converted to an '#de2d26', '#a50f15', ], + isVisible: true, }, ], }) @@ -228,12 +274,13 @@ test('getMigratedMapConfig with colorScale with single value returns value', () expect.objectContaining({ id: 'mapId', name: 'map name', - basemap: { id: 'osmStreet' }, + basemap: { id: 'osmStreet', opacity: 1, isVisible: true }, mapViews: [ { layer: 'thematic', name: 'Thematic layer', colorScale: '#fee5d9', + isVisible: true, }, ], }) diff --git a/src/util/external.js b/src/util/external.js index 8d4e96746..60628ac55 100644 --- a/src/util/external.js +++ b/src/util/external.js @@ -28,7 +28,6 @@ export const createExternalBasemapLayer = (layer) => ({ layer: EXTERNAL_LAYER, id: layer.id, name: layer.name, - opacity: 1, config: createExternalLayerConfig(layer), }) @@ -39,7 +38,6 @@ export const createExternalOverlayLayer = (layer) => ({ : EXTERNAL_LAYER, img: 'images/featurelayer.png', name: layer.name, - opacity: 1, config: createExternalLayerConfig(layer), }) diff --git a/src/util/favorites.js b/src/util/favorites.js index 0966ff9a9..898408357 100644 --- a/src/util/favorites.js +++ b/src/util/favorites.js @@ -9,7 +9,7 @@ import { // TODO: get latitude, longitude, zoom from map + basemap: 'none' const validMapProperties = [ - 'basemap', + // 'basemap' and 'basemaps' removed — set exclusively by getBasemapPayload 'id', 'latitude', 'longitude', @@ -65,6 +65,7 @@ const validLayerProperties = [ 'noDataColor', 'noDataLegend', 'unclassifiedLegend', + 'hidden', 'opacity', 'organisationUnitColor', 'organisationUnitGroupSet', @@ -108,28 +109,47 @@ export const cleanMapConfig = ({ config, defaultBasemapId, cleanMapviewConfig = true, + serverVersion, }) => ({ ...omitBy(isNil, pick(validMapProperties, config)), - basemap: getBasemapString(config.basemap, defaultBasemapId), + ...getBasemapPayload(config.basemap, defaultBasemapId, serverVersion), mapViews: config.mapViews.map((view) => cleanLayerConfig(view, cleanMapviewConfig) ), }) -const getBasemapString = (basemap, defaultBasemapId) => { - if (!basemap) { - return defaultBasemapId +// VERSION-TOGGLE: https://dhis2.atlassian.net/browse/DHIS2-20417 +const getBasemapPayload = (basemap, defaultBasemapId, serverVersion) => { + if (serverVersion?.minor >= 43) { + return { + basemaps: [ + { + id: basemap?.id ?? defaultBasemapId, + opacity: basemap?.opacity ?? 1, + hidden: basemap?.isVisible === false, + }, + ], + } } - if (basemap.isVisible === false) { - return 'none' + // Legacy: store as JSON to preserve opacity and id when hidden + return { + basemap: JSON.stringify({ + id: basemap?.id ?? defaultBasemapId, + opacity: basemap?.opacity ?? 1, + hidden: basemap?.isVisible === false, + }), } - - return basemap.id || defaultBasemapId } const cleanLayerConfig = (layer, cleanMapviewConfig) => ({ - ...models2objects(pick(validLayerProperties, layer), cleanMapviewConfig), + ...models2objects( + pick(validLayerProperties, { + ...layer, + hidden: layer.isVisible === false, + }), + cleanMapviewConfig + ), }) // TODO: This feels hacky, find better way to clean map configs before saving diff --git a/src/util/getMigratedMapConfig.js b/src/util/getMigratedMapConfig.js index 6d213e7a7..65417a87a 100644 --- a/src/util/getMigratedMapConfig.js +++ b/src/util/getMigratedMapConfig.js @@ -6,7 +6,7 @@ export const getMigratedMapConfig = (config, defaultBasemapId) => upgradeGisAppLayers(extractBasemap(config, defaultBasemapId)) ) -// Different ways of specifying a basemap - TODO: simplify! +// Different ways of specifying a basemap const extractBasemap = (config, defaultBasemapId) => { const externalBasemap = config.mapViews.find( (view) => @@ -21,21 +21,37 @@ const extractBasemap = (config, defaultBasemapId) => { mapViews = config.mapViews.filter( (view) => view.id !== externalBasemap.id ) + } else if (Array.isArray(config.basemaps) && config.basemaps.length > 0) { + // VERSION-TOGGLE: https://dhis2.atlassian.net/browse/DHIS2-20417 + const { id, opacity, hidden } = config.basemaps[0] + basemap = { id, opacity, isVisible: !hidden } } else if (isString(config.basemap)) { - basemap = - config.basemap === 'none' - ? { id: defaultBasemapId, isVisible: false } - : { id: config.basemap } + if (config.basemap === 'none') { + basemap = { isVisible: false } + } else { + try { + const { id, opacity, hidden } = JSON.parse(config.basemap) + basemap = { id, opacity, isVisible: !hidden } + } catch { + // Plain basemap ID saved before JSON encoding + basemap = { id: config.basemap } + } + } } else if (isObject(config.basemap)) { basemap = config.basemap } else { - basemap = { id: defaultBasemapId } + basemap = {} } return { ...config, - basemap: basemap, - mapViews: mapViews, + basemap: { + ...basemap, + id: basemap.id ?? defaultBasemapId, + opacity: basemap.opacity ?? 1, + isVisible: basemap.isVisible ?? true, + }, + mapViews, } } @@ -79,43 +95,41 @@ const upgradeGisAppLayers = (config) => { } const upgradeMapViews = (config) => { - const needsUpgrade = config.mapViews.some( + const needsLegacyUpgrade = config.mapViews.some( (view) => view.layer === 'boundary' || typeof view.colorScale === 'string' || view.geometryCentroid === undefined ) - if (!needsUpgrade) { - return config - } - const upgradedViews = config.mapViews.map((view) => { let layer = view.layer - if (layer === 'boundary') { - layer = 'orgUnit' - } - - if ( - view.geometryCentroid === undefined && - view.layer === 'event' && - !EVENT_CENTROID_DEFAULT.includes(view.eventCoordinateField) - ) { - // We should test !EVENT_CENTROID_DEFAULT.includes(view.eventCoordinateFieldType) but it is not currently saved with the mapView. - // This will set geometryCentroid: true when eventCoordinateField is a DE/TEA of type 'COORDINATE' too, which is unnecessary but harmless. - view.geometryCentroid = true - } - let colorScale = view.colorScale - if (typeof colorScale === 'string') { - const parts = colorScale.split(',') - colorScale = parts.length === 1 ? parts[0] : parts + + if (needsLegacyUpgrade) { + if (layer === 'boundary') { + layer = 'orgUnit' + } + if ( + view.geometryCentroid === undefined && + view.layer === 'event' && + !EVENT_CENTROID_DEFAULT.includes(view.eventCoordinateField) + ) { + // We should test !EVENT_CENTROID_DEFAULT.includes(view.eventCoordinateFieldType) but it is not currently saved with the mapView. + // This will set geometryCentroid: true when eventCoordinateField is a DE/TEA of type 'COORDINATE' too, which is unnecessary but harmless. + view.geometryCentroid = true + } + if (typeof colorScale === 'string') { + const parts = colorScale.split(',') + colorScale = parts.length === 1 ? parts[0] : parts + } } return { ...view, layer, colorScale, + isVisible: view.hidden !== true, } }) diff --git a/src/util/helpers.js b/src/util/helpers.js index abde96e95..402535dd2 100644 --- a/src/util/helpers.js +++ b/src/util/helpers.js @@ -23,6 +23,7 @@ const getBaseFields = (withSubscribers) => { 'latitude', 'zoom', 'basemap', + 'basemaps', 'created', 'lastUpdated', 'access', From 92a3e8fab6ebc56ae68500a976b07332010fdfb9 Mon Sep 17 00:00:00 2001 From: Bruno Raimbault Date: Fri, 1 May 2026 19:23:41 +0200 Subject: [PATCH 3/3] chore: add tests --- .../plugin/__tests__/LegendLayer.spec.jsx | 103 +++++++++++++++++ src/util/__tests__/favorites.spec.js | 104 ++++++++++++++++++ .../__tests__/getMigratedMapConfig.spec.js | 80 ++++++++++++++ 3 files changed, 287 insertions(+) create mode 100644 src/components/plugin/__tests__/LegendLayer.spec.jsx diff --git a/src/components/plugin/__tests__/LegendLayer.spec.jsx b/src/components/plugin/__tests__/LegendLayer.spec.jsx new file mode 100644 index 000000000..8bff20100 --- /dev/null +++ b/src/components/plugin/__tests__/LegendLayer.spec.jsx @@ -0,0 +1,103 @@ +import { render, screen, fireEvent } from '@testing-library/react' +import React from 'react' +import LegendLayer from '../LegendLayer.jsx' + +jest.mock('@dhis2/d2-i18n', () => ({ t: (s) => s })) +jest.mock('@dhis2/ui', () => ({ + IconView24: () => eye-open, + IconViewOff24: () => eye-off, +})) +jest.mock('../../legend/Legend.jsx', () => () => null) +jest.mock('../../../util/legend.js', () => ({ + getRenderingLabel: () => '', +})) + +const legend = { title: 'My Layer', period: '2023' } + +describe('LegendLayer', () => { + test('renders legend title and period', () => { + render() + expect(screen.getByText('My Layer')).toBeInTheDocument() + expect(screen.getByText('2023')).toBeInTheDocument() + }) + + test('renders nothing when legend is not provided', () => { + render() + expect(screen.queryByText('My Layer')).not.toBeInTheDocument() + expect(screen.queryByRole('button')).not.toBeInTheDocument() + }) + + test('shows eye-open button when layer is visible', () => { + render( + + ) + expect(screen.getByTitle('Hide layer')).toBeInTheDocument() + expect(screen.getByText('eye-open')).toBeInTheDocument() + }) + + test('shows eye-off button when layer is hidden', () => { + render( + + ) + expect(screen.getByTitle('Show layer')).toBeInTheDocument() + expect(screen.getByText('eye-off')).toBeInTheDocument() + }) + + test('defaults to visible (eye-open) when isVisible is not provided', () => { + render( + + ) + expect(screen.getByTitle('Hide layer')).toBeInTheDocument() + }) + + test('does not render visibility button when toggleLayerVisibility is not provided', () => { + render() + expect(screen.queryByRole('button')).not.toBeInTheDocument() + }) + + test('calls toggleLayerVisibility with the layer id on button click', () => { + const toggle = jest.fn() + render( + + ) + fireEvent.click(screen.getByRole('button')) + expect(toggle).toHaveBeenCalledWith('layer-1') + expect(toggle).toHaveBeenCalledTimes(1) + }) + + test('button click does not propagate to parent', () => { + const toggle = jest.fn() + const parentClick = jest.fn() + render( + + ) + fireEvent.click(screen.getByTitle('Hide layer')) + expect(parentClick).not.toHaveBeenCalled() + }) +}) diff --git a/src/util/__tests__/favorites.spec.js b/src/util/__tests__/favorites.spec.js index f3461d70d..2445b4579 100644 --- a/src/util/__tests__/favorites.spec.js +++ b/src/util/__tests__/favorites.spec.js @@ -501,6 +501,110 @@ describe('cleanMapConfig', () => { ) }) + test('writes hidden: true for a layer with isVisible: false', () => { + const cleanedConfig = cleanMapConfig({ + config: { + mapViews: [ + { + layer: 'thematic', + name: 'Hidden layer', + opacity: 1, + isVisible: false, + }, + ], + }, + defaultBasemapId: 'thedefaultBasemap', + }) + expect(cleanedConfig.mapViews[0].hidden).toBe(true) + }) + + test('writes hidden: false for a layer with isVisible: true', () => { + const cleanedConfig = cleanMapConfig({ + config: { + mapViews: [ + { + layer: 'thematic', + name: 'Visible layer', + opacity: 1, + isVisible: true, + }, + ], + }, + defaultBasemapId: 'thedefaultBasemap', + }) + expect(cleanedConfig.mapViews[0].hidden).toBe(false) + }) + + test('writes hidden: false for a layer with no explicit isVisible', () => { + const cleanedConfig = cleanMapConfig({ + config: { + mapViews: [ + { + layer: 'thematic', + name: 'No-visibility layer', + opacity: 1, + }, + ], + }, + defaultBasemapId: 'thedefaultBasemap', + }) + expect(cleanedConfig.mapViews[0].hidden).toBe(false) + }) + + test('writes hidden: true in legacy basemap JSON when basemap isVisible is false', () => { + const cleanedConfig = cleanMapConfig({ + config: { + basemap: { id: 'osmLight', opacity: 1, isVisible: false }, + mapViews: [], + }, + defaultBasemapId: 'thedefaultBasemap', + }) + const parsed = JSON.parse(cleanedConfig.basemap) + expect(parsed.hidden).toBe(true) + expect(parsed.id).toBe('osmLight') + }) + + test('writes opacity in legacy basemap JSON', () => { + const cleanedConfig = cleanMapConfig({ + config: { + basemap: { id: 'osmLight', opacity: 0.4, isVisible: true }, + mapViews: [], + }, + defaultBasemapId: 'thedefaultBasemap', + }) + const parsed = JSON.parse(cleanedConfig.basemap) + expect(parsed.opacity).toBe(0.4) + expect(parsed.hidden).toBe(false) + }) + + test('uses basemaps array for v43+', () => { + const cleanedConfig = cleanMapConfig({ + config: { + basemap: { id: 'osmLight', opacity: 0.5, isVisible: true }, + mapViews: [], + }, + defaultBasemapId: 'thedefaultBasemap', + serverVersion: { minor: 43 }, + }) + expect(cleanedConfig.basemaps).toEqual([ + { id: 'osmLight', opacity: 0.5, hidden: false }, + ]) + expect(cleanedConfig).not.toHaveProperty('basemap') + }) + + test('writes hidden: true in basemaps array for v43+ when basemap isVisible is false', () => { + const cleanedConfig = cleanMapConfig({ + config: { + basemap: { id: 'osmLight', opacity: 1, isVisible: false }, + mapViews: [], + }, + defaultBasemapId: 'thedefaultBasemap', + serverVersion: { minor: 43 }, + }) + expect(cleanedConfig.basemaps[0].hidden).toBe(true) + expect(cleanedConfig.basemaps[0].id).toBe('osmLight') + }) + test('correctly converts TEI mapview', () => { const config = { bounds: [ diff --git a/src/util/__tests__/getMigratedMapConfig.spec.js b/src/util/__tests__/getMigratedMapConfig.spec.js index 2363c3db1..451d0798e 100644 --- a/src/util/__tests__/getMigratedMapConfig.spec.js +++ b/src/util/__tests__/getMigratedMapConfig.spec.js @@ -286,3 +286,83 @@ test('getMigratedMapConfig with colorScale with single value returns value', () }) ) }) + +test('getMigratedMapConfig with hidden: true mapView sets isVisible: false', () => { + const config = { + id: 'mapId', + name: 'map name', + basemap: { id: 'osmStreet' }, + mapViews: [{ layer: 'thematic', name: 'Hidden layer', hidden: true }], + } + const result = getMigratedMapConfig(config, defaultBasemapId) + expect(result.mapViews[0].isVisible).toBe(false) +}) + +test('getMigratedMapConfig with hidden: false mapView sets isVisible: true', () => { + const config = { + id: 'mapId', + name: 'map name', + basemap: { id: 'osmStreet' }, + mapViews: [{ layer: 'thematic', name: 'Visible layer', hidden: false }], + } + const result = getMigratedMapConfig(config, defaultBasemapId) + expect(result.mapViews[0].isVisible).toBe(true) +}) + +test('getMigratedMapConfig with JSON basemap string restores opacity and isVisible', () => { + const config = { + id: 'mapId', + name: 'map name', + basemap: JSON.stringify({ + id: 'osmLight', + opacity: 0.5, + hidden: false, + }), + mapViews: [{ layer: 'thematic', name: 'Layer' }], + } + const result = getMigratedMapConfig(config, defaultBasemapId) + expect(result.basemap).toEqual({ + id: 'osmLight', + opacity: 0.5, + isVisible: true, + }) +}) + +test('getMigratedMapConfig with JSON basemap string with hidden: true sets isVisible: false', () => { + const config = { + id: 'mapId', + name: 'map name', + basemap: JSON.stringify({ id: 'osmLight', opacity: 1, hidden: true }), + mapViews: [{ layer: 'thematic', name: 'Layer' }], + } + const result = getMigratedMapConfig(config, defaultBasemapId) + expect(result.basemap.isVisible).toBe(false) + expect(result.basemap.id).toBe('osmLight') +}) + +test('getMigratedMapConfig with v43 basemaps array restores opacity and isVisible', () => { + const config = { + id: 'mapId', + name: 'map name', + basemaps: [{ id: 'osmLight', opacity: 0.7, hidden: false }], + mapViews: [{ layer: 'thematic', name: 'Layer' }], + } + const result = getMigratedMapConfig(config, defaultBasemapId) + expect(result.basemap).toEqual({ + id: 'osmLight', + opacity: 0.7, + isVisible: true, + }) +}) + +test('getMigratedMapConfig with v43 basemaps array with hidden: true sets isVisible: false', () => { + const config = { + id: 'mapId', + name: 'map name', + basemaps: [{ id: 'osmLight', opacity: 1, hidden: true }], + mapViews: [{ layer: 'thematic', name: 'Layer' }], + } + const result = getMigratedMapConfig(config, defaultBasemapId) + expect(result.basemap.isVisible).toBe(false) + expect(result.basemap.id).toBe('osmLight') +})