From 4669053b18d40fa3f6ece557d17f35a4ab566fe9 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 27 Mar 2024 20:14:43 +0100 Subject: [PATCH] chore: refactor initCenter and controls ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We had an issue (not in Github :p) where a map was not loading because the defaultView was set to "data", and the layers were remote data layers. In this case, when computing the remote URL, we allow to replace georelated variables (like east, west, north, lat…), which needs the map to have a view. So: - the default view was expecting the data to be loaded (="data") - the data to be loaded needed a default view… So instead of adding yet another call to _setDefaultView in an edge case, we reordered the way we initialize the map elements: - first we initialize the controls (because initCenter needs the locate control to exist) - then we call initCenter - then we initialize the tile layers (because the miniMap needs it to render itself) - then we call renderControls --- umap/static/umap/js/umap.controls.js | 29 +++++++++++++++++ umap/static/umap/js/umap.js | 47 ++++++++++------------------ umap/tests/integration/test_map.py | 12 +++++++ 3 files changed, 58 insertions(+), 30 deletions(-) diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index 160d856c..138adcde 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -1234,6 +1234,35 @@ U.StarControl = L.Control.extend({ }, }) +/* + * Take control over L.Control.Locate to be able to + * call start() before adding the control (and thus the button) to the map. + */ +U.Locate = L.Control.Locate.extend({ + initialize: function (map, options) { + // When calling start(), it will try to add a location marker + // on the layer, which is normally added in the addTo/onAdd method + this._layer = this.options.layer = new L.LayerGroup() + // When calling start(), it will call _activate(), which then adds + // location related event listeners on the map + this.map = map + L.Control.Locate.prototype.initialize.call(this, options) + }, + + onAdd: function (map) { + const active = this._active + const container = L.Control.Locate.prototype.onAdd.call(this, map) + this._active = active + return container + }, + + _activate: function () { + this._map = this.map + L.Control.Locate.prototype._activate.call(this) + this._map = null + } +}) + U.Search = L.PhotonSearch.extend({ initialize: function (map, input, options) { this.options.placeholder = L._('Type a place name or coordinates') diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index 7383f74c..97de52c6 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -67,7 +67,8 @@ U.Map = L.Map.extend({ this.description = this.options.description this.demoTileInfos = this.options.demoTileInfos this.options.zoomControl = zoomControl !== undefined ? zoomControl : true - this.options.fullscreenControl = fullscreenControl !== undefined ? fullscreenControl : true + this.options.fullscreenControl = + fullscreenControl !== undefined ? fullscreenControl : true this.datalayersOnLoad = L.Util.queryString('datalayers') if (this.datalayersOnLoad) { this.datalayersOnLoad = this.datalayersOnLoad.toString().split(',') @@ -114,12 +115,12 @@ U.Map = L.Map.extend({ // Needed for actions labels this.help = new U.Help(this) - if (this.options.hash) this.addHash() - this.initTileLayers() - // Needs tilelayer to exist for minimap this.initControls() // Needs locate control and hash to exist this.initCenter() + this.initTileLayers() + // Needs tilelayer to exist for minimap + this.renderControls() this.handleLimitBounds() this.initDataLayers() @@ -268,9 +269,9 @@ U.Map = L.Map.extend({ }, overrideSchema: function (schema) { - for (const [key, extra] of Object.entries(schema)) { - U.SCHEMA[key] = L.extend({}, U.SCHEMA[key], extra) - } + for (const [key, extra] of Object.entries(schema)) { + U.SCHEMA[key] = L.extend({}, U.SCHEMA[key], extra) + } }, initControls: function () { @@ -297,7 +298,7 @@ U.Map = L.Map.extend({ zoomOutTitle: L._('Zoom out'), }) this._controls.datalayers = new U.DataLayersControl(this) - this._controls.locate = L.control.locate({ + this._controls.locate = new U.Locate(this, { strings: { title: L._('Center map on your location'), }, @@ -336,9 +337,6 @@ U.Map = L.Map.extend({ this.drop = new U.DropControl(this) this.share = new U.Share(this) this._controls.tilelayers = new U.TileLayerControl(this) - this._controls.tilelayers.setLayers() - - this.renderControls() }, renderControls: function () { @@ -353,13 +351,13 @@ U.Map = L.Map.extend({ 'umap-slideshow-enabled', this.options.slideshow && this.options.slideshow.active ) - for (const i in this._controls) { - this.removeControl(this._controls[i]) + for (const control of Object.values(this._controls)) { + this.removeControl(control) } if (this.options.noControl) return this._controls.attribution = new U.AttributionControl().addTo(this) - if (this.options.miniMap && !this.options.noControl) { + if (this.options.miniMap) { this.whenReady(function () { if (this.selected_tilelayer) { this._controls.miniMap = new L.Control.MiniMap(this.selected_tilelayer, { @@ -392,6 +390,7 @@ U.Map = L.Map.extend({ if (this.getOption('permanentCredit')) this._controls.permanentCredit.addTo(this) if (this.getOption('moreControl')) this._controls.more.addTo(this) if (this.getOption('scaleControl')) this._controls.scale.addTo(this) + this._controls.tilelayers.setLayers() }, initDataLayers: async function (datalayers) { @@ -652,26 +651,18 @@ U.Map = L.Map.extend({ }, initCenter: function () { + this._setDefaultCenter() + if (this.options.hash) this.addHash() if (this.options.hash && this._hash.parseHash(location.hash)) { // FIXME An invalid hash will cause the load to fail this._hash.update() } else if (this.options.defaultView === 'locate' && !this.options.noControl) { - // When using locate as default map view AND activating easing - // Leaflet.locate will ask the map view to compute transition to user - // position, so in this case we do need a default center, so let's - // set it anyway - this._setDefaultCenter() this._controls.locate.start() } else if (this.options.defaultView === 'data') { - this.onceDataLoaded(() => { - if (!this.fitDataBounds()) return this._setDefaultCenter() - }) + this.onceDataLoaded(this.fitDataBounds) } else if (this.options.defaultView === 'latest') { this.onceDataLoaded(() => { - if (!this.hasData()) { - this._setDefaultCenter() - return - } + if (!this.hasData()) return const datalayer = this.firstVisibleDatalayer() let feature if (datalayer) { @@ -681,11 +672,7 @@ U.Map = L.Map.extend({ return } } - // Fallback, no datalayer or no feature found - this._setDefaultCenter() }) - } else { - this._setDefaultCenter() } }, diff --git a/umap/tests/integration/test_map.py b/umap/tests/integration/test_map.py index ef27ab72..b5356192 100644 --- a/umap/tests/integration/test_map.py +++ b/umap/tests/integration/test_map.py @@ -148,6 +148,18 @@ def test_default_view_latest_with_polygon(map, live_server, page): expect(layers).to_have_count(1) +def test_default_view_locate(browser, live_server, map): + context = browser.new_context( + geolocation={"longitude": 8.52967, "latitude": 39.16267}, + permissions=["geolocation"], + ) + map.settings["properties"]["defaultView"] = "locate" + map.save() + page = context.new_page() + page.goto(f"{live_server.url}{map.get_absolute_url()}") + expect(page).to_have_url(re.compile(r".*#18/39\.16267/8\.52967")) + + def test_remote_layer_should_not_be_used_as_datalayer_for_created_features( map, live_server, datalayer, page ):