From 3ab268c932926596e68ac2d21e6c71dab8e0bd0b Mon Sep 17 00:00:00 2001 From: Rory Doak Date: Fri, 18 Oct 2024 12:57:32 +0100 Subject: [PATCH 1/4] fix activeIndex bug and previous data fn --- .../components/MapAndLabel/Public/Context.tsx | 8 +- .../MapAndLabel/Public/index.bugTest.test.tsx | 80 +++++++++++++++++++ .../components/MapAndLabel/Public/index.tsx | 3 +- .../MapAndLabel/test/mocks/Trees.ts | 14 ++++ .../MapAndLabel/test/mocks/mockPayload.tsx | 47 ++++++++++- 5 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.bugTest.test.tsx diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx index a26e98c991..b0086728d1 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx @@ -99,7 +99,7 @@ export const MapAndLabelProvider: React.FC = ( }); const [activeIndex, setActiveIndex] = useState( - previousFormData?.length - 1 || -1, + previousFormData?.length > 1 ? previousFormData?.length - 1 : 0, ); const [minError, setMinError] = useState(false); @@ -179,8 +179,10 @@ export const MapAndLabelProvider: React.FC = ( setActiveIndex(newFeatures.length - 1); }; - const addInitialFeaturesToMap = (features: Feature[]) => { - setFeatures(features); + const addInitialFeaturesToMap = (initialFeatures: Feature[]) => { + if (!features?.length) { + setFeatures(initialFeatures); + } }; const addFeatureToForm = () => { diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.bugTest.test.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.bugTest.test.tsx new file mode 100644 index 0000000000..133a86acd6 --- /dev/null +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.bugTest.test.tsx @@ -0,0 +1,80 @@ +import { MyMap } from "@opensystemslab/map"; +import React from "react"; +import { setup } from "testUtils"; +import { it, vi } from "vitest"; + +import { point1, point2, point3 } from "../test/mocks/geojson"; +import { + previousDoubleDataProps, + previousSingleDataProps, +} from "../test/mocks/Trees"; +import { addFeaturesToMap, addMultipleFeatures } from "../test/utils"; +import { Presentational as MapAndLabel } from "."; + +beforeAll(() => { + if (!window.customElements.get("my-map")) { + window.customElements.define("my-map", MyMap); + } + + const ResizeObserverMock = vi.fn(() => ({ + observe: vi.fn(), + unobserve: vi.fn(), + disconnect: vi.fn(), + })); + + vi.stubGlobal("ResizeObserver", ResizeObserverMock); +}); + +describe("navigating back after adding single feature", () => { + it("shows previously submitted features", async () => { + const { getByTestId, queryByRole, user } = setup( + , + ); + const map = getByTestId("map-and-label-map"); + + expect(map).toBeInTheDocument(); + + const firstTab = queryByRole("tab", { name: /Tree 1/ }); + expect(firstTab).toBeInTheDocument(); + expect(firstTab).toBeVisible(); + + const firstTabPanel = getByTestId("vertical-tabpanel-0"); + expect(firstTabPanel).toBeInTheDocument(); + expect(firstTabPanel).toBeVisible(); + + // point1 here needs to be reflected in the props you pass in at the start so labels match + addMultipleFeatures([point1, point2]); + + const secondTab = queryByRole("tab", { name: /Tree 2/ }); + expect(secondTab).toBeInTheDocument(); + expect(secondTab).toBeVisible(); + }); +}); + +describe("navigating back after adding two features", () => { + it("shows previously submitted features", async () => { + const { getByTestId, queryByRole, user } = setup( + , + ); + const map = getByTestId("map-and-label-map"); + + expect(map).toBeInTheDocument(); + + const firstTab = queryByRole("tab", { name: /Tree 1/ }); + const secondTab = queryByRole("tab", { name: /Tree 2/ }); + expect(firstTab).toBeInTheDocument(); + expect(secondTab).toBeInTheDocument(); + + const firstTabPanel = getByTestId("vertical-tabpanel-0"); + const secondTabPanel = getByTestId("vertical-tabpanel-1"); + expect(firstTabPanel).toBeInTheDocument(); + expect(secondTabPanel).toBeInTheDocument(); + expect(secondTabPanel).toBeVisible(); + + addMultipleFeatures([point1, point2, point3]); + + const thirdTab = queryByRole("tab", { name: /Tree 3/ }); + expect(thirdTab).toBeInTheDocument(); + expect(thirdTab).toBeVisible(); + }); +}); diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.tsx index f582ad3ec2..766d19ff0c 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.tsx @@ -11,9 +11,10 @@ import { SiteAddress } from "@planx/components/FindProperty/model"; import { SchemaFields } from "@planx/components/shared/Schema/SchemaFields"; import { GraphError } from "components/Error/GraphError"; import { GeoJsonObject } from "geojson"; +import { Feature } from "geojson"; import sortBy from "lodash/sortBy"; import { useStore } from "pages/FlowEditor/lib/store"; -import React from "react"; +import React, { useEffect, useState } from "react"; import { FONT_WEIGHT_SEMI_BOLD } from "theme"; import FullWidthWrapper from "ui/public/FullWidthWrapper"; import ErrorWrapper from "ui/shared/ErrorWrapper"; diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/Trees.ts b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/Trees.ts index db502de886..8d79b1ad1f 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/Trees.ts +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/Trees.ts @@ -2,6 +2,10 @@ import { Schema } from "@planx/components/shared/Schema/model"; import { PresentationalProps } from "../../Public"; import { Trees } from "../../schemas/Trees"; +import { + previouslySubmittedDoubleData, + previouslySubmittedSingleData, +} from "./mockPayload"; const mockTreeSchema: Schema = { ...Trees, @@ -20,3 +24,13 @@ export const props: PresentationalProps = { longitude: -0.1629784, latitude: 51.5230919, }; + +export const previousSingleDataProps: PresentationalProps = { + ...props, + previouslySubmittedData: previouslySubmittedSingleData, +}; + +export const previousDoubleDataProps: PresentationalProps = { + ...props, + previouslySubmittedData: previouslySubmittedDoubleData, +}; diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/mockPayload.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/mockPayload.tsx index 6395d5453b..fa12d5dc6f 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/mockPayload.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/mockPayload.tsx @@ -1,6 +1,13 @@ import { FeatureCollection } from "geojson"; + import { mockTreeData } from "./GenericValues"; -export type MockPayload = { data: { MockFn: FeatureCollection } }; +export interface MockPayload { + data: { MockFn: FeatureCollection }; +} + +interface PreviousData extends MockPayload { + auto: boolean; +} export const mockSingleFeaturePayload: MockPayload = { data: { @@ -11,11 +18,47 @@ export const mockSingleFeaturePayload: MockPayload = { coordinates: [-3.685929607119201, 57.15301433687542], type: "Point", }, - properties: { mockTreeData }, + properties: { mockTreeData, label: "1" }, + type: "Feature", + }, + ], + type: "FeatureCollection", + }, + }, +}; + +export const previousMockDoubleFeatureData: MockPayload = { + data: { + MockFn: { + features: [ + { + geometry: { + coordinates: [-3.685929607119201, 57.15301433687542], + type: "Point", + }, + properties: { mockTreeData, label: "1" }, + type: "Feature", + }, + { type: "Feature", + properties: { ...mockTreeData, label: "2" }, + geometry: { + type: "Point", + coordinates: [-3.686529607119201, 57.15310433687542], + }, }, ], type: "FeatureCollection", }, }, }; + +export const previouslySubmittedSingleData: PreviousData = { + auto: false, + ...mockSingleFeaturePayload, +}; + +export const previouslySubmittedDoubleData: PreviousData = { + auto: false, + ...previousMockDoubleFeatureData, +}; From 407e57e98d3922360504f59aa84de993b3356fa1 Mon Sep 17 00:00:00 2001 From: Rory Doak Date: Fri, 18 Oct 2024 16:20:01 +0100 Subject: [PATCH 2/4] fix: label bug when copy - continue - back move test files to test folder tidy test expects fix: remove unused imports --- .../components/MapAndLabel/Public/Context.tsx | 18 +++++++++++++----- .../components/MapAndLabel/Public/index.tsx | 5 ++--- .../public.backNavigation.test.tsx} | 19 ++++++------------- .../index.test.tsx => test/public.test.tsx} | 13 ++++--------- 4 files changed, 25 insertions(+), 30 deletions(-) rename editor.planx.uk/src/@planx/components/MapAndLabel/{Public/index.bugTest.test.tsx => test/public.backNavigation.test.tsx} (75%) rename editor.planx.uk/src/@planx/components/MapAndLabel/{Public/index.test.tsx => test/public.test.tsx} (98%) diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx index b0086728d1..2f023e69a2 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx @@ -99,11 +99,14 @@ export const MapAndLabelProvider: React.FC = ( }); const [activeIndex, setActiveIndex] = useState( - previousFormData?.length > 1 ? previousFormData?.length - 1 : 0, + previousFormData ? previousFormData?.length - 1 : 0, ); const [minError, setMinError] = useState(false); const [maxError, setMaxError] = useState(false); + const [loadPreviousValues, setLoadPreviousValues] = useState( + Boolean(previouslySubmittedData), + ); const handleGeoJSONChange = (event: GeoJSONChangeEvent) => { // If the user clicks 'reset' on the map, geojson will be empty object @@ -147,7 +150,9 @@ export const MapAndLabelProvider: React.FC = ( formik.setErrors({}); }; - const removeAllFeaturesFromMap = () => setFeatures(undefined); + const removeAllFeaturesFromMap = () => { + setFeatures(undefined); + }; const removeAllFeaturesFromForm = () => { formik.setFieldValue("schemaData", []); @@ -180,8 +185,9 @@ export const MapAndLabelProvider: React.FC = ( }; const addInitialFeaturesToMap = (initialFeatures: Feature[]) => { - if (!features?.length) { + if (loadPreviousValues) { setFeatures(initialFeatures); + setLoadPreviousValues(false); } }; @@ -200,7 +206,10 @@ export const MapAndLabelProvider: React.FC = ( const copyFeature = (sourceIndex: number, destinationIndex: number) => { const sourceFeature = formik.values.schemaData[sourceIndex]; - formik.setFieldValue(`schemaData[${destinationIndex}]`, sourceFeature); + formik.setFieldValue(`schemaData[${destinationIndex}]`, { + ...sourceFeature, + label: `${destinationIndex + 1}`, + }); }; const removeFeatureFromForm = (index: number) => { @@ -238,7 +247,6 @@ export const MapAndLabelProvider: React.FC = ( // 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) || 0); }; - return ( { const passport = useStore((state) => state.computePassport().data); // 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 + // 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); } diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.bugTest.test.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/test/public.backNavigation.test.tsx similarity index 75% rename from editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.bugTest.test.tsx rename to editor.planx.uk/src/@planx/components/MapAndLabel/test/public.backNavigation.test.tsx index 133a86acd6..5e7b52db90 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.bugTest.test.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/test/public.backNavigation.test.tsx @@ -3,13 +3,13 @@ import React from "react"; import { setup } from "testUtils"; import { it, vi } from "vitest"; -import { point1, point2, point3 } from "../test/mocks/geojson"; +import { Presentational as MapAndLabel } from "../Public"; +import { point1, point2, point3 } from "./mocks/geojson"; import { previousDoubleDataProps, previousSingleDataProps, -} from "../test/mocks/Trees"; -import { addFeaturesToMap, addMultipleFeatures } from "../test/utils"; -import { Presentational as MapAndLabel } from "."; +} from "./mocks/Trees"; +import { addMultipleFeatures } from "./utils"; beforeAll(() => { if (!window.customElements.get("my-map")) { @@ -27,7 +27,7 @@ beforeAll(() => { describe("navigating back after adding single feature", () => { it("shows previously submitted features", async () => { - const { getByTestId, queryByRole, user } = setup( + const { getByTestId, queryByRole } = setup( , ); const map = getByTestId("map-and-label-map"); @@ -35,25 +35,22 @@ describe("navigating back after adding single feature", () => { expect(map).toBeInTheDocument(); const firstTab = queryByRole("tab", { name: /Tree 1/ }); - expect(firstTab).toBeInTheDocument(); expect(firstTab).toBeVisible(); const firstTabPanel = getByTestId("vertical-tabpanel-0"); - expect(firstTabPanel).toBeInTheDocument(); expect(firstTabPanel).toBeVisible(); // point1 here needs to be reflected in the props you pass in at the start so labels match addMultipleFeatures([point1, point2]); const secondTab = queryByRole("tab", { name: /Tree 2/ }); - expect(secondTab).toBeInTheDocument(); expect(secondTab).toBeVisible(); }); }); describe("navigating back after adding two features", () => { it("shows previously submitted features", async () => { - const { getByTestId, queryByRole, user } = setup( + const { getByTestId, queryByRole } = setup( , ); const map = getByTestId("map-and-label-map"); @@ -65,16 +62,12 @@ describe("navigating back after adding two features", () => { expect(firstTab).toBeInTheDocument(); expect(secondTab).toBeInTheDocument(); - const firstTabPanel = getByTestId("vertical-tabpanel-0"); const secondTabPanel = getByTestId("vertical-tabpanel-1"); - expect(firstTabPanel).toBeInTheDocument(); - expect(secondTabPanel).toBeInTheDocument(); expect(secondTabPanel).toBeVisible(); addMultipleFeatures([point1, point2, point3]); const thirdTab = queryByRole("tab", { name: /Tree 3/ }); - expect(thirdTab).toBeInTheDocument(); expect(thirdTab).toBeVisible(); }); }); diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.test.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/test/public.test.tsx similarity index 98% rename from editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.test.tsx rename to editor.planx.uk/src/@planx/components/MapAndLabel/test/public.test.tsx index 8b363f01b1..afd7ad8603 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/index.test.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/test/public.test.tsx @@ -6,14 +6,9 @@ import { setup } from "testUtils"; import { vi } from "vitest"; import { axe } from "vitest-axe"; -import { mockTreeData } from "../test/mocks/GenericValues"; -import { - mockFeaturePointObj, - point1, - point2, - point3, -} from "../test/mocks/geojson"; -import { props } from "../test/mocks/Trees"; +import { mockTreeData } from "./mocks/GenericValues"; +import { mockFeaturePointObj, point1, point2, point3 } from "./mocks/geojson"; +import { props } from "./mocks/Trees"; import { addFeaturesToMap, addMultipleFeatures, @@ -23,7 +18,7 @@ import { fillOutFirstHalfOfForm, fillOutForm, fillOutSecondHalfOfForm, -} from "../test/utils"; +} from "./utils"; beforeAll(() => { if (!window.customElements.get("my-map")) { From 960c1eb20cec8163674bbdc47c04965c81e401c8 Mon Sep 17 00:00:00 2001 From: Rory Doak Date: Tue, 22 Oct 2024 13:30:54 +0100 Subject: [PATCH 3/4] refactor: new test for label bug and naming refinement refine naming of test mocks remove test.only --- .../components/MapAndLabel/Public/Context.tsx | 1 + .../MapAndLabel/test/mocks/GenericValues.ts | 2 +- .../MapAndLabel/test/mocks/Trees.ts | 12 +++--- .../MapAndLabel/test/mocks/mockPayload.tsx | 8 ++-- .../test/public.backNavigation.test.tsx | 43 ++++++++++++++++--- 5 files changed, 49 insertions(+), 17 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx index 2f023e69a2..4c39177df2 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx @@ -83,6 +83,7 @@ export const MapAndLabelProvider: React.FC = ( const mergedProperties = { ...feature.properties, ...values.schemaData[i], + label: `${i + 1}`, }; feature["properties"] = mergedProperties; geojson.features.push(feature); diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/GenericValues.ts b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/GenericValues.ts index 16c3737b3c..1ee9ab0758 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/GenericValues.ts +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/GenericValues.ts @@ -2,7 +2,7 @@ export type TreeData = { species: string; work: string; justification: string; - urgency: "low" | "moderate" | "high" | "urgenct"; + urgency: "low" | "moderate" | "high" | "urgent"; label: string; }; diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/Trees.ts b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/Trees.ts index 8d79b1ad1f..abebddd4c4 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/Trees.ts +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/Trees.ts @@ -3,8 +3,8 @@ import { Schema } from "@planx/components/shared/Schema/model"; import { PresentationalProps } from "../../Public"; import { Trees } from "../../schemas/Trees"; import { - previouslySubmittedDoubleData, - previouslySubmittedSingleData, + previouslySubmittedDoubleFeature, + previouslySubmittedSingleFeature, } from "./mockPayload"; const mockTreeSchema: Schema = { @@ -25,12 +25,12 @@ export const props: PresentationalProps = { latitude: 51.5230919, }; -export const previousSingleDataProps: PresentationalProps = { +export const previouslySubmittedSingleFeatureProps: PresentationalProps = { ...props, - previouslySubmittedData: previouslySubmittedSingleData, + previouslySubmittedData: previouslySubmittedSingleFeature, }; -export const previousDoubleDataProps: PresentationalProps = { +export const previouslySubmittedDoubleFeatureProps: PresentationalProps = { ...props, - previouslySubmittedData: previouslySubmittedDoubleData, + previouslySubmittedData: previouslySubmittedDoubleFeature, }; diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/mockPayload.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/mockPayload.tsx index fa12d5dc6f..036e7bc719 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/mockPayload.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/test/mocks/mockPayload.tsx @@ -27,7 +27,7 @@ export const mockSingleFeaturePayload: MockPayload = { }, }; -export const previousMockDoubleFeatureData: MockPayload = { +export const mockDoubleFeaturePayload: MockPayload = { data: { MockFn: { features: [ @@ -53,12 +53,12 @@ export const previousMockDoubleFeatureData: MockPayload = { }, }; -export const previouslySubmittedSingleData: PreviousData = { +export const previouslySubmittedSingleFeature: PreviousData = { auto: false, ...mockSingleFeaturePayload, }; -export const previouslySubmittedDoubleData: PreviousData = { +export const previouslySubmittedDoubleFeature: PreviousData = { auto: false, - ...previousMockDoubleFeatureData, + ...mockDoubleFeaturePayload, }; diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/test/public.backNavigation.test.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/test/public.backNavigation.test.tsx index 5e7b52db90..f84e90468a 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/test/public.backNavigation.test.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/test/public.backNavigation.test.tsx @@ -6,10 +6,10 @@ import { it, vi } from "vitest"; import { Presentational as MapAndLabel } from "../Public"; import { point1, point2, point3 } from "./mocks/geojson"; import { - previousDoubleDataProps, - previousSingleDataProps, + previouslySubmittedDoubleFeatureProps, + previouslySubmittedSingleFeatureProps, } from "./mocks/Trees"; -import { addMultipleFeatures } from "./utils"; +import { addMultipleFeatures, clickContinue } from "./utils"; beforeAll(() => { if (!window.customElements.get("my-map")) { @@ -28,7 +28,7 @@ beforeAll(() => { describe("navigating back after adding single feature", () => { it("shows previously submitted features", async () => { const { getByTestId, queryByRole } = setup( - , + ); const map = getByTestId("map-and-label-map"); @@ -40,18 +40,21 @@ describe("navigating back after adding single feature", () => { const firstTabPanel = getByTestId("vertical-tabpanel-0"); expect(firstTabPanel).toBeVisible(); - // point1 here needs to be reflected in the props you pass in at the start so labels match + // To properly add the features to the map, you need to pass all previous features as well addMultipleFeatures([point1, point2]); const secondTab = queryByRole("tab", { name: /Tree 2/ }); expect(secondTab).toBeVisible(); + + const secondTabPanel = getByTestId("vertical-tabpanel-1"); + expect(secondTabPanel).toBeVisible(); }); }); describe("navigating back after adding two features", () => { it("shows previously submitted features", async () => { const { getByTestId, queryByRole } = setup( - , + ); const map = getByTestId("map-and-label-map"); @@ -65,9 +68,37 @@ describe("navigating back after adding two features", () => { const secondTabPanel = getByTestId("vertical-tabpanel-1"); expect(secondTabPanel).toBeVisible(); + // To properly add the features to the map, you need to pass all previous features as well addMultipleFeatures([point1, point2, point3]); const thirdTab = queryByRole("tab", { name: /Tree 3/ }); expect(thirdTab).toBeVisible(); + + const thirdTabPanel = getByTestId("vertical-tabpanel-2"); + expect(thirdTabPanel).toBeVisible(); + }); + it("should maintain labelling when removing a feature", async () => { + const handleSubmit = vi.fn(); + const { getByRole, user } = setup( + + ); + + const firstTab = getByRole("tab", { name: /Tree 1/ }); + + await user.click(firstTab); + + const removeButton = getByRole("button", { name: /remove/i }); + + await user.click(removeButton); + + await clickContinue(user); + + const output = + handleSubmit.mock.calls[0][0].data.MockFn.features[0].properties.label; + + expect(output).toEqual("1"); }); }); From c9e05e2d524f604569cc37bce68c491094a1c0fb Mon Sep 17 00:00:00 2001 From: Rory Doak Date: Tue, 22 Oct 2024 15:32:30 +0100 Subject: [PATCH 4/4] refine state variable naming --- .../components/MapAndLabel/Public/Context.tsx | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx index 4c39177df2..2cfa03302b 100644 --- a/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx +++ b/editor.planx.uk/src/@planx/components/MapAndLabel/Public/Context.tsx @@ -45,11 +45,11 @@ interface MapAndLabelContextValue { type MapAndLabelProviderProps = PropsWithChildren; const MapAndLabelContext = createContext( - undefined, + undefined ); export const MapAndLabelProvider: React.FC = ( - props, + props ) => { const { schema, children, handleSubmit, previouslySubmittedData, fn } = props; const { formikConfig, initialValues } = useSchema({ @@ -62,7 +62,7 @@ export const MapAndLabelProvider: React.FC = ( fn ] as FeatureCollection; const previousFormData = previousGeojson?.features.map( - (feature) => feature.properties, + (feature) => feature.properties ) as SchemaUserResponse[]; const _previousMapData = previousGeojson?.features; @@ -100,14 +100,13 @@ export const MapAndLabelProvider: React.FC = ( }); const [activeIndex, setActiveIndex] = useState( - previousFormData ? previousFormData?.length - 1 : 0, + previousFormData ? previousFormData?.length - 1 : 0 ); const [minError, setMinError] = useState(false); const [maxError, setMaxError] = useState(false); - const [loadPreviousValues, setLoadPreviousValues] = useState( - Boolean(previouslySubmittedData), - ); + const [loadPreviousValuesOnMap, setLoadPreviousValuesOnMap] = + useState(Boolean(previouslySubmittedData)); const handleGeoJSONChange = (event: GeoJSONChangeEvent) => { // If the user clicks 'reset' on the map, geojson will be empty object @@ -186,9 +185,9 @@ export const MapAndLabelProvider: React.FC = ( }; const addInitialFeaturesToMap = (initialFeatures: Feature[]) => { - if (loadPreviousValues) { + if (loadPreviousValuesOnMap) { setFeatures(initialFeatures); - setLoadPreviousValues(false); + setLoadPreviousValuesOnMap(false); } }; @@ -216,7 +215,7 @@ export const MapAndLabelProvider: React.FC = ( const removeFeatureFromForm = (index: number) => { formik.setFieldValue( "schemaData", - formik.values.schemaData.filter((_, i) => i !== index), + formik.values.schemaData.filter((_, i) => i !== index) ); }; @@ -224,7 +223,7 @@ export const MapAndLabelProvider: React.FC = ( // 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, + (f) => f.properties?.label !== label ); // Shift any feature labels that are larger than the removed feature label so they remain incremental @@ -278,7 +277,7 @@ export const useMapAndLabelContext = (): MapAndLabelContextValue => { const context = useContext(MapAndLabelContext); if (!context) { throw new Error( - "useMapAndLabelContext must be used within a MapAndLabelProvider", + "useMapAndLabelContext must be used within a MapAndLabelProvider" ); } return context;