Skip to content

Commit

Permalink
fix: do not try to create legend for non loaded classified layer
Browse files Browse the repository at this point in the history
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…
  • Loading branch information
yohanboniface committed Oct 23, 2024
1 parent 45f1221 commit 3477be9
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 17 deletions.
14 changes: 10 additions & 4 deletions umap/static/umap/js/modules/caption.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
27 changes: 20 additions & 7 deletions umap/static/umap/js/modules/data/layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ export class DataLayer {
return this._isDeleted
}

get cssId() {
return `datalayer-${stamp(this)}`
}

getSyncMetadata() {
return {
subject: 'datalayer',
Expand Down Expand Up @@ -235,17 +239,23 @@ export class DataLayer {
}

dataChanged() {
if (!this.hasDataLoaded()) return
this.map.onDataLayersChanged()
this.layer.dataChanged()
}

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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions umap/static/umap/js/modules/rendering/layers/classified.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions umap/static/umap/map.css
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions umap/tests/integration/test_caption.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 3477be9

Please sign in to comment.