From ba0582deb18199d8c6dbcc17b6775d87ce332cb1 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Mon, 10 Feb 2025 13:13:56 +0100 Subject: [PATCH 1/3] chore: refactor layer.isLoaded() --- umap/static/umap/js/modules/data/layer.js | 14 +++++--------- umap/static/umap/js/modules/sync/updaters.js | 4 ---- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/umap/static/umap/js/modules/data/layer.js b/umap/static/umap/js/modules/data/layer.js index 1ca1b785..7208f06b 100644 --- a/umap/static/umap/js/modules/data/layer.js +++ b/umap/static/umap/js/modules/data/layer.js @@ -45,7 +45,6 @@ export class DataLayer extends ServerStored { this._features = {} this._geojson = null this._propertiesIndex = [] - this._loaded = false // Are layer metadata loaded this._dataloaded = false // Are layer data loaded this._leafletMap = leafletMap @@ -85,6 +84,7 @@ export class DataLayer extends ServerStored { this.connectToMap() this.permissions = new DataLayerPermissions(this._umap, this) + this._needsFetch = this.createdOnServer if (!this.createdOnServer) { if (this.showAtLoad()) this.show() } @@ -268,7 +268,7 @@ export class DataLayer extends ServerStored { if (geojson._umap_options) this.setOptions(geojson._umap_options) if (this.isRemoteLayer()) await this.fetchRemoteData() else this.fromGeoJSON(geojson, false) - this._loaded = true + this._needsFetch = false } clear() { @@ -345,7 +345,7 @@ export class DataLayer extends ServerStored { } isLoaded() { - return !this.createdOnServer || this._loaded + return !this._needsFetch } hasDataLoaded() { @@ -633,7 +633,6 @@ export class DataLayer extends ServerStored { this.propagateDelete() this._leaflet_events_bk = this._leaflet_events this.clear() - delete this._loaded delete this._dataloaded } @@ -652,7 +651,6 @@ export class DataLayer extends ServerStored { this.hide() if (this.isRemoteLayer()) this.fetchRemoteData() else if (this._geojson_bk) this.fromGeoJSON(this._geojson_bk) - this._loaded = true this.show() this.isDirty = false } @@ -1108,9 +1106,7 @@ export class DataLayer extends ServerStored { async save() { if (this.isDeleted) return await this.saveDelete() - if (!this.isLoaded()) { - return - } + if (!this.isLoaded()) return const geojson = this.umapGeoJSON() const formData = new FormData() formData.append('name', this.options.name) @@ -1172,7 +1168,7 @@ export class DataLayer extends ServerStored { this.backupOptions() this.backupData() this.connectToMap() - this._loaded = true + this._needsFetch = false this.redraw() // Needed for reordering features return true } diff --git a/umap/static/umap/js/modules/sync/updaters.js b/umap/static/umap/js/modules/sync/updaters.js index f694d23e..9c6ff196 100644 --- a/umap/static/umap/js/modules/sync/updaters.js +++ b/umap/static/umap/js/modules/sync/updaters.js @@ -55,15 +55,11 @@ export class DataLayerUpdater extends BaseUpdater { upsert({ value }) { // Upsert only happens when a new datalayer is created. const datalayer = this._umap.createDataLayer(value, false) - // Prevent the layer to get data from the server, as it will get it - // from the sync. - datalayer._loaded = true } update({ key, metadata, value }) { const datalayer = this.getDataLayerFromID(metadata.id) if (fieldInSchema(key)) { - datalayer._loaded = true this.updateObjectValue(datalayer, key, value) } else { console.debug( From 175e27a535793c6d4c839f2490bc884326190e4d Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Mon, 10 Feb 2025 15:44:41 +0100 Subject: [PATCH 2/3] chore: remove DataLayer._dataloaded in favor of isLoaded() At the end, we only need two states: has this datalayer loaded the data it should load ? yes / no. Whether it local or remote data should not be a matter. --- umap/static/umap/js/modules/data/layer.js | 13 +++---------- .../umap/js/modules/rendering/layers/classified.js | 2 +- umap/tests/integration/test_websocket_sync.py | 4 +++- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/umap/static/umap/js/modules/data/layer.js b/umap/static/umap/js/modules/data/layer.js index 7208f06b..04ef3757 100644 --- a/umap/static/umap/js/modules/data/layer.js +++ b/umap/static/umap/js/modules/data/layer.js @@ -45,7 +45,6 @@ export class DataLayer extends ServerStored { this._features = {} this._geojson = null this._propertiesIndex = [] - this._dataloaded = false // Are layer data loaded this._leafletMap = leafletMap this.parentPane = this._leafletMap.getPane('overlayPane') @@ -243,7 +242,7 @@ export class DataLayer extends ServerStored { } dataChanged() { - if (!this.hasDataLoaded()) return + if (!this.isLoaded()) return this._umap.onDataLayersChanged() this.layer.dataChanged() } @@ -252,13 +251,13 @@ export class DataLayer extends ServerStored { if (!geojson) return [] const features = this.addData(geojson, sync) this._geojson = geojson + this._needsFetch = false this.onDataLoaded() this.dataChanged() return features } onDataLoaded() { - this._dataloaded = true this.renderLegend() } @@ -268,7 +267,6 @@ export class DataLayer extends ServerStored { if (geojson._umap_options) this.setOptions(geojson._umap_options) if (this.isRemoteLayer()) await this.fetchRemoteData() else this.fromGeoJSON(geojson, false) - this._needsFetch = false } clear() { @@ -320,7 +318,7 @@ export class DataLayer extends ServerStored { async fetchRemoteData(force) { if (!this.isRemoteLayer()) return - if (!this.hasDynamicData() && this.hasDataLoaded() && !force) return + if (!this.hasDynamicData() && this.isLoaded() && !force) return if (!this.isVisible()) return // Keep non proxied url for later use in Alert. const remoteUrl = this._umap.renderUrl(this.options.remoteData.url) @@ -348,10 +346,6 @@ export class DataLayer extends ServerStored { return !this._needsFetch } - hasDataLoaded() { - return this._dataloaded - } - backupOptions() { this._backupOptions = Utils.CopyJSON(this.options) } @@ -633,7 +627,6 @@ export class DataLayer extends ServerStored { this.propagateDelete() this._leaflet_events_bk = this._leaflet_events this.clear() - delete this._dataloaded } reset() { diff --git a/umap/static/umap/js/modules/rendering/layers/classified.js b/umap/static/umap/js/modules/rendering/layers/classified.js index 138f53b0..6b8f39aa 100644 --- a/umap/static/umap/js/modules/rendering/layers/classified.js +++ b/umap/static/umap/js/modules/rendering/layers/classified.js @@ -75,7 +75,7 @@ const ClassifiedMixin = { }, renderLegend: function (container) { - if (!this.datalayer.hasDataLoaded()) return + if (!this.datalayer.isLoaded()) return const parent = DomUtil.create('ul', '', container) const items = this.getLegendItems() for (const [color, label] of items) { diff --git a/umap/tests/integration/test_websocket_sync.py b/umap/tests/integration/test_websocket_sync.py index dc1ae21a..e0c19d47 100644 --- a/umap/tests/integration/test_websocket_sync.py +++ b/umap/tests/integration/test_websocket_sync.py @@ -398,7 +398,9 @@ def test_should_sync_datalayers(new_page, asgi_live_server, tilelayer): peerA.locator("#map").click() # Make sure this new marker is in Layer 2 for peerB - expect(peerB.get_by_text("Layer 2")).to_be_visible() + # Show features for this layer in the brower. + peerB.get_by_role("heading", name="Layer 2").locator(".datalayer-name").click() + expect(peerB.locator("li").filter(has_text="Layer 2")).to_be_visible() peerB.locator(".panel.left").get_by_role("button", name="Show/hide layer").nth( 1 ).click() From a8e18c167ca1cc18c8ec09f7c9e25b339236a11c Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Mon, 10 Feb 2025 16:30:00 +0100 Subject: [PATCH 3/3] chore: no need to reset datalayer._needsFetch after a save It is reset only after loading the layer data. Co-authored-by: David Larlet --- umap/static/umap/js/modules/data/layer.js | 1 - 1 file changed, 1 deletion(-) diff --git a/umap/static/umap/js/modules/data/layer.js b/umap/static/umap/js/modules/data/layer.js index 04ef3757..b8f06bb5 100644 --- a/umap/static/umap/js/modules/data/layer.js +++ b/umap/static/umap/js/modules/data/layer.js @@ -1161,7 +1161,6 @@ export class DataLayer extends ServerStored { this.backupOptions() this.backupData() this.connectToMap() - this._needsFetch = false this.redraw() // Needed for reordering features return true }