Skip to content

Commit

Permalink
🐛 [open-formulieren/open-forms#2177] Duplicate Leaflet shapes
Browse files Browse the repository at this point in the history
Since we persist the leaflet changes in React state, and have to act on state changes, only the shapes from the React state should be shown.

Leaflet adds new shapes to its map before calling `onCreated`. If we then save the new shape to state, and show this using `<GeoJSON ... />`, we have two shapes on the exact same spot. This causes weird behavior when you want to remove them using the delete button.
Clearing the leaflet map before saving the new shape does ensure only one shape is present. If we then use `<GeoJSON ... />` to show the new shape, the leaflet soft-delete won't work. (Because `<GeoJSON ... />` can't be temporarily hidden, without some extra work)

To ensure only one shape is present and to allow soft-deletion, we have two options:
- We clear the map every time a new shape is created, and save the new shape to state. Using `<GeoJSON ... />` we show the newly created shape. For the deletion we keep inventory of the soft-delete actions and change the map display accordingly, and persist the changes when confirmed by the user.
- We clear the map every time a new shape is created, and save the new shape to state. Using useEffect we add the newly created shape to the leaflet map (this doesn't trigger `onCreated`). With this we can use the complete deletion functionality from leaflet, and only have to persist on `onDeleted`.

To keep this component as simple as we can, I've opted for the second option
  • Loading branch information
robinmolen committed Jan 22, 2025
1 parent eeaed17 commit e3978d7
Showing 1 changed file with 17 additions and 17 deletions.
34 changes: 17 additions & 17 deletions src/components/Map/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {GeoSearchControl} from 'leaflet-geosearch';
import PropTypes from 'prop-types';
import {useContext, useEffect, useRef} from 'react';
import {useIntl} from 'react-intl';
import {FeatureGroup, GeoJSON, MapContainer, TileLayer, useMap} from 'react-leaflet';
import {FeatureGroup, MapContainer, TileLayer, useMap} from 'react-leaflet';
import {EditControl} from 'react-leaflet-draw';
import {useGeolocation} from 'react-use';

Expand Down Expand Up @@ -88,23 +88,24 @@ const LeaftletMap = ({
};

const updateGeoJsonGeometry = newFeatureLayer => {
const featureGroup = featureGroupRef.current;
if (featureGroup) {
const newLayerId = featureGroup.getLayerId(newFeatureLayer);

featureGroup.eachLayer(layer => {
const layerId = featureGroup.getLayerId(layer);

// Remove all layers that aren't the newly added layer
// Ensuring that only 1 layer/shape will be present at any time
if (layerId !== newLayerId) {
featureGroup.removeLayer(layer);
}
});
}
// Remove all existing shapes from the map, ensuring that shapes are only added through
// `geoJsonGeometry` changes.
featureGroupRef.current?.clearLayers();
onGeoJsonGeometrySet(newFeatureLayer.toGeoJSON().geometry);
};

useEffect(() => {
// Remove all shapes from the map.
// Only `geoJsonGeometry` should be shown on the map.
featureGroupRef.current?.clearLayers();
if (!geoJsonGeometry) {
return;
}
// Add the current `geoJsonGeometry` as shape
const layer = Leaflet.GeoJSON.geometryToLayer(geoJsonGeometry);
featureGroupRef.current?.addLayer(layer);
}, [geoJsonGeometry]);

return (
<>
<MapContainer
Expand Down Expand Up @@ -144,7 +145,6 @@ const LeaftletMap = ({
}}
/>
)}
{geoJsonGeometry && <GeoJSON data={geoJsonGeometry} />}
</FeatureGroup>
{coordinates && <MapView coordinates={coordinates} />}
{!disabled && (
Expand Down Expand Up @@ -188,7 +188,7 @@ LeaftletMap.propTypes = {
.isRequired,
}),
]),
onGeoJsonGeometrySet: PropTypes.func,
onGeoJsonGeometrySet: PropTypes.func.isRequired,
interactions: PropTypes.shape({
polyline: PropTypes.bool,
polygon: PropTypes.bool,
Expand Down

0 comments on commit e3978d7

Please sign in to comment.