Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(map-and-label): remove individual features #3625

Merged
merged 13 commits into from
Sep 9, 2024
+170 −67
Merged

feat(map-and-label): remove individual features #3625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
500d50b
feat: Remove feature from formik state
DafyddLlyr Aug 29, 2024
7f71269
refactor: Make CopyFeature it's own component
DafyddLlyr Aug 30, 2024
f5bb2f4
feat: Add mapId as an arg to hook
DafyddLlyr Aug 30, 2024
cefbe1c
feat: Remove feature from array (but not yet map)
DafyddLlyr Aug 30, 2024
5666a81
rebased
jessicamcinchak Sep 4, 2024
628aaeb
rebased again, does not remove map feature
jessicamcinchak Sep 5, 2024
e40686b
add back data-testid
jessicamcinchak Sep 5, 2024
2807a98
map updates
jessicamcinchak Sep 5, 2024
0e59f17
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Sep 6, 2024
600d650
fix remove after first bug
jessicamcinchak Sep 6, 2024
a9211bc
shift labels after removing non-last feature
jessicamcinchak Sep 6, 2024
a54c9b6
basic back/change navigation re-added
jessicamcinchak Sep 6, 2024
9b48836
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions editor.planx.uk/src/@planx/components/List/Editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import { ProtectedSpaceGLA } from "./schemas/GLA/ProtectedSpace";
import { MaterialDetails } from "./schemas/Materials";
import { Parking } from "./schemas/Parking";
import { ResidentialUnitsExisting } from "./schemas/ResidentialUnits/Existing";
import { ResidentialUnitsGLAGained } from "./schemas/ResidentialUnits/GLA/Gained";
import { ResidentialUnitsGLALost } from "./schemas/ResidentialUnits/GLA/Lost";
import { ResidentialUnitsGLANew } from "./schemas/ResidentialUnits/GLA/New";
import { ResidentialUnitsGLARebuilt } from "./schemas/ResidentialUnits/GLA/Rebuilt";
import { ResidentialUnitsGLARemoved } from "./schemas/ResidentialUnits/GLA/Removed";
import { ResidentialUnitsGLARetained } from "./schemas/ResidentialUnits/GLA/Retained";
import { ResidentialUnitsGLALost } from "./schemas/ResidentialUnits/GLA/Lost";
import { ResidentialUnitsGLAGained } from "./schemas/ResidentialUnits/GLA/Gained";
import { ResidentialUnitsProposed } from "./schemas/ResidentialUnits/Proposed";
import { Trees } from "./schemas/Trees";
import { TreesMapFirst } from "./schemas/TreesMapFirst";
Expand Down
108 changes: 95 additions & 13 deletions editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
makeData,
} from "@planx/components/shared/utils";
import { FormikProps, useFormik } from "formik";
import { FeatureCollection } from "geojson";
import { Feature, FeatureCollection } from "geojson";
import { GeoJSONChange, GeoJSONChangeEvent, useGeoJSONChange } from "lib/gis";
import { get } from "lodash";
import React, {
createContext,
Expand All @@ -20,15 +21,20 @@ import React, {

import { PresentationalProps } from ".";

export const MAP_ID = "map-and-label-map";

interface MapAndLabelContextValue {
schema: Schema;
features?: Feature[];
updateMapKey: number;
activeIndex: number;
editFeature: (index: number) => void;
formik: FormikProps<SchemaUserData>;
validateAndSubmitForm: () => void;
isFeatureInvalid: (index: number) => boolean;
addFeature: () => void;
addInitialFeaturesToMap: (features: Feature[]) => void;
editFeature: (index: number) => void;
copyFeature: (sourceIndex: number, destinationIndex: number) => void;
removeFeature: (index: number) => void;
mapAndLabelProps: PresentationalProps;
errors: {
min: boolean;
Expand All @@ -51,29 +57,28 @@ export const MapAndLabelProvider: React.FC<MapAndLabelProviderProps> = (
previousValues: getPreviouslySubmittedData(props),
});

// Deconstruct GeoJSON saved to passport back into schemaData & geoData
// Deconstruct GeoJSON saved to passport back into form data and map data
const previousGeojson = previouslySubmittedData?.data?.[
fn
] as FeatureCollection;
const previousSchemaData = previousGeojson?.features.map(
const previousFormData = previousGeojson?.features.map(
(feature) => feature.properties,
) as SchemaUserResponse[];
const previousGeoData = previousGeojson?.features;
const _previousMapData = previousGeojson?.features;

const formik = useFormik<SchemaUserData>({
...formikConfig,
// The user interactions are map driven - start with no values added
initialValues: {
schemaData: previousSchemaData || [],
geoData: previousGeoData || [],
schemaData: previousFormData || [],
},
onSubmit: (values) => {
const geojson: FeatureCollection = {
type: "FeatureCollection",
features: [],
};

values.geoData?.forEach((feature, i) => {
features?.forEach((feature, i) => {
// Store user inputs as GeoJSON properties
const mergedProperties = {
...feature.properties,
Expand All @@ -93,14 +98,40 @@ export const MapAndLabelProvider: React.FC<MapAndLabelProviderProps> = (
},
});

const [activeIndex, setActiveIndex] = useState<number>(0);
const [activeIndex, setActiveIndex] = useState<number>(-1);

const [minError, setMinError] = useState<boolean>(false);
const [maxError, setMaxError] = useState<boolean>(false);

const handleGeoJSONChange = (event: GeoJSONChangeEvent) => {
// If the user clicks 'reset' on the map, geojson will be empty object
const userHitsReset = !event.detail["EPSG:3857"];

if (userHitsReset) {
removeAllFeaturesFromMap();
removeAllFeaturesFromForm();
return;
}

addFeatureToMap(event.detail);
addFeatureToForm();
};

const [features, setFeatures] = useGeoJSONChange(MAP_ID, handleGeoJSONChange);

const [updateMapKey, setUpdateMapKey] = useState<number>(0);

const resetErrors = () => {
setMinError(false);
setMaxError(false);
formik.setErrors({});
};

const removeAllFeaturesFromMap = () => setFeatures(undefined);

const removeAllFeaturesFromForm = () => {
formik.setFieldValue("schemaData", []);
setActiveIndex(-1);
};

const validateAndSubmitForm = () => {
Expand All @@ -119,7 +150,18 @@ export const MapAndLabelProvider: React.FC<MapAndLabelProviderProps> = (
const isFeatureInvalid = (index: number) =>
Boolean(get(formik.errors, ["schemaData", index]));

const addFeature = () => {
const addFeatureToMap = (geojson: GeoJSONChange) => {
resetErrors();
setFeatures(geojson["EPSG:3857"].features);
setActiveIndex((features && features?.length - 2) || activeIndex + 1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some ugly active tab index management going on! Very much need to revisit this / open to suggestions.

In many instances, I want the active tab index to be the last in the list. Otherwise there can be some strange behavior, for example on staging:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is an issue in the List component which is avoided by only allowing a single active item at a time (it's not possible to add when there's an active item). That solution won't work here due to the map driven nature of the component.

It's going to be a little tricky as setFeatures() is async, but we can take the length from geojson["EPSG:3857"].features reliably I believe.

Something like this might work?

const newFeatures = geojson["EPSG:3857"].features;
setFeatures(newFeatures);
setActiveIndex(newFeatures.length - 1)

I've not tried this so I might be off the mark a little here and treading over old ground you've already worked through.

An alternative might a useEffect() to catch changes to features.length and tie the update to setActiveIndex to that?

};

const addInitialFeaturesToMap = (features: Feature[]) => {
setFeatures(features);
// TODO: setActiveIndex ?
};

const addFeatureToForm = () => {
resetErrors();

const currentFeatures = formik.values.schemaData;
Expand All @@ -130,24 +172,64 @@ export const MapAndLabelProvider: React.FC<MapAndLabelProviderProps> = (
if (schema.max && updatedFeatures.length > schema.max) {
setMaxError(true);
}

setActiveIndex(activeIndex + 1);
};

const copyFeature = (sourceIndex: number, destinationIndex: number) => {
const sourceFeature = formik.values.schemaData[sourceIndex];
formik.setFieldValue(`schemaData[${destinationIndex}]`, sourceFeature);
};

const removeFeatureFromForm = (index: number) => {
formik.setFieldValue(
"schemaData",
formik.values.schemaData.filter((_, i) => i !== index),
);
};

const removeFeatureFromMap = (index: number) => {
// Order of features can vary by change/modification, filter on label not array position
const label = `${index + 1}`;
const filteredFeatures = features?.filter(
(f) => f.properties?.label !== label,
);

// Shift any feature labels that are larger than the removed feature label so they remain incremental
filteredFeatures?.map((f) => {
if (f.properties && Number(f.properties?.label) > Number(label)) {
const newLabel = Number(f.properties.label) - 1;
Object.assign(f, { properties: { label: `${newLabel}` } });
}
});
setFeatures(filteredFeatures);

// `updateMapKey` is set as a unique `key` prop on the map container to force a re-render of its children (aka <my-map />) on change
setUpdateMapKey(updateMapKey + 1);
};

const removeFeature = (index: number) => {
resetErrors();
removeFeatureFromForm(index);
removeFeatureFromMap(index);
// Set active index as highest tab after removal, so that when you "add" a new feature the tabs increment correctly
setActiveIndex((features && features.length - 2) || activeIndex - 1);
};

return (
<MapAndLabelContext.Provider
value={{
features,
updateMapKey,
activeIndex,
schema,
mapAndLabelProps: props,
editFeature,
formik,
validateAndSubmitForm,
addFeature,
addInitialFeaturesToMap,
editFeature,
copyFeature,
removeFeature,
isFeatureInvalid,
errors: {
min: minError,
Expand Down
83 changes: 32 additions & 51 deletions editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import Typography from "@mui/material/Typography";
import { SiteAddress } from "@planx/components/FindProperty/model";
import { ErrorSummaryContainer } from "@planx/components/shared/Preview/ErrorSummaryContainer";
import { SchemaFields } from "@planx/components/shared/Schema/SchemaFields";
import { Feature, FeatureCollection, GeoJsonObject } from "geojson";
import { GeoJsonObject } from "geojson";
import sortBy from "lodash/sortBy";
import { useStore } from "pages/FlowEditor/lib/store";
import React, { useEffect, useState } from "react";
import React from "react";
import { FONT_WEIGHT_SEMI_BOLD } from "theme";
import FullWidthWrapper from "ui/public/FullWidthWrapper";
import ErrorWrapper from "ui/shared/ErrorWrapper";
Expand All @@ -23,7 +23,7 @@ import CardHeader from "../../shared/Preview/CardHeader";
import { MapContainer } from "../../shared/Preview/MapContainer";
import { PublicProps } from "../../ui";
import type { MapAndLabel } from "./../model";
import { MapAndLabelProvider, useMapAndLabelContext } from "./Context";
import { MAP_ID, MapAndLabelProvider, useMapAndLabelContext } from "./Context";
import { CopyFeature } from "./CopyFeature";

type Props = PublicProps<MapAndLabel>;
Expand Down Expand Up @@ -55,11 +55,20 @@ const StyledTab = styled((props: TabProps) => (
},
})) as typeof Tab;

const VerticalFeatureTabs: React.FC<{ features: Feature[] }> = ({
features,
}) => {
const { schema, activeIndex, formik, editFeature, isFeatureInvalid } =
useMapAndLabelContext();
const VerticalFeatureTabs: React.FC = () => {
const {
schema,
activeIndex,
formik,
features,
editFeature,
isFeatureInvalid,
removeFeature,
} = useMapAndLabelContext();

if (!features) {
throw new Error("Cannot render MapAndLabel tabs without features");
}

// Features is inherently sorted by recently added/modified, order tabs by stable labels
const sortedFeatures = sortBy(features, ["properties.label"]);
Expand Down Expand Up @@ -152,11 +161,7 @@ const VerticalFeatureTabs: React.FC<{ features: Feature[] }> = ({
formik={formik}
/>
<Button
onClick={() =>
console.log(
`TODO - Remove ${schema.type} ${feature.properties?.label}`,
)
}
onClick={() => removeFeature(activeIndex)}
sx={{
fontWeight: FONT_WEIGHT_SEMI_BOLD,
gap: (theme) => theme.spacing(2),
Expand Down Expand Up @@ -192,12 +197,13 @@ const PlotFeatureToBegin = () => (

const Root = () => {
const {
validateAndSubmitForm,
mapAndLabelProps,
errors,
addFeature,
features,
mapAndLabelProps,
schema,
formik,
updateMapKey,
validateAndSubmitForm,
addInitialFeaturesToMap,
} = useMapAndLabelContext();
const {
title,
Expand All @@ -216,36 +222,11 @@ const Root = () => {
previouslySubmittedData,
} = mapAndLabelProps;

const previousFeatures = previouslySubmittedData?.data?.[
fn
] as FeatureCollection;
const [features, setFeatures] = useState<Feature[] | undefined>(
previousFeatures?.features?.length > 0
? previousFeatures.features
: undefined,
);

useEffect(() => {
const geojsonChangeHandler = ({ detail: geojson }: any) => {
if (geojson["EPSG:3857"]?.features) {
setFeatures(geojson["EPSG:3857"].features);
formik.setFieldValue("geoData", geojson["EPSG:3857"].features);
addFeature();
} else {
// if the user clicks 'reset' on the map, geojson will be empty object, so set features to undefined
setFeatures(undefined);
formik.setFieldValue("geoData", undefined);
}
};

const map: HTMLElement | null =
document.getElementById("map-and-label-map");
map?.addEventListener("geojsonChange", geojsonChangeHandler);

return function cleanup() {
map?.removeEventListener("geojsonChange", geojsonChangeHandler);
};
}, [setFeatures, addFeature]);
// If coming "back" or "changing", load initial features & tabs onto the map
// Pre-populating form fields within tabs is handled via formik.initialValues in Context.tsx
if (previouslySubmittedData?.data?.[fn]?.features?.length > 0) {
addInitialFeaturesToMap(previouslySubmittedData?.data?.[fn]?.features);
}

const rootError: string =
(errors.min &&
Expand All @@ -264,12 +245,12 @@ const Root = () => {
howMeasured={howMeasured}
/>
<FullWidthWrapper>
<ErrorWrapper error={rootError}>
<ErrorWrapper error={rootError} key={updateMapKey}>
<MapContainer environment="standalone">
{/* @ts-ignore */}
<my-map
id="map-and-label-map"
data-testid="map-and-label-map"
id={MAP_ID}
data-testid={MAP_ID}
basemap={basemap}
ariaLabelOlFixedOverlay={`An interactive map for plotting and describing individual ${schemaName.toLocaleLowerCase()}`}
drawMode
Expand Down Expand Up @@ -303,7 +284,7 @@ const Root = () => {
</MapContainer>
</ErrorWrapper>
{features && features?.length > 0 ? (
<VerticalFeatureTabs features={features} />
<VerticalFeatureTabs />
) : (
<PlotFeatureToBegin />
)}
Expand Down
1 change: 0 additions & 1 deletion editor.planx.uk/src/@planx/components/shared/Schema/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ export type SchemaUserResponse = Record<
*/
export type SchemaUserData = {
schemaData: SchemaUserResponse[];
geoData?: Feature[];
};

/**
Expand Down
Loading
Loading