From c334f7554e4357924f75f6d27de80ab3c0f5e99e Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Thu, 1 Feb 2024 12:24:22 +0100 Subject: [PATCH 1/3] feat: add experimental "map preview" This is the same as "map new", but it is not in edit mode. This allow to click on the elements and see the popups instead of editing it when using a `dataUrl` query string. This way of using uMap is not documented, but it's used by some partners (Deveco recently, data.gouv.fr historicaly). In the same time, this PR adds two things: - possibility to pass data direclty in querystring (instead of an URL): in the case of Deveco, they have pages where only point is shown (for each company) - possibility to pass style options directly from query string: may allow for example to control the `popupTemplate`, eg. to use a table one that will display all properties of the clicked feature Note: dataUrl and such also works in normal "map new" view. There are two use cases around those parameters, from external sites: - see this data on uMap (should point on map preview) - create a map with those data (should point on map new) --- umap/static/umap/js/umap.js | 25 +++++-- umap/tests/integration/test_map_preview.py | 78 ++++++++++++++++++++++ umap/urls.py | 1 + umap/views.py | 43 +++++++----- 4 files changed, 128 insertions(+), 19 deletions(-) create mode 100644 umap/tests/integration/test_map_preview.py diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index 929c6009..7935eaeb 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -233,18 +233,35 @@ L.U.Map.include({ // Creation mode if (!this.options.umap_id) { - this.isDirty = true + if (!this.options.preview) { + this.isDirty = true + this.enableEdit() + } this._default_extent = true this.options.name = L._('Untitled map') - this.options.editMode = 'advanced' - this.enableEdit() + let style = L.Util.queryString('style', null) + if (style) { + style = decodeURIComponent(style) + try { + style = JSON.parse(style) + L.Util.setOptions(this, style) + } catch (error) { + console.error(error) + } + } + let data = L.Util.queryString('data', null) let dataUrl = L.Util.queryString('dataUrl', null) const dataFormat = L.Util.queryString('dataFormat', 'geojson') if (dataUrl) { dataUrl = decodeURIComponent(dataUrl) dataUrl = this.localizeUrl(dataUrl) dataUrl = this.proxyUrl(dataUrl) + const datalayer = this.createDataLayer() datalayer.importFromUrl(dataUrl, dataFormat) + } else if (data) { + data = decodeURIComponent(data) + const datalayer = this.createDataLayer() + datalayer.importRaw(data, dataFormat) } } @@ -290,7 +307,7 @@ L.U.Map.include({ } }) - window.onbeforeunload = () => this.isDirty || null + window.onbeforeunload = () => this.editEnabled && this.isDirty || null this.backup() this.initContextMenu() this.on('click contextmenu.show', this.closeInplaceToolbar) diff --git a/umap/tests/integration/test_map_preview.py b/umap/tests/integration/test_map_preview.py new file mode 100644 index 00000000..9b4ba0d3 --- /dev/null +++ b/umap/tests/integration/test_map_preview.py @@ -0,0 +1,78 @@ +import json +from urllib.parse import quote + +import pytest +from playwright.sync_api import expect + +pytestmark = pytest.mark.django_db + +GEOJSON = { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": { + "name": "Niagara Falls", + }, + "geometry": { + "type": "Point", + "coordinates": [-79.04, 43.08], + }, + } + ], +} +CSV = "name,latitude,longitude\nNiagara Falls,43.08,-79.04" + + +def test_map_preview(page, live_server, tilelayer): + page.goto(f"{live_server.url}/map/") + # Edit mode is not enabled + edit_button = page.get_by_role("button", name="Edit") + expect(edit_button).to_be_visible() + + +def test_map_preview_can_load_remote_geojson(page, live_server, tilelayer): + def handle(route): + route.fulfill(json=GEOJSON) + + # Intercept the route to the proxy + page.route("*/**/ajax-proxy/**", handle) + + page.goto(f"{live_server.url}/map/?dataUrl=http://some.org/geo.json") + markers = page.locator(".leaflet-marker-icon") + expect(markers).to_have_count(1) + + +def test_map_preview_can_load_remote_csv(page, live_server, tilelayer): + def handle(route): + csv = """name,latitude,longitude\nNiagara Falls,43.08,-79.04""" + route.fulfill(body=csv) + + # Intercept the route to the proxy + page.route("*/**/ajax-proxy/**", handle) + + page.goto(f"{live_server.url}/map/?dataUrl=http://some.org/geo.csv&dataFormat=csv") + markers = page.locator(".leaflet-marker-icon") + expect(markers).to_have_count(1) + + +def test_map_preview_can_load_geojson_in_querystring(page, live_server, tilelayer): + page.goto(f"{live_server.url}/map/?data={quote(json.dumps(GEOJSON))}") + markers = page.locator(".leaflet-marker-icon") + expect(markers).to_have_count(1) + + +def test_map_preview_can_load_csv_in_querystring(page, live_server, tilelayer): + page.goto(f"{live_server.url}/map/?data={quote(CSV)}&dataFormat=csv") + markers = page.locator(".leaflet-marker-icon") + expect(markers).to_have_count(1) + + +def test_map_preview_can_change_styling_from_querystring(page, live_server, tilelayer): + style = {"color": "DarkRed"} + page.goto( + f"{live_server.url}/map/?data={quote(json.dumps(GEOJSON))}&style={quote(json.dumps(style))}" + ) + markers = page.locator(".leaflet-marker-icon .icon_container") + expect(markers).to_have_count(1) + expect(markers).to_have_css("background-color", "rgb(139, 0, 0)") diff --git a/umap/urls.py b/umap/urls.py index 422c2e78..a62c1efc 100644 --- a/umap/urls.py +++ b/umap/urls.py @@ -96,6 +96,7 @@ i18n_urls += decorated_patterns( ) i18n_urls += decorated_patterns( [ensure_csrf_cookie], + re_path(r"^map/$", views.MapPreview.as_view(), name="map_preview"), re_path(r"^map/new/$", views.MapNew.as_view(), name="map_new"), ) i18n_urls += decorated_patterns( diff --git a/umap/views.py b/umap/views.py index f4484269..0d670058 100644 --- a/umap/views.py +++ b/umap/views.py @@ -455,8 +455,7 @@ class MapDetailMixin: if domain and "{" not in domain: context["preconnect_domains"] = [f"//{domain}"] - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) + def get_map_properties(self): user = self.request.user properties = { "urls": _urls_for_js(), @@ -486,6 +485,17 @@ class MapDetailMixin: if self.get_short_url(): properties["shortUrl"] = self.get_short_url() + if not user.is_anonymous: + properties["user"] = { + "id": user.pk, + "name": str(user), + "url": reverse("user_dashboard"), + } + return properties + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + properties = self.get_map_properties() if settings.USE_I18N: lang = settings.LANGUAGE_CODE # Check attr in case the middleware is not active @@ -495,19 +505,13 @@ class MapDetailMixin: locale = to_locale(lang) properties["locale"] = locale context["locale"] = locale - if not user.is_anonymous: - properties["user"] = { - "id": user.pk, - "name": str(user), - "url": reverse("user_dashboard"), - } - map_settings = self.get_geojson() - if "properties" not in map_settings: - map_settings["properties"] = {} - map_settings["properties"].update(properties) - map_settings["properties"]["datalayers"] = self.get_datalayers() - context["map_settings"] = json.dumps(map_settings, indent=settings.DEBUG) - self.set_preconnect(map_settings["properties"], context) + geojson = self.get_geojson() + if "properties" not in geojson: + geojson["properties"] = {} + geojson["properties"].update(properties) + geojson["properties"]["datalayers"] = self.get_datalayers() + context["map_settings"] = json.dumps(geojson, indent=settings.DEBUG) + self.set_preconnect(geojson["properties"], context) return context def get_datalayers(self): @@ -709,6 +713,15 @@ class MapNew(MapDetailMixin, TemplateView): template_name = "umap/map_detail.html" +class MapPreview(MapDetailMixin, TemplateView): + template_name = "umap/map_detail.html" + + def get_map_properties(self): + properties = super().get_map_properties() + properties["preview"] = True + return properties + + class MapCreate(FormLessEditMixin, PermissionsMixin, CreateView): model = Map form_class = MapSettingsForm From f09e399b3c7151601ca85a93bdb95b7da532c572 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 7 Feb 2024 17:53:45 +0100 Subject: [PATCH 2/3] chore: refactor setting Map options from querystring --- umap/static/umap/js/umap.js | 180 +++++++++++---------- umap/tests/integration/test_map_preview.py | 5 +- umap/tests/integration/test_querystring.py | 13 ++ 3 files changed, 105 insertions(+), 93 deletions(-) diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index 7935eaeb..c7161d8e 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -69,6 +69,65 @@ L.U.Map.include({ 'tilelayers', ], + editableOptions: { + 'zoom': undefined, + 'scrollWheelZoom': Boolean, + 'scaleControl': Boolean, + 'moreControl': Boolean, + 'miniMap': Boolean, + 'displayPopupFooter': undefined, + 'onLoadPanel': String, + 'defaultView': String, + 'name': String, + 'description': String, + 'licence': undefined, + 'tilelayer': undefined, + 'overlay': undefined, + 'limitBounds': undefined, + 'color': String, + 'iconClass': String, + 'iconUrl': String, + 'smoothFactor': undefined, + 'iconOpacity': undefined, + 'opacity': undefined, + 'weight': undefined, + 'fill': undefined, + 'fillColor': undefined, + 'fillOpacity': undefined, + 'dashArray': undefined, + 'popupShape': String, + 'popupTemplate': String, + 'popupContentTemplate': String, + 'zoomTo': undefined, + 'captionBar': Boolean, + 'captionMenus': Boolean, + 'slideshow': undefined, + 'sortKey': undefined, + 'labelKey': undefined, + 'filterKey': undefined, + 'facetKey': undefined, + 'slugKey': undefined, + 'showLabel': undefined, + 'labelDirection': undefined, + 'labelInteractive': undefined, + 'outlinkTarget': undefined, + 'shortCredit': undefined, + 'longCredit': undefined, + 'permanentCredit': undefined, + 'permanentCreditBackground': undefined, + 'zoomControl': 'NullableBoolean', + 'datalayersControl': 'NullableBoolean', + 'searchControl': 'NullableBoolean', + 'locateControl': 'NullableBoolean', + 'fullscreenControl': 'NullableBoolean', + 'editinosmControl': 'NullableBoolean', + 'embedControl': 'NullableBoolean', + 'measureControl': 'NullableBoolean', + 'tilelayersControl': 'NullableBoolean', + 'starControl': 'NullableBoolean', + 'easing': undefined, + }, + initialize: function (el, geojson) { // Locale name (pt_PT, en_US…) // To be used for Django localization @@ -89,7 +148,7 @@ L.U.Map.include({ ? geojson.properties.fullscreenControl : true geojson.properties.fullscreenControl = false - L.Util.setBooleanFromQueryString(geojson.properties, 'scrollWheelZoom') + this.setOptionsFromQueryString(geojson.properties) L.Map.prototype.initialize.call(this, el, geojson.properties) @@ -109,32 +168,10 @@ L.U.Map.include({ this.demoTileInfos = this.options.demoTileInfos this.options.zoomControl = zoomControl this.options.fullscreenControl = fullscreenControl - L.Util.setBooleanFromQueryString(this.options, 'moreControl') - L.Util.setBooleanFromQueryString(this.options, 'scaleControl') - L.Util.setBooleanFromQueryString(this.options, 'miniMap') - L.Util.setFromQueryString(this.options, 'editMode') - L.Util.setBooleanFromQueryString(this.options, 'displayDataBrowserOnLoad') - L.Util.setBooleanFromQueryString(this.options, 'displayCaptionOnLoad') - L.Util.setBooleanFromQueryString(this.options, 'captionBar') - L.Util.setBooleanFromQueryString(this.options, 'captionMenus') - for (let i = 0; i < this.HIDDABLE_CONTROLS.length; i++) { - L.Util.setNullableBooleanFromQueryString( - this.options, - `${this.HIDDABLE_CONTROLS[i]}Control` - ) - } - // Specific case for datalayersControl - // which accept "expanded" value, on top of true/false/null - if (L.Util.queryString('datalayersControl') === 'expanded') { - L.Util.setFromQueryString(this.options, 'datalayersControl') - } this.datalayersOnLoad = L.Util.queryString('datalayers') - this.options.onLoadPanel = L.Util.queryString( - 'onLoadPanel', - this.options.onLoadPanel - ) - if (this.datalayersOnLoad) + if (this.datalayersOnLoad) { this.datalayersOnLoad = this.datalayersOnLoad.toString().split(',') + } if (L.Browser.ielt9) this.options.editMode = 'disabled' // TODO include ie9 @@ -313,6 +350,31 @@ L.U.Map.include({ this.on('click contextmenu.show', this.closeInplaceToolbar) }, + setOptionsFromQueryString: function (options) { + // This is not an editable option + L.Util.setFromQueryString(options, 'editMode') + // FIXME retrocompat + L.Util.setBooleanFromQueryString(options, 'displayDataBrowserOnLoad') + L.Util.setBooleanFromQueryString(options, 'displayCaptionOnLoad') + for (const [key, type] of Object.entries(this.editableOptions)) { + switch (type) { + case Boolean: + L.Util.setBooleanFromQueryString(options, key) + break + case 'NullableBoolean': + L.Util.setNullableBooleanFromQueryString(options, key) + break + case String: + L.Util.setFromQueryString(options, key) + } + } + // Specific case for datalayersControl + // which accepts "expanded" value, on top of true/false/null + if (L.Util.queryString('datalayersControl') === 'expanded') { + L.Util.setFromQueryString(options, 'datalayersControl') + } + }, + initControls: function () { this.helpMenuActions = {} this._controls = {} @@ -869,8 +931,7 @@ L.U.Map.include({ let mustReindex = false - for (let i = 0; i < this.editableOptions.length; i++) { - const option = this.editableOptions[i] + for (const option of Object.keys(this.editableOptions)) { if (typeof importedData.properties[option] !== 'undefined') { this.options[option] = importedData.properties[option] if (option === 'sortKey') mustReindex = true @@ -997,70 +1058,11 @@ L.U.Map.include({ else this.fire('saved') }, - editableOptions: [ - 'zoom', - 'scrollWheelZoom', - 'scaleControl', - 'moreControl', - 'miniMap', - 'displayPopupFooter', - 'onLoadPanel', - 'defaultView', - 'name', - 'description', - 'licence', - 'tilelayer', - 'overlay', - 'limitBounds', - 'color', - 'iconClass', - 'iconUrl', - 'smoothFactor', - 'iconOpacity', - 'opacity', - 'weight', - 'fill', - 'fillColor', - 'fillOpacity', - 'dashArray', - 'popupShape', - 'popupTemplate', - 'popupContentTemplate', - 'zoomTo', - 'captionBar', - 'captionMenus', - 'slideshow', - 'sortKey', - 'labelKey', - 'filterKey', - 'facetKey', - 'slugKey', - 'showLabel', - 'labelDirection', - 'labelInteractive', - 'outlinkTarget', - 'shortCredit', - 'longCredit', - 'permanentCredit', - 'permanentCreditBackground', - 'zoomControl', - 'datalayersControl', - 'searchControl', - 'locateControl', - 'fullscreenControl', - 'editinosmControl', - 'embedControl', - 'measureControl', - 'tilelayersControl', - 'starControl', - 'easing', - ], - exportOptions: function () { const properties = {} - for (let i = this.editableOptions.length - 1; i >= 0; i--) { - if (typeof this.options[this.editableOptions[i]] !== 'undefined') { - properties[this.editableOptions[i]] = this.options[this.editableOptions[i]] + for (const option of Object.keys(this.editableOptions)) { + if (typeof this.options[option] !== 'undefined') { + properties[option] = this.options[option] } } return properties diff --git a/umap/tests/integration/test_map_preview.py b/umap/tests/integration/test_map_preview.py index 9b4ba0d3..9176eba8 100644 --- a/umap/tests/integration/test_map_preview.py +++ b/umap/tests/integration/test_map_preview.py @@ -69,10 +69,7 @@ def test_map_preview_can_load_csv_in_querystring(page, live_server, tilelayer): def test_map_preview_can_change_styling_from_querystring(page, live_server, tilelayer): - style = {"color": "DarkRed"} - page.goto( - f"{live_server.url}/map/?data={quote(json.dumps(GEOJSON))}&style={quote(json.dumps(style))}" - ) + page.goto(f"{live_server.url}/map/?data={quote(json.dumps(GEOJSON))}&color=DarkRed") markers = page.locator(".leaflet-marker-icon .icon_container") expect(markers).to_have_count(1) expect(markers).to_have_css("background-color", "rgb(139, 0, 0)") diff --git a/umap/tests/integration/test_querystring.py b/umap/tests/integration/test_querystring.py index faab701e..5118fe6c 100644 --- a/umap/tests/integration/test_querystring.py +++ b/umap/tests/integration/test_querystring.py @@ -1,3 +1,5 @@ +import re + import pytest from playwright.sync_api import expect @@ -37,3 +39,14 @@ def test_datalayers_control(map, live_server, datalayer, page): page.goto(f"{live_server.url}{map.get_absolute_url()}?datalayersControl=expanded") expect(control).to_be_hidden() expect(box).to_be_visible() + + +def test_can_deactivate_wheel_from_query_string(map, live_server, page): + page.goto(f"{live_server.url}{map.get_absolute_url()}") + expect(page).to_have_url(re.compile(r".*#7/.+")) + page.mouse.wheel(0, 1) + expect(page).to_have_url(re.compile(r".*#6/.+")) + page.goto(f"{live_server.url}{map.get_absolute_url()}?scrollWheelZoom=false") + expect(page).to_have_url(re.compile(r".*#7/.+")) + page.mouse.wheel(0, 1) + expect(page).to_have_url(re.compile(r".*#7/.+")) From 7943c61b3eec4475f46f4f23be5d6aade097e177 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 7 Feb 2024 18:47:00 +0100 Subject: [PATCH 3/3] chore: do not consume style query string now that we handle keys separatly --- umap/static/umap/js/umap.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index c7161d8e..ee7527dc 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -276,16 +276,6 @@ L.U.Map.include({ } this._default_extent = true this.options.name = L._('Untitled map') - let style = L.Util.queryString('style', null) - if (style) { - style = decodeURIComponent(style) - try { - style = JSON.parse(style) - L.Util.setOptions(this, style) - } catch (error) { - console.error(error) - } - } let data = L.Util.queryString('data', null) let dataUrl = L.Util.queryString('dataUrl', null) const dataFormat = L.Util.queryString('dataFormat', 'geojson')