Skip to content

Commit

Permalink
💥 [open-formulieren/open-forms#2177] Map component now only returns g…
Browse files Browse the repository at this point in the history
…eoJson geometry

Because we won't support circle shapes (due to geoJson and the backend registrations not supporting this type), we no longer need the geoJson feature `properties`.

Because we don't use `properties` for anything else, it doesn't make sense to save it
  • Loading branch information
robinmolen committed Jan 14, 2025
1 parent b6a18bc commit c206b88
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 71 deletions.
2 changes: 1 addition & 1 deletion src/components/FormStepSummary/ComponentValueDisplay.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ const MapDisplay = ({component, value}) => {
return <EmptyDisplay />;
}

return <Map geoJsonFeature={value} disabled tileLayerUrl={component.tileLayerUrl} />;
return <Map geoJsonGeometry={value} disabled tileLayerUrl={component.tileLayerUrl} />;
};

const CoSignDisplay = ({value}) => {
Expand Down
59 changes: 24 additions & 35 deletions src/components/Map/Map.stories.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ const withMapLayout = Story => (
);

const StorybookLeafletMap = props => {
const [geoJson, setGeoJson] = useState(props?.geoJsonFeature);
const [geoJson, setGeoJson] = useState(props?.geoJsonGeometry);
const handleGeoJsonChange = args => {
if (props?.onGeoJsonFeatureSet) {
props?.onGeoJsonFeatureSet(args);
if (props?.onGeoJsonGeometrySet) {
props?.onGeoJsonGeometrySet(args);
}
setGeoJson(args);
};
return (
<LeafletMap {...props} geoJsonFeature={geoJson} onGeoJsonFeatureSet={handleGeoJsonChange} />
<LeafletMap {...props} geoJsonGeometry={geoJson} onGeoJsonGeometrySet={handleGeoJsonChange} />
);
};

Expand All @@ -31,13 +31,9 @@ export default {
decorators: [withMapLayout, ConfigDecorator],
render: StorybookLeafletMap,
args: {
geoJsonFeature: {
type: 'Feature',
properties: {},
geometry: {
type: 'Point',
coordinates: [5.291266, 52.1326332],
},
geoJsonGeometry: {
type: 'Point',
coordinates: [5.291266, 52.1326332],
},
defaultCenter: [52.1326332, 5.291266],
defaultZoomLevel: 12,
Expand All @@ -54,7 +50,7 @@ export const Map = {};

export const MapWithAddressSearch = {
args: {
onGeoJsonFeatureSet: fn(),
onGeoJsonGeometrySet: fn(),
},
play: async ({canvasElement, args}) => {
const canvas = within(canvasElement);
Expand All @@ -73,14 +69,10 @@ export const MapWithAddressSearch = {
await userEvent.click(searchResult);
await waitFor(async () => {
// A marker is placed on the search result
expect(args.onGeoJsonFeatureSet).toBeCalledWith({
type: 'Feature',
properties: {},
geometry: {
type: 'Point',
// To make sure that this test doesn't magically fail, just expect any 2 values
coordinates: [expect.anything(), expect.anything()],
},
expect(args.onGeoJsonGeometrySet).toBeCalledWith({
type: 'Point',
// To make sure that this test doesn't magically fail, just expect any 2 values
coordinates: [expect.anything(), expect.anything()],
});
});
},
Expand Down Expand Up @@ -108,7 +100,7 @@ export const MapWithInteractions = {
polyline: true,
marker: true,
},
onGeoJsonFeatureSet: fn(),
onGeoJsonGeometrySet: fn(),
},
parameters: {
msw: {
Expand All @@ -120,28 +112,25 @@ export const MapWithInteractions = {
const map = canvasElement.querySelector('.leaflet-pane.leaflet-map-pane');

await step('All interactions are available', async () => {
expect(await canvas.findByTitle('Draw a marker')).toBeVisible();
expect(await canvas.findByTitle('Draw a polygon')).toBeVisible();
expect(await canvas.findByTitle('Draw a polyline')).toBeVisible();
expect(await canvas.findByTitle('Pin/punt')).toBeVisible();
expect(await canvas.findByTitle('Veelhoek (polygoon)')).toBeVisible();
expect(await canvas.findByTitle('Lijn')).toBeVisible();
});

await step('Draw a marker', async () => {
const markerButton = await canvas.findByTitle('Draw a marker');
const markerButton = await canvas.findByTitle('Pin/punt');
await userEvent.click(markerButton);

await userEvent.click(map, {x: 100, y: 100});

// This 'button' is the placed marker on the map
expect(await canvas.findByRole('button', {name: 'Marker'})).toBeVisible();
expect(args.onGeoJsonFeatureSet).toBeCalledWith({
type: 'Feature',
properties: {},
geometry: {
type: 'Point',
// Expect that the coordinates array contains 2 items.
// We cannot pin it to specific values, because they can differentiate.
// To make sure that this test doesn't magically fail, just expect any 2 values
coordinates: [expect.anything(), expect.anything()],
},
expect(args.onGeoJsonGeometrySet).toBeCalledWith({
type: 'Point',
// Expect that the coordinates array contains 2 items.
// We cannot pin it to specific values, because they can differentiate.
// To make sure that this test doesn't magically fail, just expect any 2 values
coordinates: [expect.anything(), expect.anything()],
});
});
},
Expand Down
61 changes: 29 additions & 32 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, MapContainer, TileLayer, useMap} from 'react-leaflet';
import {FeatureGroup, GeoJSON, MapContainer, TileLayer, useMap} from 'react-leaflet';
import {EditControl} from 'react-leaflet-draw';
import {useGeolocation} from 'react-use';

Expand Down Expand Up @@ -43,18 +43,18 @@ const useDefaultCoordinates = () => {
return [latitude, longitude];
};

const getCoordinates = geoJsonFeature => {
if (!geoJsonFeature) {
const getCoordinates = geoJsonGeometry => {
if (!geoJsonGeometry) {
return null;
}

const center = Leaflet.geoJSON(geoJsonFeature).getBounds().getCenter();
const center = Leaflet.geoJSON(geoJsonGeometry).getBounds().getCenter();
return [center.lat, center.lng];
};

const LeaftletMap = ({
geoJsonFeature,
onGeoJsonFeatureSet,
geoJsonGeometry,
onGeoJsonGeometrySet,
defaultCenter = DEFAULT_LAT_LNG,
defaultZoomLevel = DEFAULT_ZOOM,
disabled = false,
Expand All @@ -64,7 +64,7 @@ const LeaftletMap = ({
const featureGroupRef = useRef();
const intl = useIntl();
const defaultCoordinates = useDefaultCoordinates();
const geoJsonCoordinates = getCoordinates(geoJsonFeature);
const geoJsonCoordinates = getCoordinates(geoJsonGeometry);
const coordinates = geoJsonCoordinates ?? defaultCoordinates;

const modifiers = disabled ? ['disabled'] : [];
Expand All @@ -73,20 +73,20 @@ const LeaftletMap = ({
applyLeafletTranslations(intl);

const onFeatureCreate = event => {
updateGeoJsonFeature(event.layer);
updateGeoJsonGeometry(event.layer);
};

const onFeatureDelete = () => {
// The value `null` is needed to make sure that Formio actually updates the value.
// node_modules/formiojs/components/_classes/component/Component.js:2528
onGeoJsonFeatureSet(null);
onGeoJsonGeometrySet(null);

Check warning on line 82 in src/components/Map/index.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/Map/index.jsx#L82

Added line #L82 was not covered by tests
};

const onSearchMarkerSet = event => {
updateGeoJsonFeature(event.marker);
updateGeoJsonGeometry(event.marker);
};

const updateGeoJsonFeature = newFeatureLayer => {
const updateGeoJsonGeometry = newFeatureLayer => {
if (featureGroupRef.current) {
const newFeatureLayerId = newFeatureLayer._leaflet_id;
const layers = featureGroupRef.current?.getLayers();
Expand All @@ -100,7 +100,7 @@ const LeaftletMap = ({
});
}
}
onGeoJsonFeatureSet(newFeatureLayer.toGeoJSON());
onGeoJsonGeometrySet(newFeatureLayer.toGeoJSON().geometry);
};

return (
Expand Down Expand Up @@ -142,6 +142,7 @@ const LeaftletMap = ({
}}
/>
)}
{geoJsonGeometry && <GeoJSON data={geoJsonGeometry} />}
</FeatureGroup>
{coordinates && <MapView coordinates={coordinates} />}
{!disabled && (
Expand Down Expand Up @@ -170,26 +171,22 @@ const LeaftletMap = ({
};

LeaftletMap.propTypes = {
geoJsonFeature: PropTypes.shape({
type: PropTypes.oneOf(['Feature']).isRequired,
properties: PropTypes.object,
geometry: PropTypes.oneOfType([
PropTypes.shape({
type: PropTypes.oneOf(['Point']).isRequired,
coordinates: PropTypes.arrayOf(PropTypes.number).isRequired,
}),
PropTypes.shape({
type: PropTypes.oneOf(['LineString']).isRequired,
coordinates: PropTypes.arrayOf(PropTypes.arrayOf(PropTypes.number)).isRequired,
}),
PropTypes.shape({
type: PropTypes.oneOf(['Polygon']).isRequired,
coordinates: PropTypes.arrayOf(PropTypes.arrayOf(PropTypes.arrayOf(PropTypes.number)))
.isRequired,
}),
]).isRequired,
}),
onGeoJsonFeatureSet: PropTypes.func,
geoJsonGeometry: PropTypes.oneOfType([
PropTypes.shape({
type: PropTypes.oneOf(['Point']).isRequired,
coordinates: PropTypes.arrayOf(PropTypes.number).isRequired,
}),
PropTypes.shape({
type: PropTypes.oneOf(['LineString']).isRequired,
coordinates: PropTypes.arrayOf(PropTypes.arrayOf(PropTypes.number)).isRequired,
}),
PropTypes.shape({
type: PropTypes.oneOf(['Polygon']).isRequired,
coordinates: PropTypes.arrayOf(PropTypes.arrayOf(PropTypes.arrayOf(PropTypes.number)))
.isRequired,
}),
]),
onGeoJsonGeometrySet: PropTypes.func,
interactions: PropTypes.shape({
polyline: PropTypes.bool,
polygon: PropTypes.bool,
Expand Down
6 changes: 3 additions & 3 deletions src/formio/components/Map.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export default class Map extends Field {
const {lat = defaultLat, lng = defaultLon} = this.component?.initialCenter || {};
const defaultCenter = [lat, lng];

const geoJsonFeature = this.getValue();
const geoJsonGeometry = this.getValue();

const container = this.refs.mapContainer;
const zoom = this.component.defaultZoom;
Expand All @@ -110,8 +110,8 @@ export default class Map extends Field {
<IntlProvider {...this.options.intl}>
<ConfigContext.Provider value={{baseUrl: this.options.baseUrl}}>
<LeafletMap
geoJsonFeature={geoJsonFeature || null}
onGeoJsonFeatureSet={this.onGeoJsonSet.bind(this)}
geoJsonGeometry={geoJsonGeometry || null}
onGeoJsonGeometrySet={this.onGeoJsonSet.bind(this)}
defaultCenter={defaultCenter}
defaultZoomLevel={zoom || DEFAULT_ZOOM}
interactions={this.component?.interactions}
Expand Down

0 comments on commit c206b88

Please sign in to comment.