From c31e2fc048032c9c9486bc159ca6897772bda40b Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Tue, 11 Jun 2024 13:16:59 +0200 Subject: [PATCH 1/2] display conditions for schema max 1, reorg test schemas --- .../src/@planx/components/List/Editor.tsx | 6 +- .../components/List/Public/index.test.tsx | 144 +++++------------- .../@planx/components/List/Public/index.tsx | 54 ++++--- .../List/schemas/GenericUnitsTest.ts | 51 ------- .../List/schemas/Tests/GenericUnits.ts | 95 ++++++++++++ .../components/List/schemas/Tests/MaxOne.ts | 58 +++++++ .../List/schemas/{ => Tests}/Zoo.ts | 43 +++++- 7 files changed, 271 insertions(+), 180 deletions(-) delete mode 100644 editor.planx.uk/src/@planx/components/List/schemas/GenericUnitsTest.ts create mode 100644 editor.planx.uk/src/@planx/components/List/schemas/Tests/GenericUnits.ts create mode 100644 editor.planx.uk/src/@planx/components/List/schemas/Tests/MaxOne.ts rename editor.planx.uk/src/@planx/components/List/schemas/{ => Tests}/Zoo.ts (58%) diff --git a/editor.planx.uk/src/@planx/components/List/Editor.tsx b/editor.planx.uk/src/@planx/components/List/Editor.tsx index 8f96df6187..db4a9b005d 100644 --- a/editor.planx.uk/src/@planx/components/List/Editor.tsx +++ b/editor.planx.uk/src/@planx/components/List/Editor.tsx @@ -13,14 +13,14 @@ import InputRowLabel from "ui/shared/InputRowLabel"; import { EditorProps, ICONS, InternalNotes, MoreInformation } from "../ui"; import { List, parseContent } from "./model"; +import { ProposedAdvertisements } from "./schemas/Adverts"; import { ResidentialUnitsExisting } from "./schemas/ResidentialUnits/Existing"; 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 { ResidentialUnitsProposed } from "./schemas/ResidentialUnits/Proposed"; -import { Zoo } from "./schemas/Zoo"; -import { ProposedAdvertisements } from "./schemas/Adverts"; +import { Zoo } from "./schemas/Tests/Zoo"; type Props = EditorProps; @@ -40,8 +40,8 @@ export const SCHEMAS = [ name: "Residential units (GLA) - Retained", schema: ResidentialUnitsGLARetained, }, - { name: "Zoo (test)", schema: Zoo }, { name: "Proposed advertisements", schema: ProposedAdvertisements }, + { name: "Zoo (test)", schema: Zoo }, ]; function ListComponent(props: Props) { diff --git a/editor.planx.uk/src/@planx/components/List/Public/index.test.tsx b/editor.planx.uk/src/@planx/components/List/Public/index.test.tsx index 2f72a1e8f9..ddb2f88b0f 100644 --- a/editor.planx.uk/src/@planx/components/List/Public/index.test.tsx +++ b/editor.planx.uk/src/@planx/components/List/Public/index.test.tsx @@ -1,106 +1,22 @@ -import { screen, within } from "@testing-library/react"; +import { getByText, screen, within } from "@testing-library/react"; import { UserEvent } from "@testing-library/user-event/dist/types/setup/setup"; import { cloneDeep, merge } from "lodash"; import React from "react"; import { axe, setup } from "testUtils"; -import { UserResponse } from "../model"; -import ListComponent, { Props } from "../Public"; -import { GenericUnitsTest } from "../schemas/GenericUnitsTest"; -import { Zoo } from "../schemas/Zoo"; +import ListComponent from "../Public"; import { - flatten, - sumIdenticalUnits, - sumIdenticalUnitsByDevelopmentType, -} from "../utils"; - -const mockProps: Props = { - fn: "mockFn", - schema: Zoo, - schemaName: "Zoo", - title: "Mock Title", - description: "Mock description", -}; - -const mockPayload = { - data: { - mockFn: [ - { - age: 10, - cuteness: "Very", - email: "richard.parker@pi.com", - name: "Richard Parker", - size: "Medium", - }, - { - age: 10, - cuteness: "Very", - email: "richard.parker@pi.com", - name: "Richard Parker", - size: "Medium", - }, - ], - "mockFn.one.age": 10, - "mockFn.one.cuteness": "Very", - "mockFn.one.email": "richard.parker@pi.com", - "mockFn.one.name": "Richard Parker", - "mockFn.one.size": "Medium", - "mockFn.two.age": 10, - "mockFn.two.cuteness": "Very", - "mockFn.two.email": "richard.parker@pi.com", - "mockFn.two.name": "Richard Parker", - "mockFn.two.size": "Medium", - "mockFn.total.listItems": 2, - }, -}; - -const mockPropsUnits: Props = { - fn: "proposal.units.residential", - schema: GenericUnitsTest, - schemaName: "Generic residential units", - title: "Describe residential units", -}; - -const mockPayloadUnits = { - data: { - "proposal.units.residential": [ - { - development: "newBuild", - garden: "Yes", - identicalUnits: 1, - }, - { - development: "newBuild", - garden: "No", - identicalUnits: 2, - }, - { - development: "changeOfUseTo", - garden: "No", - identicalUnits: 2, - }, - ], - "proposal.units.residential.one.development": "newBuild", - "proposal.units.residential.one.garden": "Yes", - "proposal.units.residential.one.identicalUnits": 1, - "proposal.units.residential.two.development": "newBuild", - "proposal.units.residential.two.garden": "No", - "proposal.units.residential.two.identicalUnits": 2, - "proposal.units.residential.three.development": "changeOfUseTo", - "proposal.units.residential.three.garden": "No", - "proposal.units.residential.three.identicalUnits": 2, - "proposal.units.residential.total.listItems": 3, - "proposal.units.residential.total.units": 5, - "proposal.units.residential.total.units.newBuid": 3, - "proposal.units.residential.total.units.changeOfUseTo": 2, - }, -}; + mockUnitsPayload, + mockUnitsProps, +} from "../schemas/Tests/GenericUnits"; +import { mockMaxOneProps } from "../schemas/Tests/MaxOne"; +import { mockZooPayload, mockZooProps } from "../schemas/Tests/Zoo"; jest.setTimeout(20_000); describe("Basic UI", () => { it("renders correctly", () => { - const { getByText } = setup(); + const { getByText } = setup(); expect(getByText(/Mock Title/)).toBeInTheDocument(); expect(getByText(/Mock description/)).toBeInTheDocument(); @@ -108,7 +24,7 @@ describe("Basic UI", () => { it("parses provided schema to render expected form", async () => { const { getByLabelText, getByText, user, getByRole, queryAllByRole } = - setup(); + setup(); // Text inputs are generated from schema... const textInput = getByLabelText(/What's their name?/) as HTMLInputElement; @@ -174,7 +90,7 @@ describe("Basic UI", () => { }); it("should not have any accessibility violations", async () => { - const { container } = setup(); + const { container } = setup(); const results = await axe(container); expect(results).toHaveNoViolations(); }); @@ -182,7 +98,9 @@ describe("Basic UI", () => { describe("Building a list", () => { it("does not display a default item if the schema has no required minimum", () => { - const mockWithMinZero = merge(cloneDeep(mockProps), { schema: { min: 0 } }); + const mockWithMinZero = merge(cloneDeep(mockZooProps), { + schema: { min: 0 }, + }); const { queryByRole, getByTestId } = setup( , ); @@ -202,7 +120,7 @@ describe("Building a list", () => { it("displays a default item if the schema has a required minimum", () => { const { getByRole, queryByLabelText } = setup( - , + , ); // Card present... @@ -218,9 +136,22 @@ describe("Building a list", () => { expect(inputField).not.toBeDisabled(); }); + it("hides the index number in the card header and the 'add another' button if the schema has a max of 1", () => { + const { getAllByTestId, queryByTestId } = setup( + , + ); + + const cards = getAllByTestId(/list-card/); + expect(cards).toHaveLength(1); + expect(cards[0]).toHaveTextContent("Parking spaces"); + + const addItemButton = queryByTestId("list-add-button"); + expect(addItemButton).not.toBeInTheDocument(); + }); + test("Adding an item", async () => { const { getAllByTestId, getByTestId, user } = setup( - , + , ); let cards = getAllByTestId(/list-card/); @@ -252,7 +183,7 @@ describe("Building a list", () => { test("Editing an item", async () => { // Setup three cards const { getAllByTestId, getByTestId, user } = setup( - , + , ); await fillInResponse(user); @@ -308,7 +239,7 @@ describe("Building a list", () => { user, getByLabelText, queryAllByTestId, - } = setup(); + } = setup(); await fillInResponse(user); @@ -378,7 +309,7 @@ describe("Building a list", () => { test("Removing an item when another card is active", async () => { // Setup two cards const { getAllByTestId, getByTestId, user } = setup( - , + , ); await fillInResponse(user); @@ -430,7 +361,7 @@ describe("Payload generation", () => { it("generates a valid payload on submission (Zoo)", async () => { const handleSubmit = jest.fn(); const { getByTestId, user } = setup( - , + , ); const addItemButton = getByTestId("list-add-button"); @@ -442,13 +373,13 @@ describe("Payload generation", () => { await user.click(screen.getByTestId("continue-button")); expect(handleSubmit).toHaveBeenCalled(); - expect(handleSubmit.mock.calls[0][0]).toMatchObject(mockPayload); + expect(handleSubmit.mock.calls[0][0]).toMatchObject(mockZooPayload); }); it.skip("generates a valid payload with summary stats on submission (Units)", async () => { const handleSubmit = jest.fn(); const { getByTestId, user } = setup( - , + , ); const saveButton = screen.getByRole("button", { name: /Save/ }); @@ -486,14 +417,17 @@ describe("Payload generation", () => { await user.click(screen.getByTestId("continue-button")); expect(handleSubmit).toHaveBeenCalled(); - expect(handleSubmit.mock.calls[0][0]).toMatchObject(mockPayloadUnits); + expect(handleSubmit.mock.calls[0][0]).toMatchObject(mockUnitsPayload); }); }); describe("Navigating back", () => { test("it pre-populates list correctly", async () => { const { getAllByText, queryByLabelText, getAllByTestId } = setup( - , + , ); const cards = getAllByTestId(/list-card/); diff --git a/editor.planx.uk/src/@planx/components/List/Public/index.tsx b/editor.planx.uk/src/@planx/components/List/Public/index.tsx index 6466cfa70a..2b72f210cb 100644 --- a/editor.planx.uk/src/@planx/components/List/Public/index.tsx +++ b/editor.planx.uk/src/@planx/components/List/Public/index.tsx @@ -64,16 +64,20 @@ const InputField: React.FC = (props) => { const ActiveListCard: React.FC<{ index: number; -}> = ({ index }) => { +}> = ({ index: i }) => { const { schema, saveItem, cancelEditItem, errors } = useListContext(); + // Hide the index number in the card title if the schema has a max length of 1 + const shouldHideIndexTitle = schema.max !== 1; + return ( - + - {schema.type} {index + 1} + {schema.type} + {shouldHideIndexTitle && ` ${i + 1}`} {schema.fields.map((field, i) => ( @@ -100,10 +104,14 @@ const InactiveListCard: React.FC<{ }> = ({ index: i }) => { const { schema, formik, removeItem, editItem } = useListContext(); + // Hide the index number in the card title if the schema has a max length of 1 + const shouldHideIndexTitle = schema.max !== 1; + return ( - {schema.type} {i + 1} + {schema.type} + {shouldHideIndexTitle && ` ${i + 1}`} @@ -155,6 +163,10 @@ const Root = () => { (errors.max && `You can provide at most ${schema.max} response(s)`) || ""; + // Hide the "+ Add another" button if the schema has a max length of 1, unless the only item has been cancelled/removed + const shouldShowAddAnotherButton = + schema.max !== 1 || formik.values.userData.length < 1; + return ( { ), )} - - - + + + )} diff --git a/editor.planx.uk/src/@planx/components/List/schemas/GenericUnitsTest.ts b/editor.planx.uk/src/@planx/components/List/schemas/GenericUnitsTest.ts deleted file mode 100644 index f8f7d45076..0000000000 --- a/editor.planx.uk/src/@planx/components/List/schemas/GenericUnitsTest.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { Schema } from "@planx/components/List/model"; - -export const GenericUnitsTest: Schema = { - type: "Unit", - fields: [ - // fn = "development" triggers summary stat and options set "val" - { - type: "question", - data: { - title: "What development does this unit result from?", - fn: "development", - options: [ - { id: "newBuild", data: { text: "New build", val: "newBuild" } }, - { - id: "changeOfUseFrom", - data: { - text: "Change of use of existing single home", - val: "changeOfUseFrom", - }, - }, - { - id: "changeOfUseTo", - data: { text: "Change of use to a home", val: "changeOfUseTo" }, - }, - ], - }, - }, - // options set "text" only - { - type: "question", - data: { - title: "Is this unit built on garden land?", - fn: "garden", - options: [ - { id: "true", data: { text: "Yes" } }, - { id: "false", data: { text: "No" } }, - ], - }, - }, - // fn = "identicalUnits" triggers summary stat - { - type: "number", - data: { - title: "How many identical units does the description above apply to?", - fn: "identicalUnits", - allowNegatives: false, - }, - }, - ], - min: 1, -} as const; diff --git a/editor.planx.uk/src/@planx/components/List/schemas/Tests/GenericUnits.ts b/editor.planx.uk/src/@planx/components/List/schemas/Tests/GenericUnits.ts new file mode 100644 index 0000000000..808da9a921 --- /dev/null +++ b/editor.planx.uk/src/@planx/components/List/schemas/Tests/GenericUnits.ts @@ -0,0 +1,95 @@ +import { Schema } from "@planx/components/List/model"; + +import { Props } from "../../Public"; + +export const GenericUnitsTest: Schema = { + type: "Unit", + fields: [ + // fn = "development" triggers summary stat and options set "val" + { + type: "question", + data: { + title: "What development does this unit result from?", + fn: "development", + options: [ + { id: "newBuild", data: { text: "New build", val: "newBuild" } }, + { + id: "changeOfUseFrom", + data: { + text: "Change of use of existing single home", + val: "changeOfUseFrom", + }, + }, + { + id: "changeOfUseTo", + data: { text: "Change of use to a home", val: "changeOfUseTo" }, + }, + ], + }, + }, + // options set "text" only + { + type: "question", + data: { + title: "Is this unit built on garden land?", + fn: "garden", + options: [ + { id: "true", data: { text: "Yes" } }, + { id: "false", data: { text: "No" } }, + ], + }, + }, + // fn = "identicalUnits" triggers summary stat + { + type: "number", + data: { + title: "How many identical units does the description above apply to?", + fn: "identicalUnits", + allowNegatives: false, + }, + }, + ], + min: 1, +} as const; + +export const mockUnitsProps: Props = { + fn: "proposal.units.residential", + schema: GenericUnitsTest, + schemaName: "Generic residential units", + title: "Describe residential units", +}; + +export const mockUnitsPayload = { + data: { + "proposal.units.residential": [ + { + development: "newBuild", + garden: "Yes", + identicalUnits: 1, + }, + { + development: "newBuild", + garden: "No", + identicalUnits: 2, + }, + { + development: "changeOfUseTo", + garden: "No", + identicalUnits: 2, + }, + ], + "proposal.units.residential.one.development": "newBuild", + "proposal.units.residential.one.garden": "Yes", + "proposal.units.residential.one.identicalUnits": 1, + "proposal.units.residential.two.development": "newBuild", + "proposal.units.residential.two.garden": "No", + "proposal.units.residential.two.identicalUnits": 2, + "proposal.units.residential.three.development": "changeOfUseTo", + "proposal.units.residential.three.garden": "No", + "proposal.units.residential.three.identicalUnits": 2, + "proposal.units.residential.total.listItems": 3, + "proposal.units.residential.total.units": 5, + "proposal.units.residential.total.units.newBuid": 3, + "proposal.units.residential.total.units.changeOfUseTo": 2, + }, +}; diff --git a/editor.planx.uk/src/@planx/components/List/schemas/Tests/MaxOne.ts b/editor.planx.uk/src/@planx/components/List/schemas/Tests/MaxOne.ts new file mode 100644 index 0000000000..bdbddaef59 --- /dev/null +++ b/editor.planx.uk/src/@planx/components/List/schemas/Tests/MaxOne.ts @@ -0,0 +1,58 @@ +import { Schema } from "@planx/components/List/model"; + +import { Props } from "../../Public"; + +export const MaxOneTest: Schema = { + type: "Parking spaces", + fields: [ + { + type: "number", + data: { + title: "How many parking spaces for cars?", + fn: "cars", + allowNegatives: false, + }, + }, + { + type: "number", + data: { + title: "How many parking spaces for bicycles?", + fn: "bikes", + allowNegatives: false, + }, + }, + { + type: "number", + data: { + title: "How many parking spaces for caravans?", + fn: "caravans", + allowNegatives: false, + }, + }, + ], + min: 1, + max: 1, +} as const; + +export const mockMaxOneProps: Props = { + fn: "proposal.parking", + schema: MaxOneTest, + schemaName: "Parking spaces", + title: "Describe parking spaces", +}; + +export const mockMaxOnePayload = { + data: { + "proposal.parking": [ + { + cars: 2, + bicycles: 4, + caravans: 0, + }, + ], + "proposal.parking.one.cars": 2, + "proposal.parking.one.bicycles": 4, + "proposal.parking.one.caravans": 0, + "proposal.parking.total.listItems": 1, + }, +}; diff --git a/editor.planx.uk/src/@planx/components/List/schemas/Zoo.ts b/editor.planx.uk/src/@planx/components/List/schemas/Tests/Zoo.ts similarity index 58% rename from editor.planx.uk/src/@planx/components/List/schemas/Zoo.ts rename to editor.planx.uk/src/@planx/components/List/schemas/Tests/Zoo.ts index 655512f182..6debeddea7 100644 --- a/editor.planx.uk/src/@planx/components/List/schemas/Zoo.ts +++ b/editor.planx.uk/src/@planx/components/List/schemas/Tests/Zoo.ts @@ -1,6 +1,7 @@ import { TextInputType } from "@planx/components/TextInput/model"; -import { Schema } from "../model"; +import { Schema } from "../../model"; +import { Props } from "../../Public"; /** * Temp simple example to build out UI @@ -67,3 +68,43 @@ export const Zoo: Schema = { min: 1, max: 10, } as const; + +export const mockZooProps: Props = { + fn: "mockFn", + schema: Zoo, + schemaName: "Zoo", + title: "Mock Title", + description: "Mock description", +}; + +export const mockZooPayload = { + data: { + mockFn: [ + { + age: 10, + cuteness: "Very", + email: "richard.parker@pi.com", + name: "Richard Parker", + size: "Medium", + }, + { + age: 10, + cuteness: "Very", + email: "richard.parker@pi.com", + name: "Richard Parker", + size: "Medium", + }, + ], + "mockFn.one.age": 10, + "mockFn.one.cuteness": "Very", + "mockFn.one.email": "richard.parker@pi.com", + "mockFn.one.name": "Richard Parker", + "mockFn.one.size": "Medium", + "mockFn.two.age": 10, + "mockFn.two.cuteness": "Very", + "mockFn.two.email": "richard.parker@pi.com", + "mockFn.two.name": "Richard Parker", + "mockFn.two.size": "Medium", + "mockFn.total.listItems": 2, + }, +}; From 7c62fef7b7e1cf0ab76826620600fc63f702c081 Mon Sep 17 00:00:00 2001 From: Jessica McInchak Date: Tue, 11 Jun 2024 13:23:35 +0200 Subject: [PATCH 2/2] tidy up naming --- .../src/@planx/components/List/Public/index.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/List/Public/index.tsx b/editor.planx.uk/src/@planx/components/List/Public/index.tsx index 2b72f210cb..50d49140cb 100644 --- a/editor.planx.uk/src/@planx/components/List/Public/index.tsx +++ b/editor.planx.uk/src/@planx/components/List/Public/index.tsx @@ -68,7 +68,7 @@ const ActiveListCard: React.FC<{ const { schema, saveItem, cancelEditItem, errors } = useListContext(); // Hide the index number in the card title if the schema has a max length of 1 - const shouldHideIndexTitle = schema.max !== 1; + const shouldShowIndexTitle = schema.max !== 1; return ( {schema.type} - {shouldHideIndexTitle && ` ${i + 1}`} + {shouldShowIndexTitle && ` ${i + 1}`} {schema.fields.map((field, i) => ( @@ -105,13 +105,13 @@ const InactiveListCard: React.FC<{ const { schema, formik, removeItem, editItem } = useListContext(); // Hide the index number in the card title if the schema has a max length of 1 - const shouldHideIndexTitle = schema.max !== 1; + const shouldShowIndexTitle = schema.max !== 1; return ( {schema.type} - {shouldHideIndexTitle && ` ${i + 1}`} + {shouldShowIndexTitle && ` ${i + 1}`}
@@ -163,7 +163,7 @@ const Root = () => { (errors.max && `You can provide at most ${schema.max} response(s)`) || ""; - // Hide the "+ Add another" button if the schema has a max length of 1, unless the only item has been cancelled/removed + // Hide the "+ Add another" button if the schema has a max length of 1, unless the only item has been cancelled/removed (userData = []) const shouldShowAddAnotherButton = schema.max !== 1 || formik.values.userData.length < 1;