Merge pull request #1483 from umap-project/escape-drawing

Better way of handling escape while drawing

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) {

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()
       }
     }

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)