fix: do not try to create legend for non loaded classified layer (#2234)

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

@@ -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 p = DomUtil.create(
-    const legend = DomUtil.create('span', '', p)
+      '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',

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) {
+  renderLegend() {
-    if (this.layer.renderLegend) return this.layer.renderLegend(container)
+    for (const container of document.querySelectorAll(`.${this.cssId} .datalayer-legend`)) {
-    const color = DomUtil.create('span', 'datalayer-color', container)
+      container.innerHTML = ''
-    color.style.backgroundColor = this.getColor()
+      if (this.layer.renderLegend) return this.layer.renderLegend(container)
+      const color = DomUtil.create('span', 'datalayer-color', container)
+      color.style.backgroundColor = this.getColor()
+    }
   }
 
   renderToolbox(container) {

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

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 {

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