From 0ad6e425b58efe954bd118f630ed4e8824695269 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Mon, 25 Sep 2023 13:42:14 +0200 Subject: [PATCH] Refactor data browser - move a dedicated class (function was becoming big) - user FormBuilder instead of custom form elements --- umap/static/umap/js/umap.browser.js | 142 +++++++++++++++++++++++++ umap/static/umap/js/umap.controls.js | 135 +---------------------- umap/static/umap/js/umap.forms.js | 2 +- umap/static/umap/js/umap.js | 3 +- umap/static/umap/js/umap.layer.js | 2 +- umap/static/umap/test/index.html | 1 + umap/templates/umap/js.html | 1 + umap/tests/integration/test_browser.py | 133 +++++++++++++++++++++++ 8 files changed, 282 insertions(+), 137 deletions(-) create mode 100644 umap/static/umap/js/umap.browser.js create mode 100644 umap/tests/integration/test_browser.py diff --git a/umap/static/umap/js/umap.browser.js b/umap/static/umap/js/umap.browser.js new file mode 100644 index 00000000..c773dcbb --- /dev/null +++ b/umap/static/umap/js/umap.browser.js @@ -0,0 +1,142 @@ +L.U.Browser = L.Class.extend({ + options: { + filter: '', + inBbox: false, + }, + + initialize: function (map) { + this.map = map + }, + + addFeature: function (feature) { + const feature_li = L.DomUtil.create('li', `${feature.getClassName()} feature`), + zoom_to = L.DomUtil.create('i', 'feature-zoom_to', feature_li), + edit = L.DomUtil.create('i', 'show-on-edit feature-edit', feature_li), + del = L.DomUtil.create('i', 'show-on-edit feature-delete', feature_li), + color = L.DomUtil.create('i', 'feature-color', feature_li), + title = L.DomUtil.create('span', 'feature-title', feature_li), + symbol = feature._getIconUrl + ? L.U.Icon.prototype.formatUrl(feature._getIconUrl(), feature) + : null + zoom_to.title = L._('Bring feature to center') + edit.title = L._('Edit this feature') + del.title = L._('Delete this feature') + title.textContent = feature.getDisplayName() || '—' + color.style.backgroundColor = feature.getOption('color') + if (symbol) color.style.backgroundImage = `url(${symbol})` + L.DomEvent.on( + zoom_to, + 'click', + function (e) { + e.callback = L.bind(this.view, this) + this.zoomTo(e) + }, + feature + ) + L.DomEvent.on( + title, + 'click', + function (e) { + e.callback = L.bind(this.view, this) + this.zoomTo(e) + }, + feature + ) + L.DomEvent.on(edit, 'click', feature.edit, feature) + L.DomEvent.on(del, 'click', feature.confirmDelete, feature) + return feature_li + }, + + addDatalayer: function (datalayer, dataContainer) { + const filterKeys = this.map.getFilterKeys() + const container = L.DomUtil.create( + 'div', + datalayer.getHidableClass(), + dataContainer + ), + headline = L.DomUtil.create('h5', '', container) + container.id = `browse_data_datalayer_${datalayer.umap_id}` + datalayer.renderToolbox(headline) + L.DomUtil.add('span', '', headline, datalayer.options.name) + const ul = L.DomUtil.create('ul', '', container) + L.DomUtil.classIf(container, 'off', !datalayer.isVisible()) + + const build = () => { + ul.innerHTML = '' + const bounds = this.map.getBounds() + datalayer.eachFeature((feature) => { + if ( + this.options.filter && + !feature.matchFilter(this.options.filter, filterKeys) + ) + return + if (this.options.inBbox && !feature.isOnScreen(bounds)) return + ul.appendChild(this.addFeature(feature)) + }) + } + + build() + datalayer.on('datachanged', build) + datalayer.map.ui.once('panel:closed', () => { + datalayer.off('datachanged', build) + this.map.off('moveend', build) + }) + datalayer.map.ui.once('panel:ready', () => { + datalayer.map.ui.once('panel:ready', () => { + datalayer.off('datachanged', build) + }) + }) + }, + + open: function () { + const container = L.DomUtil.create('div', 'umap-browse-data') + // HOTFIX. Remove when this is merged and released: + // https://github.com/Leaflet/Leaflet/pull/9052 + L.DomEvent.disableClickPropagation(container) + + const title = L.DomUtil.add( + 'h3', + 'umap-browse-title', + container, + this.map.options.name + ) + + const formContainer = L.DomUtil.create('div', '', container) + const dataContainer = L.DomUtil.create('div', 'umap-browse-features', container) + + const appendAll = () => { + dataContainer.innerHTML = '' + this.map.eachBrowsableDataLayer((datalayer) => { + this.addDatalayer(datalayer, dataContainer) + }) + } + const resetLayers = () => { + this.map.eachBrowsableDataLayer((datalayer) => { + datalayer.resetLayer(true) + }) + } + const fields = [ + ['options.filter', { handler: 'Input' }], + ['options.inBbox', { handler: 'Switch', label: L._('Current map view') }], + ] + const builder = new L.U.FormBuilder(this, fields, { + makeDirty: false, + callback: (e) => { + if (e.helper.field === 'options.inBbox') { + if (this.options.inBbox) this.map.on('moveend', appendAll) + else this.map.off('moveend', appendAll) + } + appendAll() + resetLayers() + }, + }) + formContainer.appendChild(builder.build()) + + appendAll() + + this.map.ui.openPanel({ + data: { html: container }, + actions: [this.map._aboutLink()], + }) + }, +}) diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index 726b9591..55877973 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -615,8 +615,7 @@ L.U.DataLayer.include({ remove.title = L._('Delete layer') if (this.isReadOnly()) { L.DomUtil.addClass(container, 'readonly') - } - else { + } else { L.DomEvent.on(edit, 'click', this.edit, this) L.DomEvent.on(table, 'click', this.tableEdit, this) L.DomEvent.on( @@ -677,138 +676,6 @@ L.U.DataLayer.addInitHook(function () { }) L.U.Map.include({ - _openBrowser: function () { - const browserContainer = L.DomUtil.create('div', 'umap-browse-data') - // HOTFIX. Remove when this is merged and released: - // https://github.com/Leaflet/Leaflet/pull/9052 - L.DomEvent.disableClickPropagation(browserContainer) - - const title = L.DomUtil.add( - 'h3', - 'umap-browse-title', - browserContainer, - this.options.name - ) - - const filter = L.DomUtil.create('input', '', browserContainer) - let filterValue = '' - const bboxLabel = L.DomUtil.add('label', '', browserContainer, L._('Current map view')) - inBbox = L.DomUtil.create('input', '', bboxLabel) - inBbox.type = 'checkbox' - - const featuresContainer = L.DomUtil.create( - 'div', - 'umap-browse-features', - browserContainer - ) - - const filterKeys = this.getFilterKeys() - filter.type = 'text' - filter.placeholder = L._('Filter…') - filter.value = this.options.filter || '' - - const addFeature = (feature) => { - const feature_li = L.DomUtil.create('li', `${feature.getClassName()} feature`), - zoom_to = L.DomUtil.create('i', 'feature-zoom_to', feature_li), - edit = L.DomUtil.create('i', 'show-on-edit feature-edit', feature_li), - del = L.DomUtil.create('i', 'show-on-edit feature-delete', feature_li), - color = L.DomUtil.create('i', 'feature-color', feature_li), - title = L.DomUtil.create('span', 'feature-title', feature_li), - symbol = feature._getIconUrl - ? L.U.Icon.prototype.formatUrl(feature._getIconUrl(), feature) - : null - zoom_to.title = L._('Bring feature to center') - edit.title = L._('Edit this feature') - del.title = L._('Delete this feature') - title.textContent = feature.getDisplayName() || '—' - color.style.backgroundColor = feature.getOption('color') - if (symbol) { - color.style.backgroundImage = `url(${symbol})` - } - L.DomEvent.on( - zoom_to, - 'click', - function (e) { - e.callback = L.bind(this.view, this) - this.zoomTo(e) - }, - feature - ) - L.DomEvent.on( - title, - 'click', - function (e) { - e.callback = L.bind(this.view, this) - this.zoomTo(e) - }, - feature - ) - L.DomEvent.on(edit, 'click', feature.edit, feature) - L.DomEvent.on(del, 'click', feature.confirmDelete, feature) - return feature_li - } - - const append = (datalayer) => { - const container = L.DomUtil.create( - 'div', - datalayer.getHidableClass(), - featuresContainer - ), - headline = L.DomUtil.create('h5', '', container) - container.id = `browse_data_datalayer_${datalayer.umap_id}` - datalayer.renderToolbox(headline) - L.DomUtil.add('span', '', headline, datalayer.options.name) - const ul = L.DomUtil.create('ul', '', container) - L.DomUtil.classIf(container, 'off', !datalayer.isVisible()) - - const build = () => { - ul.innerHTML = '' - const bounds = this.getBounds() - datalayer.eachFeature((feature) => { - if (filterValue && !feature.matchFilter(filterValue, filterKeys)) return - if (inBbox.checked && !feature.isOnScreen(bounds)) return - ul.appendChild(addFeature(feature)) - }) - } - build() - L.DomEvent.on(inBbox, 'click', build) - L.DomEvent.on(inBbox, 'click', () => { - if (inBbox.checked) this.on('moveend', build) - else this.off('moveend', build) - }) - datalayer.on('datachanged', build) - datalayer.map.ui.once('panel:closed', () => { - datalayer.off('datachanged', build) - this.off('moveend', build) - }) - datalayer.map.ui.once('panel:ready', () => { - datalayer.map.ui.once('panel:ready', () => { - datalayer.off('datachanged', build) - }) - }) - } - - const appendAll = function () { - this.options.filter = filterValue = filter.value - featuresContainer.innerHTML = '' - this.eachBrowsableDataLayer((datalayer) => { - append(datalayer) - }) - } - const resetLayers = function () { - this.eachBrowsableDataLayer((datalayer) => { - datalayer.resetLayer(true) - }) - } - L.bind(appendAll, this)() - L.DomEvent.on(filter, 'input', appendAll, this) - L.DomEvent.on(filter, 'input', resetLayers, this) - - this.ui.openPanel({ - data: { html: browserContainer }, - actions: [this._aboutLink()], - }) - }, _openFacet: function () { const container = L.DomUtil.create('div', 'umap-facet-search'), diff --git a/umap/static/umap/js/umap.forms.js b/umap/static/umap/js/umap.forms.js index 37c7c85f..0d983e67 100644 --- a/umap/static/umap/js/umap.forms.js +++ b/umap/static/umap/js/umap.forms.js @@ -1154,7 +1154,7 @@ L.U.FormBuilder = L.FormBuilder.extend({ }, initialize: function (obj, fields, options) { - this.map = obj.getMap() + this.map = obj.map || obj.getMap() L.FormBuilder.prototype.initialize.call(this, obj, fields, options) this.on('finish', this.finish) }, diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index f363ec88..a6a77572 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -331,6 +331,7 @@ L.U.Map.include({ this._controls.permanentCredit = new L.U.PermanentCreditsControl(this) if (this.options.scrollWheelZoom) this.scrollWheelZoom.enable() else this.scrollWheelZoom.disable() + this.browser = new L.U.Browser(this) this.renderControls() }, @@ -992,7 +993,7 @@ L.U.Map.include({ openBrowser: function () { this.onceDatalayersLoaded(function () { - this._openBrowser() + this.browser.open() }) }, diff --git a/umap/static/umap/js/umap.layer.js b/umap/static/umap/js/umap.layer.js index 8bc110b4..7f0b03fc 100644 --- a/umap/static/umap/js/umap.layer.js +++ b/umap/static/umap/js/umap.layer.js @@ -518,7 +518,7 @@ L.U.DataLayer = L.Evented.extend({ showFeature: function (feature) { const filterKeys = this.map.getFilterKeys(), - filter = this.map.options.filter + filter = this.map.browser.options.filter if (filter && !feature.matchFilter(filter, filterKeys)) return if (!feature.matchFacets()) return this.layer.addLayer(feature) diff --git a/umap/static/umap/test/index.html b/umap/static/umap/test/index.html index 67a8aa59..620d0274 100644 --- a/umap/static/umap/test/index.html +++ b/umap/static/umap/test/index.html @@ -38,6 +38,7 @@ + diff --git a/umap/templates/umap/js.html b/umap/templates/umap/js.html index 5b321fc4..e2da8669 100644 --- a/umap/templates/umap/js.html +++ b/umap/templates/umap/js.html @@ -40,6 +40,7 @@ + {% endcompress %} diff --git a/umap/tests/integration/test_browser.py b/umap/tests/integration/test_browser.py new file mode 100644 index 00000000..bde3acd7 --- /dev/null +++ b/umap/tests/integration/test_browser.py @@ -0,0 +1,133 @@ +from time import sleep + +import pytest +from playwright.sync_api import expect + +from ..base import DataLayerFactory + +pytestmark = pytest.mark.django_db + + +DATALAYER_DATA = { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "properties": {"name": "one point in france"}, + "geometry": {"type": "Point", "coordinates": [3.339844, 46.920255]}, + }, + { + "type": "Feature", + "properties": {"name": "one polygon in greenland"}, + "geometry": { + "type": "Polygon", + "coordinates": [ + [ + [-41.3, 71.8], + [-43.5, 70.8], + [-39.3, 70.9], + [-37.7, 72.2], + [-41.3, 71.8], + ] + ], + }, + }, + { + "type": "Feature", + "properties": {"name": "one line in new zeland"}, + "geometry": { + "type": "LineString", + "coordinates": [ + [176.1, -38.6], + [172.9, -43.3], + [168.3, -45.2], + ], + }, + }, + ], + "_umap_options": { + "displayOnLoad": True, + "browsable": True, + "name": "Calque 1", + }, +} + + +@pytest.fixture +def bootstrap(map, live_server): + map.settings["properties"]["onLoadPanel"] = "databrowser" + map.save() + DataLayerFactory(map=map, data=DATALAYER_DATA) + + +def test_data_browser_should_be_open(live_server, page, bootstrap, map): + page.goto(f"{live_server.url}{map.get_absolute_url()}") + el = page.locator(".umap-browse-data") + expect(el).to_be_visible() + expect(page.get_by_text("one point in france")).to_be_visible() + expect(page.get_by_text("one line in new zeland")).to_be_visible() + expect(page.get_by_text("one polygon in greenland")).to_be_visible() + + +def test_data_browser_should_be_filterable(live_server, page, bootstrap, map): + page.goto(f"{live_server.url}{map.get_absolute_url()}") + markers = page.locator(".leaflet-marker-icon") + expect(markers).to_have_count(1) + el = page.locator("input[name='filter']") + expect(el).to_be_visible() + el.type("poly") + expect(page.get_by_text("one point in france")).to_be_hidden() + expect(page.get_by_text("one line in new zeland")).to_be_hidden() + expect(page.get_by_text("one polygon in greenland")).to_be_visible() + expect(markers).to_have_count(0) # Hidden by filter + + +def test_data_browser_can_show_only_visible_features(live_server, page, bootstrap, map): + # Zoom on France + page.goto(f"{live_server.url}{map.get_absolute_url()}#6/51.000/2.000") + el = page.get_by_text("Current map view") + expect(el).to_be_visible() + el.click() + expect(page.get_by_text("one point in france")).to_be_visible() + expect(page.get_by_text("one line in new zeland")).to_be_hidden() + expect(page.get_by_text("one polygon in greenland")).to_be_hidden() + + +def test_data_browser_can_mix_filter_and_bbox(live_server, page, bootstrap, map): + # Zoom on north west + page.goto(f"{live_server.url}{map.get_absolute_url()}#4/61.98/-2.68") + el = page.get_by_text("Current map view") + expect(el).to_be_visible() + el.click() + expect(page.get_by_text("one point in france")).to_be_visible() + expect(page.get_by_text("one polygon in greenland")).to_be_visible() + expect(page.get_by_text("one line in new zeland")).to_be_hidden() + el = page.locator("input[name='filter']") + expect(el).to_be_visible() + el.type("poly") + expect(page.get_by_text("one polygon in greenland")).to_be_visible() + expect(page.get_by_text("one point in france")).to_be_hidden() + expect(page.get_by_text("one line in new zeland")).to_be_hidden() + + +def test_data_browser_bbox_limit_should_be_dynamic(live_server, page, bootstrap, map): + # Zoom on Europe + page.goto(f"{live_server.url}{map.get_absolute_url()}#6/51.000/2.000") + el = page.get_by_text("Current map view") + expect(el).to_be_visible() + el.click() + expect(page.get_by_text("one point in france")).to_be_visible() + expect(page.get_by_text("one line in new zeland")).to_be_hidden() + expect(page.get_by_text("one polygon in greenland")).to_be_hidden() + unzoom = page.get_by_role("button", name="Zoom out") + expect(unzoom).to_be_visible() + # Unzoom until we see the Greenland + unzoom.click() + sleep(0.5) # Zooming is async + unzoom.click() + sleep(0.5) # Zooming is async + unzoom.click() + sleep(0.5) # Zooming is async + expect(page.get_by_text("one point in france")).to_be_visible() + expect(page.get_by_text("one polygon in greenland")).to_be_visible() + expect(page.get_by_text("one line in new zeland")).to_be_hidden()