wip: edit field and filter on the right parent from tableeditor

Co-authored-by: David Larlet <[email protected]>

Author: yohanboniface

umap/static/umap/js/modules/filters.js

@@ -291,7 +291,12 @@ export default class Filters {
   }
 
   filterForm(name) {
-    const properties = { name, ...(this.defined.get(name) || {}) }
+    let widget = WIDGETS[0]
+    const field = this.#parent.fields.get(name)
+    if (['Number', 'Date', 'Datetime'].includes(field?.type)) {
+      widget = 'minmax'
+    }
+    const properties = { name, widget, ...(this.defined.get(name) || {}) }
     const fieldKeys = name
       ? [name]
       : ['', ...this.#parent.fieldKeys.filter((key) => !this.defined.has(key))]

umap/static/umap/js/modules/tableeditor.js

@@ -31,53 +31,72 @@ export default class TableEditor extends WithTemplate {
     this.elements.body.addEventListener('keydown', (event) => this.onKeyDown(event))
     this.elements.header.addEventListener('click', (event) => {
       const property = event.target.dataset.property
-      if (property) this.openHeaderMenu(property)
+      const parentType = event.target.dataset.fieldParent
+      if (property)
+        this.openHeaderMenu(
+          property,
+          parentType === 'map' ? this._umap : this.datalayer
+        )
     })
   }
 
-  openHeaderMenu(property) {
+  openHeaderMenu(name, parent) {
     const actions = []
-    let filterItem
-    if (this.datalayer.filters.has(property)) {
-      filterItem = {
-        label: translate('Remove filter for this field'),
-        action: () => {
-          this.datalayer.filters.remove(property)
-          this._umap.browser.open('filters')
-        },
-      }
+    let actionLabel
+    if (parent.filters.has(name)) {
+      actionLabel = translate('Edit filter for this field')
     } else {
-      filterItem = {
-        label: translate('Add filter for this field'),
-        action: () => {
-          this.datalayer.filters.add({ name: property })
-          this._umap.browser.open('filters')
-        },
-      }
+      actionLabel = translate('Add filter for this field')
     }
-    actions.push(filterItem)
-    if (!this.datalayer.isRemoteLayer()) {
+    actions.push({
+      label: actionLabel,
+      action: () => {
+        parent.filters.filterForm(name)
+        this._umap.browser.open('filters')
+      },
+    })
+    // Only allow editing fields for map and local datalayer.
+    if (!parent.isRemoteLayer?.()) {
       actions.push({
         label: translate('Edit this field'),
-        action: () => this.editField(property),
+        action: () => {
+          parent.fields.editField(name).then(() => this.open())
+        },
       })
       actions.push({
         label: translate('Delete this field'),
-        action: () => this.deleteField(property),
+        action: () => {
+          parent.fields.confirmDelete(name).then(() => this.open())
+        },
       })
     }
     this.contextmenu.open(event, actions)
   }
 
+  get fields() {
+    return [
+      ...this.datalayer.fields.all().map((field) => {
+        const copy = { ...field }
+        copy.parent = 'datalayer'
+        return copy
+      }),
+      ...this._umap.fields.all().map((field) => {
+        const copy = { ...field }
+        copy.parent = 'map'
+        return copy
+      }),
+    ]
+  }
+
   renderHeaders() {
     this.elements.header.innerHTML = ''
     const th = loadTemplate('<th><input type="checkbox" /></th>')
     const checkbox = th.firstChild
     this.elements.header.appendChild(th)
-    for (const field of this.datalayer.fields.all()) {
+    for (const field of this.fields) {
       this.elements.header.appendChild(
         loadTemplate(
-          `<th>${field.key}<button data-property="${field.key}" class="flat" aria-label="${translate('Advanced actions')}">…</button></th>`
+          `<th>${field.key}<button data-property="${field.key}" data-field-parent="${field.parent}" class="flat" aria-label="${translate('Advanced actions')}">…</button></th>`
         )
       )
     }
@@ -94,25 +113,15 @@ export default class TableEditor extends WithTemplate {
     this.datalayer.features.forEach((feature) => {
       if (feature.isFiltered()) return
       if (inBbox && !feature.isOnScreen(bounds)) return
-      const tds = this.datalayer.fields
-        .all()
-        .map(
-          (field) =>
-            `<td tabindex="0" data-property="${field.key}">${feature.properties[field.key] ?? ''}</td>`
-        )
+      const tds = this.fields.map(
+        (field) =>
+          `<td tabindex="0" data-property="${field.key}">${feature.properties[field.key] ?? ''}</td>`
+      )
       html += `<tr data-feature="${feature.id}"><th><input type="checkbox" /></th>${tds.join('')}</tr>`
     })
     this.elements.body.innerHTML = html
   }
 
-  editField(name) {
-    this.datalayer.fields.editField(name).then(() => this.open())
-  }
-
-  deleteField(name) {
-    this.datalayer.fields.confirmDelete(name).then(() => this.open())
-  }
-
   addField() {
     this.datalayer.fields.editField().then(() => this.open())
   }

umap/tests/integration/test_tableeditor.py

@@ -4,7 +4,7 @@
 
 from playwright.sync_api import expect
 
-from umap.models import DataLayer
+from umap.models import DataLayer, Map
 
 from ..base import DataLayerFactory
 
@@ -116,7 +116,7 @@ def test_cannot_add_property_with_a_dot(live_server, openmap, datalayer, page):
     expect(page.locator("table th button[data-property=name]")).to_have_count(1)
 
 
-def test_rename_property(live_server, openmap, page):
+def test_rename_field(live_server, openmap, page):
     DataLayerFactory(map=openmap, data=DATALAYER_DATA)
     page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit#6/48.093/1.890")
     page.get_by_role("button", name="Manage layers").click()
@@ -142,7 +142,7 @@ def test_rename_property(live_server, openmap, page):
     expect(page.locator(".panel.right .umap-field-mytype")).to_be_visible()
 
 
-def test_delete_property(live_server, openmap, page):
+def test_delete_field(live_server, openmap, page):
     DataLayerFactory(map=openmap, data=DATALAYER_DATA)
     page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit#6/48.093/1.890")
     page.get_by_role("button", name="Manage layers").click()
@@ -203,6 +203,7 @@ def test_filter_and_delete_rows(live_server, openmap, page):
     expect(page.locator(".leaflet-marker-icon")).to_have_count(4)
     table.locator("thead button[data-property=mytype]").click()
     page.get_by_role("button", name="Add filter for this field").click()
+    page.get_by_role("button", name="OK").click()
     expect(panel).to_be_visible()
     panel.get_by_label("even").check()
     table.locator("thead").get_by_role("checkbox").check()
@@ -214,3 +215,26 @@ def test_filter_and_delete_rows(live_server, openmap, page):
     expect(table.get_by_text("Point 3")).to_be_visible()
     expect(table.get_by_text("Point 2")).to_be_hidden()
     expect(table.get_by_text("Point 4")).to_be_hidden()
+
+
+def test_add_filter_on_map_field(live_server, openmap, page):
+    openmap.settings["properties"]["fields"] = [{"key": "mynumber", "type": "Number"}]
+    openmap.save()
+    table = page.locator(".panel.full table")
+    DataLayerFactory(map=openmap, data=DATALAYER_DATA)
+    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit#6/48.093/1.890")
+    page.get_by_role("button", name="Manage layers").click()
+    page.locator(".panel").get_by_title("Edit properties in a table").click()
+    table.locator("thead button[data-property=mynumber]").click()
+    page.get_by_role("button", name="Add filter for this field").click()
+    expect(page.locator("dialog").get_by_label("minmax", exact=True)).to_be_checked()
+    page.locator("dialog").get_by_label("human readable name").fill("My Fun Filter")
+    page.wait_for_timeout(300)  # Throttling…
+    page.get_by_role("button", name="OK").click()
+    expect(page.locator(".panel.left.on").get_by_text("My Fun Filter")).to_be_visible()
+    with page.expect_response(re.compile("./update/settings/.*")):
+        page.get_by_role("button", name="Save").click()
+    saved = Map.objects.first()
+    assert saved.settings["properties"]["filters"] == {
+        "mynumber": {"widget": "minmax", "label": "My Fun Filter"}
+    }