From da2fb856c25a35b7d1804cc02fea5fb956fa3a0a Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Fri, 22 Dec 2023 13:01:06 +0100 Subject: [PATCH] Better way of handling escape while drawing With previous fix (in 60c002f94a80ef03a91edcd5c8d6019a59a07cd0), the feature was created even if invalid. cf #1428 --- umap/static/umap/js/umap.controls.js | 11 ++- umap/static/umap/js/umap.js | 4 +- umap/tests/integration/test_drawing.py | 106 +++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 5 deletions(-) diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index 157f32bf..ab25569b 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -1210,7 +1210,6 @@ L.U.TileLayerControl = L.Control.IconLayers.extend({ L.Control.IconLayers.prototype.setLayers.call(this, layers.slice(0, maxShown)) if (this.map.selected_tilelayer) this.setActiveLayer(this.map.selected_tilelayer) }, - }) /* Used in edit mode to define the default tilelayer */ @@ -1582,11 +1581,15 @@ L.U.Editable = L.Editable.extend({ 'editable:drawing:start editable:drawing:click editable:drawing:move', this.drawingTooltip ) - this.on('editable:drawing:end', this.closeTooltip) + this.on('editable:drawing:end', (e) => { + this.closeTooltip() + // Leaflet.Editable will delete the drawn shape if invalid + // (eg. line has only one drawn point) + // So let's check if the layer has no more shape + if (!e.layer.hasGeom()) e.layer.del() + }) // Layer for items added by users this.on('editable:drawing:cancel', (e) => { - if (e.layer._latlngs && e.layer._latlngs.length < e.layer.editor.MIN_VERTEX) - e.layer.del() if (e.layer instanceof L.U.Marker) e.layer.del() }) this.on('editable:drawing:commit', function (e) { diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index c814854e..add161cf 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -577,7 +577,9 @@ L.U.Map.include({ this.help.show('edit') } if (e.keyCode === L.U.Keys.ESC) { - if (this.editEnabled) this.editTools.commitDrawing() + if (this.editEnabled && this.editTools.drawing()) { + this.editTools.stopDrawing() + } if (this.measureTools.enabled()) this.measureTools.stopDrawing() } } diff --git a/umap/tests/integration/test_drawing.py b/umap/tests/integration/test_drawing.py index 67c6618a..27a11036 100644 --- a/umap/tests/integration/test_drawing.py +++ b/umap/tests/integration/test_drawing.py @@ -69,6 +69,58 @@ def test_clicking_esc_should_finish_line(page, live_server, tilelayer): expect(guide).to_have_count(0) +def test_clicking_esc_should_delete_line_if_empty(page, live_server, tilelayer): + page.goto(f"{live_server.url}/en/map/new/") + + # Click on the Draw a line button on a new map. + create_line = page.locator(".leaflet-control-toolbar ").get_by_title( + "Draw a polyline (Ctrl+L)" + ) + create_line.click() + + # Check no line is present by default. + # We target with the color, because there is also the drawing line guide (dash-array) + # around + lines = page.locator(".leaflet-overlay-pane path[stroke='DarkBlue']") + guide = page.locator(".leaflet-overlay-pane > svg > g > path") + expect(lines).to_have_count(0) + expect(guide).to_have_count(0) + + map = page.locator("#map") + map.click(position={"x": 200, "y": 200}) + # At this stage, the line as one element, it should not be created + # on pressing esc, as invalid + # Click ESC to finish + page.keyboard.press("Escape") + expect(lines).to_have_count(0) + expect(guide).to_have_count(0) + + +def test_clicking_esc_should_delete_line_if_invalid(page, live_server, tilelayer): + page.goto(f"{live_server.url}/en/map/new/") + + # Click on the Draw a line button on a new map. + create_line = page.locator(".leaflet-control-toolbar ").get_by_title( + "Draw a polyline (Ctrl+L)" + ) + create_line.click() + + # Check no line is present by default. + # We target with the color, because there is also the drawing line guide (dash-array) + # around + lines = page.locator(".leaflet-overlay-pane path[stroke='DarkBlue']") + guide = page.locator(".leaflet-overlay-pane > svg > g > path") + expect(lines).to_have_count(0) + expect(guide).to_have_count(0) + + # At this stage, the line as no element, it should not be created + # on pressing esc + # Click ESC to finish + page.keyboard.press("Escape") + expect(lines).to_have_count(0) + expect(guide).to_have_count(0) + + def test_draw_polygon(page, live_server, tilelayer): page.goto(f"{live_server.url}/en/map/new/") @@ -135,3 +187,57 @@ def test_clicking_esc_should_finish_polygon(page, live_server, tilelayer): page.keyboard.press("Escape") expect(lines).to_have_count(1) expect(guide).to_have_count(0) + + +def test_clicking_esc_should_delete_polygon_if_empty(page, live_server, tilelayer): + page.goto(f"{live_server.url}/en/map/new/") + + # Click on the Draw a polygon button on a new map. + create_line = page.locator(".leaflet-control-toolbar ").get_by_title( + "Draw a polygon (Ctrl+P)" + ) + create_line.click() + + # Check no polygon is present by default. + # We target with the color, because there is also the drawing line guide (dash-array) + # around + lines = page.locator(".leaflet-overlay-pane path[stroke='DarkBlue']") + guide = page.locator(".leaflet-overlay-pane > svg > g > path") + expect(lines).to_have_count(0) + expect(guide).to_have_count(0) + + # Click ESC to finish, no polygon should have been created + page.keyboard.press("Escape") + expect(lines).to_have_count(0) + expect(guide).to_have_count(0) + + +def test_clicking_esc_should_delete_polygon_if_invalid(page, live_server, tilelayer): + page.goto(f"{live_server.url}/en/map/new/") + + # Click on the Draw a polygon button on a new map. + create_line = page.locator(".leaflet-control-toolbar ").get_by_title( + "Draw a polygon (Ctrl+P)" + ) + create_line.click() + + # Check no polygon is present by default. + # We target with the color, because there is also the drawing line guide (dash-array) + # around + lines = page.locator(".leaflet-overlay-pane path[stroke='DarkBlue']") + guide = page.locator(".leaflet-overlay-pane > svg > g > path") + expect(lines).to_have_count(0) + expect(guide).to_have_count(0) + + # Click on the map twice, it will start a polygon. + map = page.locator("#map") + map.click(position={"x": 200, "y": 200}) + expect(lines).to_have_count(1) + expect(guide).to_have_count(1) + map.click(position={"x": 100, "y": 200}) + expect(lines).to_have_count(1) + expect(guide).to_have_count(2) + # Click ESC to finish, the polygon is invalid, it should not be persisted + page.keyboard.press("Escape") + expect(lines).to_have_count(0) + expect(guide).to_have_count(0)