From 00c384bf259b847ec6e88806025b98c3e6e576f4 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Fri, 2 Aug 2024 12:12:56 +0200 Subject: [PATCH] feat: allow to display a polygon "negative" fix #1125 --- umap/static/umap/js/modules/data/features.js | 120 +++++++++++-------- umap/static/umap/js/modules/data/layer.js | 6 +- umap/static/umap/js/modules/rendering/ui.js | 76 +++++++++--- umap/static/umap/js/modules/schema.js | 5 + umap/static/umap/js/umap.controls.js | 10 +- umap/tests/integration/test_draw_polygon.py | 55 +++++++++ 6 files changed, 195 insertions(+), 77 deletions(-) diff --git a/umap/static/umap/js/modules/data/features.js b/umap/static/umap/js/modules/data/features.js index 54010818..e15b4924 100644 --- a/umap/static/umap/js/modules/data/features.js +++ b/umap/static/umap/js/modules/data/features.js @@ -9,7 +9,12 @@ import * as Utils from '../utils.js' import { SCHEMA } from '../schema.js' import { translate } from '../i18n.js' import { uMapAlert as Alert } from '../../components/alerts/alert.js' -import { LeafletMarker, LeafletPolyline, LeafletPolygon } from '../rendering/ui.js' +import { + LeafletMarker, + LeafletPolyline, + LeafletPolygon, + MaskPolygon, +} from '../rendering/ui.js' import loadPopup from '../rendering/popup.js' class Feature { @@ -60,7 +65,7 @@ class Feature { } get ui() { - if (!this._ui) this._ui = this.makeUI() + if (!this._ui) this.makeUI() return this._ui } @@ -76,13 +81,41 @@ class Feature { return this.ui.getBounds() } + get type() { + return this.geometry.type + } + + get coordinates() { + return this.geometry.coordinates + } + get geometry() { return this._geometry } set geometry(value) { this._geometry = value - this.geometryChanged() + this.pushGeometry() + } + + pushGeometry() { + this.ui.setLatLngs(this.toLatLngs()) + } + + pullGeometry(sync = true) { + this.fromLatLngs(this.ui.getLatLngs()) + if (sync) { + this.sync.update('geometry', this.geometry) + } + } + + fromLatLngs(latlngs) { + this._geometry = this.convertLatLngs(latlngs) + } + + makeUI() { + const klass = this.getUIClass() + this._ui = new klass(this, this.toLatLngs()) } getClassName() { @@ -536,7 +569,13 @@ class Feature { redraw() { if (this.datalayer?.isVisible()) { - this.ui._redraw() + if (this.getUIClass() !== this.ui.getClass()) { + this.datalayer.hideFeature(this) + this.makeUI() + this.datalayer.showFeature(this) + } else { + this.ui._redraw() + } } } } @@ -550,20 +589,16 @@ export class Point extends Feature { } } - get coordinates() { - return GeoJSON.coordsToLatLng(this.geometry.coordinates) + toLatLngs() { + return GeoJSON.coordsToLatLng(this.coordinates) } - set coordinates(latlng) { - this.geometry.coordinates = GeoJSON.latLngToCoords(latlng) + convertLatLngs(latlng) { + return { coordinates: GeoJSON.latLngToCoords(latlng), type: 'Point' } } - geometryChanged() { - this.ui.setLatLng(this.coordinates) - } - - makeUI() { - return new LeafletMarker(this) + getUIClass() { + return LeafletMarker } hasGeom() { @@ -620,7 +655,7 @@ export class Point extends Feature { isOnScreen(bounds) { bounds = bounds || this.map.getBounds() - return bounds.contains(this.coordinates) + return bounds.contains(this.toLatLngs()) } } @@ -629,20 +664,6 @@ class Path extends Feature { return !this.isEmpty() } - get coordinates() { - return this._toLatlngs(this.geometry) - } - - set coordinates(latlngs) { - const { coordinates, type } = this._toGeometry(latlngs) - this.geometry.coordinates = coordinates - this.geometry.type = type - } - - geometryChanged() { - this.ui.setLatLngs(this.coordinates) - } - connectToDataLayer(datalayer) { super.connectToDataLayer(datalayer) // We keep markers on their own layer on top of the paths. @@ -722,18 +743,18 @@ class Path extends Feature { transferShape(at, to) { const shape = this.ui.enableEdit().deleteShapeAt(at) // FIXME: make Leaflet.Editable send an event instead - this.ui.geometryChanged() + this.pullGeometry() this.ui.disableEdit() if (!shape) return to.ui.enableEdit().appendShape(shape) - to.ui.geometryChanged() + to.pullGeometry() if (this.isEmpty()) this.del() } isolateShape(latlngs) { const properties = this.cloneProperties() const type = this instanceof LineString ? 'LineString' : 'Polygon' - const geometry = this._toGeometry(latlngs) + const geometry = this.convertLatLngs(latlngs) const other = this.datalayer.makeFeature({ type, geometry, properties }) other.edit() return other @@ -776,14 +797,11 @@ export class LineString extends Path { } } - _toLatlngs(geometry) { - return GeoJSON.coordsToLatLngs( - geometry.coordinates, - geometry.type === 'LineString' ? 0 : 1 - ) + toLatLngs(geometry) { + return GeoJSON.coordsToLatLngs(this.coordinates, this.type === 'LineString' ? 0 : 1) } - _toGeometry(latlngs) { + convertLatLngs(latlngs) { let multi = !LineUtil.isFlat(latlngs) let coordinates = GeoJSON.latLngsToCoords(latlngs, multi ? 1 : 0, false) if (coordinates.length === 1 && typeof coordinates[0][0] !== 'number') { @@ -798,8 +816,8 @@ export class LineString extends Path { return !this.coordinates.length } - makeUI() { - return new LeafletPolyline(this) + getUIClass() { + return LeafletPolyline } isSameClass(other) { @@ -875,7 +893,7 @@ export class LineString extends Path { while (latlngs.length > 1) { latlngs.splice(0, 2, this._mergeShapes(latlngs[1], latlngs[0])) } - this.setLatLngs(latlngs[0]) + this.ui.setLatLngs(latlngs[0]) if (!this.editEnabled()) this.edit() this.editor.reset() this.isDirty = true @@ -895,14 +913,11 @@ export class Polygon extends Path { } } - _toLatlngs(geometry) { - return GeoJSON.coordsToLatLngs( - geometry.coordinates, - geometry.type === 'Polygon' ? 1 : 2 - ) + toLatLngs() { + return GeoJSON.coordsToLatLngs(this.coordinates, this.type === 'Polygon' ? 1 : 2) } - _toGeometry(latlngs) { + convertLatLngs(latlngs) { const holes = !LineUtil.isFlat(latlngs) let multi = holes && !LineUtil.isFlat(latlngs[0]) let coordinates = GeoJSON.latLngsToCoords(latlngs, multi ? 2 : holes ? 1 : 0, true) @@ -918,8 +933,9 @@ export class Polygon extends Path { return !this.coordinates.length || !this.coordinates[0].length } - makeUI() { - return new LeafletPolygon(this) + getUIClass() { + if (this.getOption('mask')) return MaskPolygon + return LeafletPolygon } isSameClass(other) { @@ -969,6 +985,12 @@ export class Polygon extends Path { this.del() } + getAdvancedOptions() { + const actions = super.getAdvancedOptions() + actions.push('properties._umap_options.mask') + return actions + } + getAdvancedEditActions(container) { super.getAdvancedEditActions(container) const toLineString = DomUtil.createButton( diff --git a/umap/static/umap/js/modules/data/layer.js b/umap/static/umap/js/modules/data/layer.js index 4e7ebe49..915fe5ba 100644 --- a/umap/static/umap/js/modules/data/layer.js +++ b/umap/static/umap/js/modules/data/layer.js @@ -363,6 +363,10 @@ export class DataLayer { this.layer.addLayer(feature.ui) } + hideFeature(feature) { + this.layer.removeLayer(feature.ui) + } + addFeature(feature) { const id = stamp(feature) feature.connectToDataLayer(this) @@ -377,7 +381,7 @@ export class DataLayer { removeFeature(feature, sync) { const id = stamp(feature) if (sync !== false) feature.sync.delete() - this.layer.removeLayer(feature.ui) + this.hideFeature(feature) delete this.map.features_index[feature.getSlug()] feature.disconnectFromDataLayer(this) this._index.splice(this._index.indexOf(id), 1) diff --git a/umap/static/umap/js/modules/rendering/ui.js b/umap/static/umap/js/modules/rendering/ui.js index 86316195..ad799a59 100644 --- a/umap/static/umap/js/modules/rendering/ui.js +++ b/umap/static/umap/js/modules/rendering/ui.js @@ -5,15 +5,17 @@ import { Polygon, DomUtil, LineUtil, + latLng, + LatLngBounds, } from '../../../vendors/leaflet/leaflet-src.esm.js' import { translate } from '../i18n.js' import { uMapAlert as Alert } from '../../components/alerts/alert.js' import * as Utils from '../utils.js' const FeatureMixin = { - initialize: function (feature) { + initialize: function (feature, latlngs) { this.feature = feature - this.parentClass.prototype.initialize.call(this, this.feature.coordinates) + this.parentClass.prototype.initialize.call(this, latlngs) }, onAdd: function (map) { @@ -134,7 +136,7 @@ const FeatureMixin = { }, onCommit: function () { - this.geometryChanged(false) + this.feature.pullGeometry(false) this.feature.onCommit() }, @@ -145,16 +147,20 @@ export const LeafletMarker = Marker.extend({ parentClass: Marker, includes: [FeatureMixin], - initialize: function (feature) { - FeatureMixin.initialize.call(this, feature) + initialize: function (feature, latlng) { + FeatureMixin.initialize.call(this, feature, latlng) this.setIcon(this.getIcon()) }, - geometryChanged: function (sync = true) { - this.feature.coordinates = this._latlng - if (sync) { - this.feature.sync.update('geometry', this.feature.geometry) - } + getClass: () => LeafletMarker, + + // Make API consistent with path + getLatLngs: function () { + return this.getLatLng() + }, + + setLatLngs: function (latlng) { + return this.setLatLng(latlng) }, addInteractions() { @@ -162,7 +168,7 @@ export const LeafletMarker = Marker.extend({ this.on('dragend', (event) => { this.isDirty = true this.feature.edit(event) - this.geometryChanged() + this.feature.pullGeometry() }) this.on('editable:drawing:commit', this.onCommit) if (!this.feature.isReadOnly()) this.on('mouseover', this._enableDragging) @@ -265,10 +271,6 @@ const PathMixin = { } }, - geometryChanged: function () { - this.feature.coordinates = this._latlngs - }, - addInteractions: function () { FeatureMixin.addInteractions.call(this) this.on('editable:disable', this.onCommit) @@ -385,20 +387,22 @@ const PathMixin = { return items }, - isolateShape: function(atLatLng) { + isolateShape: function (atLatLng) { if (!this.feature.isMulti()) return const shape = this.enableEdit().deleteShapeAt(atLatLng) - this.geometryChanged() + this.feature.pullGeometry() this.disableEdit() if (!shape) return return this.feature.isolateShape(shape) - } + }, } export const LeafletPolyline = Polyline.extend({ parentClass: Polyline, includes: [FeatureMixin, PathMixin], + getClass: () => LeafletPolyline, + getVertexActions: function (event) { const actions = PathMixin.getVertexActions.call(this, event) const index = event.vertex.getIndex() @@ -455,6 +459,8 @@ export const LeafletPolygon = Polygon.extend({ parentClass: Polygon, includes: [FeatureMixin, PathMixin], + getClass: () => LeafletPolygon, + getContextMenuEditItems: function (event) { const items = PathMixin.getContextMenuEditItems.call(this, event) const shape = this.shapeAt(event.latlng) @@ -482,3 +488,37 @@ export const LeafletPolygon = Polygon.extend({ this.enableEdit().newHole(event.latlng) }, }) +const WORLD = [ + latLng([90, 180]), + latLng([90, -180]), + latLng([-90, -180]), + latLng([-90, 180]), +] + +export const MaskPolygon = LeafletPolygon.extend({ + getClass: () => MaskPolygon, + + getLatLngs: function () { + // Exclude World coordinates. + return LeafletPolygon.prototype.getLatLngs.call(this).slice(1) + }, + + _setLatLngs: function (latlngs) { + const newLatLngs = [] + newLatLngs.push(WORLD) + + if (!this.feature.isMulti()) { + latlngs = [latlngs] + } + for (const ring of latlngs) { + newLatLngs.push(ring) + } + LeafletPolygon.prototype._setLatLngs.call(this, newLatLngs) + this._bounds = new LatLngBounds(latlngs) + }, + + _defaultShape: function () { + // Do not compute with world coordinates (eg. for centering the popup). + return this._latlngs[1] + }, +}) diff --git a/umap/static/umap/js/modules/schema.js b/umap/static/umap/js/modules/schema.js index 564d8b69..363b56c4 100644 --- a/umap/static/umap/js/modules/schema.js +++ b/umap/static/umap/js/modules/schema.js @@ -287,6 +287,11 @@ export const SCHEMA = { nullable: true, label: translate('Display the measure control'), }, + mask: { + type: Boolean, + impacts: ['data'], + label: translate('Display the polygon inverted'), + }, miniMap: { type: Boolean, impacts: ['ui'], diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index 8b5ea4e9..210271b8 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -1167,15 +1167,7 @@ U.Editable = L.Editable.extend({ this.on('editable:editing', (event) => { const layer = event.layer layer.feature.isDirty = true - if (layer instanceof L.Marker) { - layer.feature.coordinates = layer._latlng - } else { - layer.feature.coordinates = layer._latlngs - } - // if (layer._tooltip && layer.isTooltipOpen()) { - // layer._tooltip.setLatLng(layer.getCenter()) - // layer._tooltip.update() - // } + layer.feature.fromLatLngs(layer.getLatLngs()) }) this.on('editable:vertex:ctrlclick', (event) => { const index = event.vertex.getIndex() diff --git a/umap/tests/integration/test_draw_polygon.py b/umap/tests/integration/test_draw_polygon.py index f3443e8e..89a4665b 100644 --- a/umap/tests/integration/test_draw_polygon.py +++ b/umap/tests/integration/test_draw_polygon.py @@ -420,3 +420,58 @@ def test_can_transform_polygon_to_line(live_server, page, tilelayer, settings): data = save_and_get_json(page) assert len(data["features"]) == 1 assert data["features"][0]["geometry"]["type"] == "LineString" + + +def test_can_draw_a_polygon_and_invert_it(live_server, page, tilelayer, settings): + settings.UMAP_ALLOW_ANONYMOUS = True + page.goto(f"{live_server.url}/en/map/new/") + paths = page.locator(".leaflet-overlay-pane path") + expect(paths).to_have_count(0) + page.get_by_title("Draw a polygon").click() + map = page.locator("#map") + map.click(position={"x": 200, "y": 100}) + map.click(position={"x": 200, "y": 200}) + map.click(position={"x": 100, "y": 200}) + map.click(position={"x": 100, "y": 100}) + # Click again to finish + map.click(position={"x": 100, "y": 100}) + expect(paths).to_have_count(1) + page.get_by_text("Advanced properties").click() + page.get_by_text("Display the polygon inverted").click() + data = save_and_get_json(page) + assert len(data["features"]) == 1 + assert data["features"][0]["geometry"]["type"] == "Polygon" + assert data["features"][0]["geometry"]["coordinates"] == [ + [ + [ + -7.668457, + 54.457267, + ], + [ + -7.668457, + 53.159947, + ], + [ + -9.865723, + 53.159947, + ], + [ + -9.865723, + 54.457267, + ], + [ + -7.668457, + 54.457267, + ], + ], + ] + + page.get_by_role("button", name="View").click() + popup = page.locator(".leaflet-popup") + expect(popup).to_be_hidden() + # Now click on the middle of the polygon, it should not show the popup + map.click(position={"x": 150, "y": 150}) + expect(popup).to_be_hidden() + # Click elsewhere on the map, it should now show the popup + map.click(position={"x": 250, "y": 250}) + expect(popup).to_be_visible()