Merge pull request #1921 from umap-project/fix-layer-labelkey

fix: use layer labelKey if any when filtering features

umap/static/umap/js/modules/browser.js

@@ -143,8 +143,6 @@ export default class Browser {
   open(mode) {
     // Force only if mode is known, otherwise keep current mode.
     if (mode) this.mode = mode
-    // Get once but use it for each feature later
-    this.filterKeys = this.map.getFilterKeys()
     const container = DomUtil.create('div')
     // HOTFIX. Remove when this is released:
     // https://github.com/Leaflet/Leaflet/pull/9052

umap/static/umap/js/umap.features.js

@@ -144,7 +144,11 @@ U.FeatureMixin = {
   edit: function (e) {
     if (!this.map.editEnabled || this.isReadOnly()) return
     const container = L.DomUtil.create('div', 'umap-feature-container')
-    L.DomUtil.createTitle(container, L._('Feature properties'), `icon-${this.getClassName()}`)
+    L.DomUtil.createTitle(
+      container,
+      L._('Feature properties'),
+      `icon-${this.getClassName()}`
+    )
 
     let builder = new U.FormBuilder(
       this,
@@ -528,7 +532,7 @@ U.FeatureMixin = {
   },
 
   isFiltered: function () {
-    const filterKeys = this.map.getFilterKeys()
+    const filterKeys = this.datalayer.getFilterKeys()
     const filter = this.map.browser.options.filter
     if (filter && !this.matchFilter(filter, filterKeys)) return true
     if (!this.matchFacets()) return true
@@ -537,6 +541,10 @@ U.FeatureMixin = {
 
   matchFilter: function (filter, keys) {
     filter = filter.toLowerCase()
+    if (U.Utils.hasVar(keys)) {
+      return this.getDisplayName().toLowerCase().indexOf(filter) !== -1
+    }
+    keys = keys.split(',')
     for (let i = 0, value; i < keys.length; i++) {
       value = (this.properties[keys[i]] || '') + ''
       if (value.toLowerCase().indexOf(filter) !== -1) return true
@@ -598,7 +606,7 @@ U.FeatureMixin = {
     if (locale) properties.locale = locale
     if (L.lang) properties.lang = L.lang
     properties.rank = this.getRank() + 1
-    if (this.hasGeom()) {
+    if (this._map && this.hasGeom()) {
       center = this.getCenter()
       properties.lat = center.lat
       properties.lon = center.lng

umap/static/umap/js/umap.js

@@ -1872,10 +1872,6 @@ U.Map = L.Map.extend({
     if (this._controls.search) this._controls.search.open()
   },
 
-  getFilterKeys: function () {
-    return (this.options.filterKey || this.options.sortKey || 'name').split(',')
-  },
-
   getLayersBounds: function () {
     const bounds = new L.latLngBounds()
     this.eachBrowsableDataLayer((d) => {

umap/static/umap/js/umap.layer.js

@@ -1757,6 +1757,16 @@ U.DataLayer = L.Evented.extend({
     const editor = new U.TableEditor(this)
     editor.edit()
   },
+
+  getFilterKeys: function () {
+    // This keys will be used to filter feature from the browser text input.
+    // By default, it will we use the "name" property, which is also the one used as label in the features list.
+    // When map owner has configured another label or sort key, we try to be smart and search in the same keys.
+    if (this.map.options.filterKey) return this.map.options.filterKey
+    else if (this.options.labelKey) return this.options.labelKey
+    else if (this.map.options.sortKey) return this.map.options.sortKey
+    else return 'name'
+  },
 })
 
 L.TileLayer.include({

umap/tests/integration/test_browser.py

@@ -15,12 +15,16 @@ DATALAYER_DATA = {
     "features": [
         {
             "type": "Feature",
-            "properties": {"name": "one point in france", "foo": "point"},
+            "properties": {"name": "one point in france", "foo": "point", "bar": "one"},
             "geometry": {"type": "Point", "coordinates": [3.339844, 46.920255]},
         },
         {
             "type": "Feature",
-            "properties": {"name": "one polygon in greenland", "foo": "polygon"},
+            "properties": {
+                "name": "one polygon in greenland",
+                "foo": "polygon",
+                "bar": "two",
+            },
             "geometry": {
                 "type": "Polygon",
                 "coordinates": [
@@ -36,7 +40,11 @@ DATALAYER_DATA = {
         },
         {
             "type": "Feature",
-            "properties": {"name": "one line in new zeland", "foo": "line"},
+            "properties": {
+                "name": "one line in new zeland",
+                "foo": "line",
+                "bar": "three",
+            },
             "geometry": {
                 "type": "LineString",
                 "coordinates": [
@@ -103,6 +111,71 @@ def test_data_browser_should_be_filterable(live_server, page, bootstrap, map):
     expect(paths).to_have_count(0)
 
 
+def test_filter_uses_layer_setting_if_any(live_server, page, bootstrap, map):
+    datalayer = map.datalayer_set.first()
+    datalayer.settings["labelKey"] = "foo"
+    datalayer.save()
+    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    expect(page.get_by_title("Features in this layer: 3")).to_be_visible()
+    markers = page.locator(".leaflet-marker-icon")
+    paths = page.locator(".leaflet-overlay-pane path")
+    expect(markers).to_have_count(1)
+    expect(paths).to_have_count(2)
+    expect(page.get_by_text("point")).to_be_visible()
+    expect(page.get_by_text("polygon")).to_be_visible()
+    expect(page.get_by_text("line")).to_be_visible()
+    page.locator(".filters summary").click()
+    filter_ = page.locator("input[name='filter']")
+    expect(filter_).to_be_visible()
+    filter_.type("po")
+    expect(page.get_by_title("Features in this layer: 2/3")).to_be_visible()
+    expect(page.get_by_title("Features in this layer: 2/3")).to_have_text("(2/3)")
+    expect(page.get_by_text("line")).to_be_hidden()
+    expect(page.get_by_text("point")).to_be_visible()
+    expect(page.get_by_text("polygon")).to_be_visible()
+    expect(markers).to_have_count(1)
+    expect(paths).to_have_count(1)  # Only polygon
+    # Empty the filter
+    filter_.fill("")
+    filter_.blur()
+    expect(markers).to_have_count(1)
+    expect(paths).to_have_count(2)
+    filter_.type("point")
+    expect(page.get_by_text("point")).to_be_visible()
+    expect(page.get_by_text("line")).to_be_hidden()
+    expect(page.get_by_text("polygon")).to_be_hidden()
+    expect(markers).to_have_count(1)
+    expect(paths).to_have_count(0)
+
+
+def test_filter_works_with_variable_in_labelKey(live_server, page, map):
+    map.settings["properties"]["onLoadPanel"] = "databrowser"
+    map.save()
+    data = deepcopy(DATALAYER_DATA)
+    data["_umap_options"]["labelKey"] = "{name} ({bar})"
+    DataLayerFactory(map=map, data=data)
+    page.goto(f"{live_server.url}{map.get_absolute_url()}")
+    expect(page.get_by_title("Features in this layer: 3")).to_be_visible()
+    markers = page.locator(".leaflet-marker-icon")
+    paths = page.locator(".leaflet-overlay-pane path")
+    expect(markers).to_have_count(1)
+    expect(paths).to_have_count(2)
+    expect(page.get_by_text("one point in france (one)")).to_be_visible()
+    expect(page.get_by_text("one line in new zeland (three)")).to_be_visible()
+    expect(page.get_by_text("one polygon in greenland (two)")).to_be_visible()
+    page.locator(".filters summary").click()
+    filter_ = page.locator("input[name='filter']")
+    expect(filter_).to_be_visible()
+    filter_.type("two")
+    expect(page.get_by_title("Features in this layer: 1/3")).to_be_visible()
+    expect(page.get_by_title("Features in this layer: 1/3")).to_have_text("(1/3)")
+    expect(page.get_by_text("one polygon in greenland (two)")).to_be_visible()
+    expect(page.get_by_text("one line in new zeland (three)")).to_be_hidden()
+    expect(page.get_by_text("one point in france (one)")).to_be_hidden()
+    expect(markers).to_have_count(0)
+    expect(paths).to_have_count(1)  # Only polygon
+
+
 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")