From 5909630e0e1a9c60b512686fc477814aeca36249 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Mon, 8 Jul 2024 17:56:31 +0200 Subject: [PATCH] wip: fix bugs and add tests for table editor --- umap/static/umap/css/panel.css | 1 - umap/static/umap/js/modules/tableeditor.js | 35 ++--- umap/static/umap/js/modules/ui/contextmenu.js | 1 - umap/static/umap/js/umap.forms.js | 4 +- umap/tests/integration/test_tableeditor.py | 143 +++++++++++++++++- 5 files changed, 160 insertions(+), 24 deletions(-) diff --git a/umap/static/umap/css/panel.css b/umap/static/umap/css/panel.css index bd6d0687..5f35938d 100644 --- a/umap/static/umap/css/panel.css +++ b/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); diff --git a/umap/static/umap/js/modules/tableeditor.js b/umap/static/umap/js/modules/tableeditor.js index 90212c84..8304482e 100644 --- a/umap/static/umap/js/modules/tableeditor.js +++ b/umap/static/umap/js/modules/tableeditor.js @@ -36,7 +36,7 @@ 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') @@ -44,7 +44,7 @@ export default class TableEditor extends WithTemplate { } } 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') + } }) } } diff --git a/umap/static/umap/js/modules/ui/contextmenu.js b/umap/static/umap/js/modules/ui/contextmenu.js index d47a0249..4274fa17 100644 --- a/umap/static/umap/js/modules/ui/contextmenu.js +++ b/umap/static/umap/js/modules/ui/contextmenu.js @@ -20,7 +20,6 @@ export default class ContextMenu { `
    • ` ) li.addEventListener('click', () => { - this.close() item.action() }) this.container.appendChild(li) diff --git a/umap/static/umap/js/umap.forms.js b/umap/static/umap/js/umap.forms.js index b1a08d9c..a7705ae0 100644 --- a/umap/static/umap/js/umap.forms.js +++ b/umap/static/umap/js/umap.forms.js @@ -1159,7 +1159,7 @@ U.FormBuilder = L.FormBuilder.extend({ } }, - finish: function () { - this.map.editPanel.close() + finish: (event) => { + event.helper?.input?.blur() }, }) diff --git a/umap/tests/integration/test_tableeditor.py b/umap/tests/integration/test_tableeditor.py index cbedf3aa..4e0792e4 100644 --- a/umap/tests/integration/test_tableeditor.py +++ b/umap/tests/integration/test_tableeditor.py @@ -2,8 +2,68 @@ import json 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,10 +72,11 @@ 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() @@ -23,3 +84,79 @@ def test_table_editor(live_server, openmap, datalayer, page): 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()