From 3477be99b86d7a3d683fc912a189753db5e69d8d Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 23 Oct 2024 18:56:26 +0200 Subject: [PATCH] fix: do not try to create legend for non loaded classified layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix #2232 A classified layer needs to have compiled the data to known its classes/categories. This commit review the way we build the legend: instead of creating with the whole caption panel, we just set a container and we populate it later. This opens the door for live changing the legend when editing the layer. But we have to clarify that "reactive" pattern at some point, as we have some concurrent pattern laying around: the `render()`, which coupled with the schema and this is nice, but for now it's a bit rough (changing the whole UI each time); the `propagate` way, which is more specific, but not coupled to the schema; the `dataChanged`; and the `onDataLoaded` now, which is a bit different, as it's about the data being loaded, not changed/modified, but for the DOM it may at the end be the same. Well, food for thoughts… --- umap/static/umap/js/modules/caption.js | 14 +++++++--- umap/static/umap/js/modules/data/layer.js | 27 ++++++++++++++----- .../js/modules/rendering/layers/classified.js | 1 + umap/static/umap/map.css | 6 ++--- umap/tests/integration/test_caption.py | 6 ++--- 5 files changed, 37 insertions(+), 17 deletions(-) diff --git a/umap/static/umap/js/modules/caption.js b/umap/static/umap/js/modules/caption.js index d31dec94b..d68d478fa 100644 --- a/umap/static/umap/js/modules/caption.js +++ b/umap/static/umap/js/modules/caption.js @@ -40,15 +40,21 @@ export default class Caption { ) const creditsContainer = DomUtil.create('div', 'credits-container', container) this.addCredits(creditsContainer) - this.map.panel.open({ content: container }) + this.map.panel.open({ content: container }).then(() => { + // Create the legend when the panel is actually on the DOM + this.map.eachDataLayerReverse((datalayer) => datalayer.renderLegend()) + }) } addDataLayer(datalayer, container) { if (!datalayer.options.inCaption) return - const p = DomUtil.create('p', 'datalayer-legend', container) - const legend = DomUtil.create('span', '', p) + const p = DomUtil.create( + 'p', + `caption-item ${datalayer.cssId}`, + container + ) + const legend = DomUtil.create('span', 'datalayer-legend', p) const headline = DomUtil.create('strong', '', p) - datalayer.renderLegend(legend) if (datalayer.options.description) { DomUtil.element({ tagName: 'span', diff --git a/umap/static/umap/js/modules/data/layer.js b/umap/static/umap/js/modules/data/layer.js index 795a1f7d0..38c1cfa2e 100644 --- a/umap/static/umap/js/modules/data/layer.js +++ b/umap/static/umap/js/modules/data/layer.js @@ -115,6 +115,10 @@ export class DataLayer { return this._isDeleted } + get cssId() { + return `datalayer-${stamp(this)}` + } + getSyncMetadata() { return { subject: 'datalayer', @@ -235,6 +239,7 @@ export class DataLayer { } dataChanged() { + if (!this.hasDataLoaded()) return this.map.onDataLayersChanged() this.layer.dataChanged() } @@ -242,10 +247,15 @@ export class DataLayer { fromGeoJSON(geojson, sync = true) { this.addData(geojson, sync) this._geojson = geojson - this._dataloaded = true + this.onDataLoaded() this.dataChanged() } + onDataLoaded() { + this._dataloaded = true + this.renderLegend() + } + async fromUmapGeoJSON(geojson) { if (geojson._storage) geojson._umap_options = geojson._storage // Retrocompat if (geojson._umap_options) this.setOptions(geojson._umap_options) @@ -384,7 +394,7 @@ export class DataLayer { this.indexProperties(feature) this.map.features_index[feature.getSlug()] = feature this.showFeature(feature) - if (this.hasDataLoaded()) this.dataChanged() + this.dataChanged() } removeFeature(feature, sync) { @@ -395,7 +405,7 @@ export class DataLayer { feature.disconnectFromDataLayer(this) this._index.splice(this._index.indexOf(id), 1) delete this._features[id] - if (this.hasDataLoaded() && this.isVisible()) this.dataChanged() + if (this.isVisible()) this.dataChanged() } indexProperties(feature) { @@ -1119,10 +1129,13 @@ export class DataLayer { return 'displayName' } - renderLegend(container) { - if (this.layer.renderLegend) return this.layer.renderLegend(container) - const color = DomUtil.create('span', 'datalayer-color', container) - color.style.backgroundColor = this.getColor() + renderLegend() { + for (const container of document.querySelectorAll(`.${this.cssId} .datalayer-legend`)) { + container.innerHTML = '' + if (this.layer.renderLegend) return this.layer.renderLegend(container) + const color = DomUtil.create('span', 'datalayer-color', container) + color.style.backgroundColor = this.getColor() + } } renderToolbox(container) { diff --git a/umap/static/umap/js/modules/rendering/layers/classified.js b/umap/static/umap/js/modules/rendering/layers/classified.js index c5f7f8415..7b5eada4a 100644 --- a/umap/static/umap/js/modules/rendering/layers/classified.js +++ b/umap/static/umap/js/modules/rendering/layers/classified.js @@ -75,6 +75,7 @@ const ClassifiedMixin = { }, renderLegend: function (container) { + if (!this.datalayer.hasDataLoaded()) return const parent = DomUtil.create('ul', '', container) const items = this.getLegendItems() for (const [color, label] of items) { diff --git a/umap/static/umap/map.css b/umap/static/umap/map.css index ddb8bd0f8..ce59df085 100644 --- a/umap/static/umap/map.css +++ b/umap/static/umap/map.css @@ -1000,18 +1000,18 @@ a.umap-control-caption, vertical-align: middle; } -.datalayer-legend { +.caption-item { color: #555; padding: 6px 8px; box-shadow: 0 0 3px rgba(0,0,0,0.2); border-radius: 1px; } -.datalayer-legend ul { +.caption-item ul { list-style-type: none; padding: 0; margin: 0; } -.datalayer-legend .circles-layer-legend { +.caption-item .circles-layer-legend { padding: var(--box-padding); } .circles-layer-legend li { diff --git a/umap/tests/integration/test_caption.py b/umap/tests/integration/test_caption.py index f9228fe54..8e3785848 100644 --- a/umap/tests/integration/test_caption.py +++ b/umap/tests/integration/test_caption.py @@ -20,11 +20,11 @@ def test_caption(live_server, page, map): panel = page.locator(".panel.left.on") expect(panel).to_have_class(re.compile(".*condensed.*")) expect(panel.locator(".umap-caption")).to_be_visible() - expect(panel.locator(".datalayer-legend").get_by_text(basic.name)).to_be_visible() + expect(panel.locator(".caption-item").get_by_text(basic.name)).to_be_visible() expect( - panel.locator(".datalayer-legend .off").get_by_text(non_loaded.name) + panel.locator(".caption-item .off").get_by_text(non_loaded.name) ).to_be_visible() - expect(panel.locator(".datalayer-legend").get_by_text(hidden.name)).to_be_hidden() + expect(panel.locator(".caption-item").get_by_text(hidden.name)).to_be_hidden() def test_caption_should_display_owner_as_author(live_server, page, map):