fix: allow to draw new proprotional circles and to drag them (#2172)

The main issue was because we overrided the `getLatLngs` function in our
PointMixin class, so I changed the way feature get the latlngs from the
UI classes. Plus getting CircleMarker to inherit more from Marker,
through PointMixin.

fix #2171

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

@@ -103,11 +103,11 @@ class Feature {
   }
 
   pushGeometry() {
-    this.ui.setLatLngs(this.toLatLngs())
+    this._setLatLngs(this.toLatLngs())
   }
 
   pullGeometry(sync = true) {
-    this.fromLatLngs(this.ui.getLatLngs())
+    this.fromLatLngs(this._getLatLngs())
     if (sync) {
       this.sync.update('geometry', this.geometry)
     }
@@ -656,6 +656,14 @@ export class Point extends Feature {
     }
   }
 
+  _getLatLngs() {
+    return this.ui.getLatLng()
+  }
+
+  _setLatLngs(latlng) {
+    this.ui.setLatLng(latlng)
+  }
+
   toLatLngs() {
     return GeoJSON.coordsToLatLng(this.coordinates)
   }
@@ -727,6 +735,14 @@ class Path extends Feature {
     return !this.isEmpty()
   }
 
+  _getLatLngs() {
+    return this.ui.getLatLngs()
+  }
+
+  _setLatLngs(latlngs) {
+    this.ui.setLatLngs(latlngs)
+  }
+
   connectToDataLayer(datalayer) {
     super.connectToDataLayer(datalayer)
     // We keep markers on their own layer on top of the paths.

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

@@ -106,27 +106,6 @@ const PointMixin = {
   isOnScreen: function (bounds) {
     return bounds.contains(this.getCenter())
   },
-}
-
-export const LeafletMarker = Marker.extend({
-  parentClass: Marker,
-  includes: [FeatureMixin, PointMixin],
-
-  initialize: function (feature, latlng) {
-    FeatureMixin.initialize.call(this, feature, latlng)
-    this.setIcon(this.getIcon())
-  },
-
-  getClass: () => LeafletMarker,
-
-  // Make API consistent with path
-  getLatLngs: function () {
-    return this.getLatLng()
-  },
-
-  setLatLngs: function (latlng) {
-    return this.setLatLng(latlng)
-  },
 
   addInteractions() {
     FeatureMixin.addInteractions.call(this)
@@ -138,9 +117,6 @@ export const LeafletMarker = Marker.extend({
     this.on('editable:drawing:commit', this.onCommit)
     if (!this.feature.isReadOnly()) this.on('mouseover', this._enableDragging)
     this.on('mouseout', this._onMouseOut)
-    this._popupHandlersAdded = true // prevent Leaflet from binding event on bindPopup
-    this.on('popupopen', this.highlight)
-    this.on('popupclose', this.resetHighlight)
   },
 
   _onMouseOut: function () {
@@ -170,6 +146,29 @@ export const LeafletMarker = Marker.extend({
       this.disableEdit()
     }
   },
+}
+
+export const LeafletMarker = Marker.extend({
+  parentClass: Marker,
+  includes: [FeatureMixin, PointMixin],
+
+  initialize: function (feature, latlng) {
+    FeatureMixin.initialize.call(this, feature, latlng)
+    this.setIcon(this.getIcon())
+  },
+
+  getClass: () => LeafletMarker,
+
+  setLatLngs: function (latlng) {
+    return this.setLatLng(latlng)
+  },
+
+  addInteractions() {
+    PointMixin.addInteractions.call(this)
+    this._popupHandlersAdded = true // prevent Leaflet from binding event on bindPopup
+    this.on('popupopen', this.highlight)
+    this.on('popupclose', this.resetHighlight)
+  },
 
   _initIcon: function () {
     this.options.icon = this.getIcon()
@@ -433,8 +432,4 @@ export const CircleMarker = BaseCircleMarker.extend({
   getCenter: function () {
     return this._latlng
   },
-  // FIXME when Leaflet.Editable knows about CircleMarker
-  editEnabled: () => false,
-  enableEdit: () => {}, // No-op
-  disableEdit: () => {}, // No-op
 })

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

@@ -1164,9 +1164,9 @@ U.Editable = L.Editable.extend({
       if (this.map.editedFeature !== event.layer) event.layer.feature.edit(event)
     })
     this.on('editable:editing', (event) => {
-      const layer = event.layer
-      layer.feature.isDirty = true
-      layer.feature.fromLatLngs(layer.getLatLngs())
+      const feature = event.layer.feature
+      feature.isDirty = true
+      feature.pullGeometry(false)
     })
     this.on('editable:vertex:ctrlclick', (event) => {
       const index = event.vertex.getIndex()

umap/tests/integration/test_circles_layer.py

@@ -67,3 +67,15 @@ def test_basic_circles_layer(map, live_server, page):
         .get_attribute("d")
         .endswith("a2,2 0 1,0 -4,0 ")
     )
+
+
+def test_can_draw_new_circles(openmap, live_server, page):
+    path = Path(__file__).parent.parent / "fixtures/test_circles_layer.geojson"
+    data = json.loads(path.read_text())
+    DataLayerFactory(data=data, map=openmap)
+    page.goto(f"{live_server.url}{openmap.get_absolute_url()}?edit#12/47.2210/-1.5621")
+    paths = page.locator("path")
+    expect(paths).to_have_count(10)
+    page.get_by_title("Draw a marker").click()
+    page.locator("#map").click(position={"x": 200, "y": 200})
+    expect(paths).to_have_count(11)