Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use layer labelKey if any when filtering features #1921

Merged
merged 1 commit into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions umap/static/umap/js/modules/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 11 additions & 3 deletions umap/static/umap/js/umap.features.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions umap/static/umap/js/umap.js
Original file line number Diff line number Diff line change
Expand Up @@ -1873,10 +1873,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) => {
Expand Down
10 changes: 10 additions & 0 deletions umap/static/umap/js/umap.layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
79 changes: 76 additions & 3 deletions umap/tests/integration/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@
"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": [
Expand All @@ -36,7 +40,11 @@
},
{
"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": [
Expand Down Expand Up @@ -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")
Expand Down
Loading