wip: fix bugs and add tests for table editor

umap/static/umap/css/panel.css

@@ -8,7 +8,6 @@
     z-index: var(--zindex-panels);
     background-color: var(--background-color);
     color: var(--text-color);
-    opacity: 0.98;
     cursor: initial;
     border-radius: 5px;
     border: 1px solid var(--color-lightGray);

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

@@ -36,15 +36,15 @@ export default class TableEditor extends WithTemplate {
     let filterItem
     if (this.map.facets.has(property)) {
       filterItem = {
-        label: translate('Remove filter for this property'),
+        label: translate('Remove filter for this column'),
         action: () => {
           this.map.facets.remove(property)
           this.map.browser.open('filters')
         },
       }
     } else {
       filterItem = {
-        label: translate('Add filter for this property'),
+        label: translate('Add filter for this column'),
         action: () => {
           this.map.facets.add(property)
           this.map.browser.open('filters')
@@ -55,11 +55,11 @@ export default class TableEditor extends WithTemplate {
       [event.clientX, event.clientY],
       [
         {
-          label: translate('Delete this property on all the features'),
+          label: translate('Delete this column'),
           action: () => this.deleteProperty(property),
         },
         {
-          label: translate('Rename this property on all the features'),
+          label: translate('Rename this column'),
           action: () => this.renameProperty(property),
         },
         filterItem,
@@ -104,9 +104,6 @@ export default class TableEditor extends WithTemplate {
   compileProperties() {
     this.resetProperties()
     if (this.properties.length === 0) this.properties = ['name']
-    // description is a forced textarea, don't edit it in a text input, or you lose cariage returns
-    if (this.properties.indexOf('description') !== -1)
-      this.properties.splice(this.properties.indexOf('description'), 1)
     this.properties.sort()
     this.field_properties = []
     for (let i = 0; i < this.properties.length; i++) {
@@ -122,10 +119,14 @@ export default class TableEditor extends WithTemplate {
   }
 
   validateName(name) {
-    if (name.includes('.') !== -1) {
+    if (name.includes('.')) {
       U.Alert.error(translate('Invalide property name: {name}', { name: name }))
       return false
     }
+    if (this.datalayer._propertiesIndex.includes(name)) {
+      U.Alert.error(translate('This name already exists: {name}', { name: name }))
+      return false
+    }
     return true
   }
 
@@ -164,7 +165,7 @@ export default class TableEditor extends WithTemplate {
       .then(({ prompt }) => {
         if (!prompt || !this.validateName(prompt)) return
         this.datalayer.indexProperty(prompt)
-        this.edit()
+        this.open()
       })
   }
 
@@ -203,10 +204,10 @@ export default class TableEditor extends WithTemplate {
   editCell(cell) {
     const property = cell.dataset.property
     const field = `properties.${property}`
-    const feature = this.datalayer.getFeatureById(
-      event.target.parentNode.dataset.feature
-    )
-    const builder = new U.FormBuilder(feature, [field], {
+    const tr = event.target.closest('tr')
+    const feature = this.datalayer.getFeatureById(tr.dataset.feature)
+    const handler = property === 'description' ? 'Textarea' : 'Input'
+    const builder = new U.FormBuilder(feature, [[field, { handler: handler }]], {
       id: `umap-feature-properties_${L.stamp(feature)}`,
       className: 'trow',
       callback: feature.resetTooltip,
@@ -226,8 +227,6 @@ export default class TableEditor extends WithTemplate {
       const current = this.getFocus()
       if (current) {
         this.editCell(current)
-        event.preventDefault()
-        event.stopPropagation()
       }
     }
   }
@@ -273,8 +272,10 @@ export default class TableEditor extends WithTemplate {
         this.datalayer.show()
         this.datalayer.fire('datachanged')
         this.renderBody()
-        this.map.browser.resetFilters()
-        this.map.browser.open('filters')
+        if (this.map.browser.isOpen()) {
+          this.map.browser.resetFilters()
+          this.map.browser.open('filters')
+        }
       })
   }
 }

umap/static/umap/js/modules/ui/contextmenu.js

@@ -20,7 +20,6 @@ export default class ContextMenu {
         `<li class="${item.className || ''}"><button tabindex="0" class="flat">${item.label}</button></li>`
       )
       li.addEventListener('click', () => {
-        this.close()
         item.action()
       })
       this.container.appendChild(li)

umap/static/umap/js/umap.forms.js

@@ -1158,7 +1158,7 @@ U.FormBuilder = L.FormBuilder.extend({
     }
   },
 
-  finish: function () {
-    this.map.editPanel.close()
+  finish: (event) => {
+    event.helper?.input?.blur()
   },
 })

umap/tests/integration/test_tableeditor.py

@@ -2,8 +2,68 @@
 import re
 from pathlib import Path
 
+from playwright.sync_api import expect
+
 from umap.models import DataLayer
 
+from ..base import DataLayerFactory
+
+DATALAYER_DATA = {
+    "type": "FeatureCollection",
+    "features": [
+        {
+            "type": "Feature",
+            "properties": {
+                "mytype": "even",
+                "name": "Point 2",
+                "mynumber": 10,
+                "myboolean": True,
+                "mydate": "2024/04/14 12:19:17",
+            },
+            "geometry": {"type": "Point", "coordinates": [0.065918, 48.385442]},
+            "id": "poin2",  # Must be exactly 5 chars long so the frontend will keep it
+        },
+        {
+            "type": "Feature",
+            "properties": {
+                "mytype": "odd",
+                "name": "Point 1",
+                "mynumber": 12,
+                "myboolean": False,
+                "mydate": "2024/03/13 12:20:20",
+            },
+            "geometry": {"type": "Point", "coordinates": [3.55957, 49.767074]},
+            "id": "poin1",
+        },
+        {
+            "type": "Feature",
+            "properties": {
+                "mytype": "even",
+                "name": "Point 4",
+                "mynumber": 10,
+                "myboolean": "true",
+                "mydate": "2024/08/18 13:14:15",
+            },
+            "geometry": {"type": "Point", "coordinates": [0.856934, 45.290347]},
+            "id": "poin4",
+        },
+        {
+            "type": "Feature",
+            "properties": {
+                "mytype": "odd",
+                "name": "Point 3",
+                "mynumber": 14,
+                "mydate": "2024-04-14T10:19:17.000Z",
+            },
+            "geometry": {"type": "Point", "coordinates": [4.372559, 47.945786]},
+            "id": "poin3",
+        },
+    ],
+    "_umap_options": {
+        "name": "Calque 2",
+    },
+}
+
 
 def test_table_editor(live_server, openmap, datalayer, page):
     page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit")
@@ -12,14 +72,91 @@ def test_table_editor(live_server, openmap, datalayer, page):
     page.get_by_text("Add a new property").click()
     page.locator("dialog").locator("input").fill("newprop")
     page.locator("dialog").get_by_role("button", name="OK").click()
+    page.locator("td").nth(2).dblclick()
     page.locator('input[name="newprop"]').fill("newvalue")
-    page.once("dialog", lambda dialog: dialog.accept())
-    page.hover(".umap-table-editor .tcell")
-    page.get_by_title("Delete this property on all").first.click()
+    page.keyboard.press("Enter")
+    page.locator("thead button[data-property=name]").click()
+    page.get_by_role("button", name="Delete this column").click()
     page.locator("dialog").get_by_role("button", name="OK").click()
     with page.expect_response(re.compile(r".*/datalayer/update/.*")):
         page.get_by_role("button", name="Save").click()
     saved = DataLayer.objects.last()
     data = json.loads(Path(saved.geojson.path).read_text())
     assert data["features"][0]["properties"]["newprop"] == "newvalue"
     assert "name" not in data["features"][0]["properties"]
+
+
+def test_cannot_add_existing_property_name(live_server, openmap, datalayer, page):
+    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit")
+    page.get_by_role("link", name="Manage layers").click()
+    page.locator(".panel").get_by_title("Edit properties in a table").click()
+    page.get_by_text("Add a new property").click()
+    page.locator("dialog").locator("input").fill("name")
+    page.get_by_role("button", name="OK").click()
+    expect(page.get_by_role("dialog")).to_contain_text("This name already exists: name")
+    expect(page.locator('table th button[data-property=name]')).to_have_count(1)
+
+
+def test_rename_property(live_server, openmap, datalayer, page):
+    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit")
+    page.get_by_role("link", name="Manage layers").click()
+    page.locator(".panel").get_by_title("Edit properties in a table").click()
+    expect(page.locator('table th button[data-property=name]')).to_have_count(1)
+    page.locator("thead button[data-property=name]").click()
+    page.get_by_text("Rename this column").click()
+    page.locator("dialog").locator("input").fill("newname")
+    page.get_by_role("button", name="OK").click()
+    expect(page.locator('table th button[data-property=newname]')).to_have_count(1)
+    expect(page.locator('table th button[data-property=name]')).to_have_count(0)
+
+
+def test_delete_selected_rows(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("link", name="Manage layers").click()
+    page.locator(".panel").get_by_title("Edit properties in a table").click()
+    expect(page.locator("tbody tr")).to_have_count(4)
+    expect(page.locator(".leaflet-marker-icon")).to_have_count(4)
+    page.locator("tr[data-feature=poin2]").get_by_role("checkbox").check()
+    page.get_by_role("button", name="Delete selected rows").click()
+    page.get_by_role("button", name="OK").click()
+    expect(page.locator("tbody tr")).to_have_count(3)
+    expect(page.locator(".leaflet-marker-icon")).to_have_count(3)
+
+
+def test_delete_all_rows(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("link", name="Manage layers").click()
+    page.locator(".panel").get_by_title("Edit properties in a table").click()
+    expect(page.locator("tbody tr")).to_have_count(4)
+    expect(page.locator(".leaflet-marker-icon")).to_have_count(4)
+    page.locator("thead").get_by_role("checkbox").check()
+    page.get_by_role("button", name="Delete selected rows").click()
+    page.get_by_role("button", name="OK").click()
+    expect(page.locator("tbody tr")).to_have_count(0)
+    expect(page.locator(".leaflet-marker-icon")).to_have_count(0)
+
+
+def test_filter_and_delete_rows(live_server, openmap, page):
+    DataLayerFactory(map=openmap, data=DATALAYER_DATA)
+    panel = page.locator(".panel.left.on")
+    table = page.locator(".panel.full table")
+    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit#6/48.093/1.890")
+    page.get_by_role("link", name="Manage layers").click()
+    page.locator(".panel").get_by_title("Edit properties in a table").click()
+    expect(table.locator("tbody tr")).to_have_count(4)
+    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 column").click()
+    expect(panel).to_be_visible()
+    panel.get_by_label("even").check()
+    table.locator("thead").get_by_role("checkbox").check()
+    page.get_by_role("button", name="Delete selected rows").click()
+    page.get_by_role("button", name="OK").click()
+    expect(table.locator("tbody tr")).to_have_count(2)
+    expect(page.locator(".leaflet-marker-icon")).to_have_count(2)
+    expect(table.get_by_text("Point 1")).to_be_visible()
+    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()