From 7dbb3e0b9ac6153c87ce8eddf3dfd15eefdaab85 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Thu, 26 Sep 2024 09:16:00 +0200 Subject: [PATCH] fix: allow to draw new proprotional circles and to drag them 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. fix #2171 --- umap/static/umap/js/modules/data/features.js | 20 +++++++- umap/static/umap/js/modules/rendering/ui.js | 51 +++++++++----------- umap/static/umap/js/umap.controls.js | 6 +-- umap/tests/integration/test_circles_layer.py | 12 +++++ 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/umap/static/umap/js/modules/data/features.js b/umap/static/umap/js/modules/data/features.js index 4f213342..a0dbeeb3 100644 --- a/umap/static/umap/js/modules/data/features.js +++ b/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) } @@ -607,6 +607,14 @@ export class Point extends Feature { } } + _getLatLngs() { + return this.ui.getLatLng() + } + + _setLatLngs(latlng) { + this.ui.setLatLng(latlng) + } + toLatLngs() { return GeoJSON.coordsToLatLng(this.coordinates) } @@ -678,6 +686,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. diff --git a/umap/static/umap/js/modules/rendering/ui.js b/umap/static/umap/js/modules/rendering/ui.js index 72a3354a..d1f8b710 100644 --- a/umap/static/umap/js/modules/rendering/ui.js +++ b/umap/static/umap/js/modules/rendering/ui.js @@ -158,27 +158,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) @@ -190,9 +169,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 () { @@ -222,6 +198,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() @@ -603,8 +602,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 }) diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index a67228df..7936e8d6 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -1181,9 +1181,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() diff --git a/umap/tests/integration/test_circles_layer.py b/umap/tests/integration/test_circles_layer.py index a8663f2c..13d5aaed 100644 --- a/umap/tests/integration/test_circles_layer.py +++ b/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)