wip: refactor isolateShape

umap/static/umap/js/modules/data/features.js

@@ -729,20 +729,11 @@ class Path extends Feature {
     if (this.isEmpty()) this.del()
   }
 
-  isolateShape(at) {
+  isolateShape(latlngs) {
-    if (!this.isMulti()) return
-    const shape = this.ui.enableEdit().deleteShapeAt(at)
-    this.ui.disableEdit()
-    if (!shape) return
     const properties = this.cloneProperties()
-    const other = new (this instanceof LineString ? LineString : Polygon)(
+    const type = this instanceof LineString ? 'LineString' : 'Polygon'
-      this.datalayer,
+    const geometry = this._toGeometry(latlngs)
-      {
+    const other = this.datalayer.makeFeature({ type, geometry, properties })
-        properties,
-        geometry: this._toGeometry(shape),
-      }
-    )
-    this.datalayer.addFeature(other)
     other.edit()
     return other
   }
@@ -912,10 +903,11 @@ export class Polygon extends Path {
 
   _toGeometry(latlngs) {
     const holes = !LineUtil.isFlat(latlngs)
-    const multi = holes && !LineUtil.isFlat(latlngs[0])
+    let multi = holes && !LineUtil.isFlat(latlngs[0])
     let coordinates = GeoJSON.latLngsToCoords(latlngs, multi ? 2 : holes ? 1 : 0, true)
-    if (!holes) {
+    if (Utils.polygonMustBeFlattened(coordinates)) {
-      coordinates = [coordinates]
+      coordinates = coordinates[0]
+      multi = false
     }
     const type = multi ? 'MultiPolygon' : 'Polygon'
     return { coordinates, type }

umap/static/umap/js/modules/data/layer.js

@@ -435,12 +435,13 @@ export class DataLayer {
   }
 
   makeFeature(geojson = {}, sync = true, id = null) {
+    // Both Feature and Geometry are valid geojson objects.
     const geometry = geojson.geometry || geojson
     let feature
 
     switch (geometry.type) {
       case 'Point':
-        // FIXME: deal with MutliPoint
+        // FIXME: deal with MultiPoint
         feature = new Point(this, geojson, id)
         break
       case 'MultiLineString':

umap/static/umap/js/modules/rendering/ui.js

@@ -378,12 +378,21 @@ const PathMixin = {
       items.push({
         text: translate('Extract shape to separate feature'),
         callback: () => {
-          this.feature.isolateShape(event.latlng, this._map.editedFeature)
+          this.isolateShape(event.latlng)
         },
       })
     }
     return items
   },
+
+  isolateShape: function(atLatLng) {
+    if (!this.feature.isMulti()) return
+    const shape = this.enableEdit().deleteShapeAt(atLatLng)
+    this.geometryChanged()
+    this.disableEdit()
+    if (!shape) return
+    return this.feature.isolateShape(shape)
+  }
 }
 
 export const LeafletPolyline = Polyline.extend({

umap/static/umap/js/modules/utils.js

@@ -302,6 +302,10 @@ export function flattenCoordinates(coords) {
   return coords
 }
 
+export function polygonMustBeFlattened(coords) {
+  return coords.length === 1 && typeof coords?.[0]?.[0]?.[0] !== 'number'
+}
+
 export function buildQueryString(params) {
   const query_string = []
   for (const key in params) {

umap/static/umap/js/umap.controls.js

@@ -244,7 +244,7 @@ U.ExtractShapeFromMultiAction = U.BaseFeatureAction.extend({
   },
 
   onClick: function (e) {
-    this.feature.isolateShape(e.latlng)
+    this.feature.ui.isolateShape(e.latlng)
   },
 })
 

umap/static/umap/unittests/utils.js

@@ -479,6 +479,113 @@ describe('Utils', () => {
     })
   })
 
+  describe('#polygonMustBeFlattened', () => {
+    it('should return false for simple polygon', () => {
+      const coords = [
+        [
+          [100.0, 0.0],
+          [101.0, 0.0],
+          [101.0, 1.0],
+          [100.0, 1.0],
+          [100.0, 0.0],
+        ],
+      ]
+      assert.notOk(Utils.polygonMustBeFlattened(coords))
+    })
+
+    it('should return false for simple polygon with hole', () => {
+      const coords = [
+        [
+          [100.0, 0.0],
+          [101.0, 0.0],
+          [101.0, 1.0],
+          [100.0, 1.0],
+          [100.0, 0.0],
+        ],
+        [
+          [100.8, 0.8],
+          [100.8, 0.2],
+          [100.2, 0.2],
+          [100.2, 0.8],
+          [100.8, 0.8],
+        ],
+      ]
+      assert.notOk(Utils.polygonMustBeFlattened(coords))
+    })
+
+    it('should return false for multipolygon', () => {
+      const coords = [
+        [
+          [
+            [102.0, 2.0],
+            [103.0, 2.0],
+            [103.0, 3.0],
+            [102.0, 3.0],
+            [102.0, 2.0],
+          ],
+        ],
+        [
+          [
+            [100.0, 0.0],
+            [101.0, 0.0],
+            [101.0, 1.0],
+            [100.0, 1.0],
+            [100.0, 0.0],
+          ],
+          [
+            [100.2, 0.2],
+            [100.2, 0.8],
+            [100.8, 0.8],
+            [100.8, 0.2],
+            [100.2, 0.2],
+          ],
+        ],
+      ]
+      assert.notOk(Utils.polygonMustBeFlattened(coords))
+    })
+
+    it('should return true for false multi polygon', () => {
+      const coords = [
+        [
+          [
+            [100.0, 0.0],
+            [101.0, 0.0],
+            [101.0, 1.0],
+            [100.0, 1.0],
+            [100.0, 0.0],
+          ],
+        ],
+      ]
+      assert.ok(Utils.polygonMustBeFlattened(coords))
+    })
+
+    it('should return true for false multi polygon with hole', () => {
+      const coords = [
+        [
+          [
+            [100.0, 0.0],
+            [101.0, 0.0],
+            [101.0, 1.0],
+            [100.0, 1.0],
+            [100.0, 0.0],
+          ],
+          [
+            [100.8, 0.8],
+            [100.8, 0.2],
+            [100.2, 0.2],
+            [100.2, 0.8],
+            [100.8, 0.8],
+          ],
+        ],
+      ]
+      assert.ok(Utils.polygonMustBeFlattened(coords))
+    })
+
+    it('should return false for empty coords', () => {
+      assert.notOk(Utils.polygonMustBeFlattened([]))
+    })
+  })
+
   describe('#usableOption()', () => {
     it('should consider false', () => {
       assert.ok(Utils.usableOption({ key: false }, 'key'))

umap/tests/integration/test_draw_polygon.py

@@ -243,7 +243,8 @@ def test_can_transfer_shape_from_simple_polygon(live_server, page, tilelayer):
     expect(polygons).to_have_count(1)
 
 
-def test_can_extract_shape(live_server, page, tilelayer):
+def test_can_extract_shape(live_server, page, tilelayer, settings):
+    settings.UMAP_ALLOW_ANONYMOUS = True
     page.goto(f"{live_server.url}/en/map/new/")
     polygons = page.locator(".leaflet-overlay-pane path")
     expect(polygons).to_have_count(0)
@@ -269,6 +270,59 @@ def test_can_extract_shape(live_server, page, tilelayer):
     polygons.first.click(position={"x": 20, "y": 20}, button="right")
     extract_button.click()
     expect(polygons).to_have_count(2)
+    data = save_and_get_json(page)
+    print(data)
+    assert len(data["features"]) == 2
+    assert data["features"][0]["geometry"]["type"] == "Polygon"
+    assert data["features"][1]["geometry"]["type"] == "Polygon"
+    assert data["features"][0]["geometry"]["coordinates"] == [
+        [
+            [
+                -6.569824,
+                53.159947,
+            ],
+            [
+                -6.569824,
+                52.49616,
+            ],
+            [
+                -7.668457,
+                52.49616,
+            ],
+            [
+                -7.668457,
+                53.159947,
+            ],
+            [
+                -6.569824,
+                53.159947,
+            ],
+        ],
+    ]
+    assert data["features"][1]["geometry"]["coordinates"] == [
+        [
+            [
+                -8.76709,
+                54.457267,
+            ],
+            [
+                -8.76709,
+                53.813626,
+            ],
+            [
+                -9.865723,
+                53.813626,
+            ],
+            [
+                -9.865723,
+                54.457267,
+            ],
+            [
+                -8.76709,
+                54.457267,
+            ],
+        ],
+    ]
 
 
 def test_cannot_transfer_shape_to_line(live_server, page, tilelayer):