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

@@ -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()],
+    })
+  },
+})

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'),

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)
   },

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()
     })
   },
 

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)

umap/static/umap/test/index.html

@@ -38,6 +38,7 @@
     <script src="../js/umap.tableeditor.js"></script>
     <script src="../js/umap.permissions.js"></script>
     <script src="../js/umap.datalayer.permissions.js"></script>
+    <script src="../js/umap.browser.js"></script>
     <script src="../js/umap.js"></script>
     <script src="../js/umap.ui.js"></script>
     <link rel="stylesheet" href="../vendors/leaflet/leaflet.css" />

umap/templates/umap/js.html

@@ -40,6 +40,7 @@
   <script src="{{ STATIC_URL }}umap/js/umap.controls.js"></script>
   <script src="{{ STATIC_URL }}umap/js/umap.slideshow.js"></script>
   <script src="{{ STATIC_URL }}umap/js/umap.tableeditor.js"></script>
+  <script src="{{ STATIC_URL }}umap/js/umap.browser.js"></script>
   <script src="{{ STATIC_URL }}umap/js/umap.js"></script>
   <script src="{{ STATIC_URL }}umap/js/umap.ui.js"></script>
 {% endcompress %}

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()