feat: support "address" fields in List schemas #4020

jessicamcinchak · 2024-11-29T13:53:04Z

Changes:

Building on feat(types): Stricter and more accurate Schema response types #4153, adds support for type: "address" fields in List & Page schemas,
"address" fields follow our AddressInput component pattern, with a mix of required and optional fields
Adds test coverage
Adds a preliminary "Owners" schema (very much anticapte content team will want to edit this!)

Testing: https://4020.planx.pizza/testing/owners-as-list

github-actions · 2024-11-29T14:28:39Z

Removed vultr server and associated DNS entries

jessicamcinchak · 2025-01-20T21:31:48Z

editor.planx.uk/src/@planx/components/shared/Preview/SummaryList.tsx

@@ -637,6 +639,8 @@ function Page(props: ComponentProps) {
    if (isTextResponse(answer)) return answer;
    if (isNumberFieldResponse(answer)) return answer.toString();
    if (isMapFieldResponse(answer)) return `${answer.length || 0} features`;
+    if (isAddressFieldResponse(answer))
+      return formatAsSingleLineAddress(answer);


Nice & tidy 👍

jessicamcinchak · 2025-01-20T21:32:19Z

editor.planx.uk/src/@planx/components/shared/Schema/InputFields/AddressFieldInput.tsx

+   * Maps changes from AddressField inputs to their corresponding nested form fields
+   * @example
+   * The input for name=line1 will be mapped to the schemaData[0][yourFn]["line1"] formik field
+   */


jessicamcinchak · 2025-01-20T21:43:23Z

List component should support Address inputs and optional fields for better UX of "Ownership" questions & schema alignment

editor.planx.uk/src/@planx/components/shared/Schema/model.ts

RODO94

Tested it locally and on the pizza, everything seems to be working for me as I'd expect. With all data tracking across nicely to the passport.

Only added one nit comment related to error handling and how strict to be with validating data in the components.

The current Address Component doesn't have this and it can get complex if we handle addresses outside the UK, so happy for this to be left out at your discretion.

editor.planx.uk/src/@planx/components/AddressInput/model.ts

RODO94 · 2025-01-21T14:06:48Z

editor.planx.uk/src/@planx/components/List/Public/index.test.tsx

-    const numberOfErrors = mockZooProps.schema.fields.length + 3;
+
+    let numberOfErrors = 0;
+    mockZooProps.schema.fields.forEach((field) => {


I like this! Nice way to clearly manage the date error count

editor.planx.uk/src/@planx/components/shared/Schema/model.ts

RODO94 · 2025-01-21T14:17:01Z

editor.planx.uk/src/@planx/components/AddressInput/Public.tsx

@@ -38,7 +30,7 @@ export default function AddressInputComponent(props: Props): FCReturn {
    },
    validateOnBlur: false,
    validateOnChange: false,
-    validationSchema: userDataSchema,
+    validationSchema: addressValidationSchema,


question: significance of this switch is now the List schema now shares the same validation schema as this address input component?

Thus, you gave it a more specific name

And the list schema is now using the <AddressFields/> within the <AddressFieldsInput/>

That's right - this PR imports and reuses the existing Address component's validation schema.

On the naming - specific names mean we don't have to import multiple instances of userDataSchema and cast them to a meaningful name.

This approach is quite fiddly and error prone, and you need to remember the naming convention and repeat across files 👇

import { userDataSchema as addressValidationSchema } from "@planx/components/AddressInput/model"; import { userDataSchema as textValidationSchema } from "@planx/components/TextInput/model";

This isn't actually applied consistently yet in this file - will push up a quick commit now to address this.

Please see 70fc546

jamdelion · 2025-01-21T14:45:57Z

editor.planx.uk/src/@planx/components/AddressInput/Public.tsx

+export interface AddressFieldsProps {
+  id?: string;
+  values: Address;
+  errors: FormikErrors<Address>;
+  handleChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
+}
+
+export function AddressFields(props: AddressFieldsProps): FCReturn {


nit/personal opinion: I usually try to have one file per React component, so would prefer to see this in its own file, but that's just me!

jamdelion · 2025-01-21T14:47:47Z

editor.planx.uk/src/@planx/components/AddressInput/model.ts

-  postcode: string().required("Enter a postcode"),
-  country: string(),
-});
+export const addressValidationSchema = (): SchemaOf<Address> =>


Good to have renamed this 👍

jamdelion · 2025-01-21T14:49:46Z

editor.planx.uk/src/@planx/components/List/Public/index.test.tsx

@@ -167,7 +167,7 @@ describe("Building a list", () => {
    expect(addItemButton).not.toBeInTheDocument();
  });

-  test("Adding an item", { timeout: 20000 }, async () => {
+  test("Adding an item", { timeout: 35000 }, async () => {


question: what is the timeout for? Are these tests flaky?

also nit: we can include a numeric separator to make these big numbers easier to read, e.g. 35_000

Great feedback thanks - timeouts now have numerical separators - please see 5435bf9 ✅

The tests here are pretty slow! They pass locally without timeouts as generous as this but I think that's an issue with underpowered GHA runners needing a larger timeout than my local machine.

This issue here has come up in a few other PRs and it's not flakeiness but some sort of state update interplay between vitest, formik, and React context which leads to these being slow tests. I've not go to the bottom of this and it does seem to be a test-only issue. It might be the case that we need to revisit how we're using context providers for the schema - I've had a few passes over adding useMemo()s etc here and there without any seeming effect on the test speeds.

jamdelion · 2025-01-21T14:54:14Z

editor.planx.uk/src/@planx/components/List/Public/index.test.tsx

This is a humungous file! Again personal preference but I'd be tempted to split this into more test files, e.g. index.validation.test.tsx, index.payloadGeneration.test.tsx so that it's easier to read/find the tests you want. 🤷

…ming pattern

DafyddLlyr force-pushed the jess/list-address-field-owners branch from ea34817 to 52adff1 Compare January 14, 2025 16:31

DafyddLlyr mentioned this pull request Jan 17, 2025

feat(types): Stricter and more accurate Schema response types #4153

Merged

jessicamcinchak added 2 commits January 17, 2025 15:47

wip

d83768f

better types

74ca0f0

DafyddLlyr force-pushed the jess/list-address-field-owners branch from 52adff1 to 74ca0f0 Compare January 17, 2025 15:48

DafyddLlyr added 3 commits January 17, 2025 16:06

chore: Types tidy up

20c22f7

feat: Wire up inputs to formik

e5fdd67

feat: Styles

f66b0fa

DafyddLlyr force-pushed the jess/list-address-field-owners branch from f479b83 to 7219a7b Compare January 18, 2025 12:31

test: Add coverage for new address field

ad24946

DafyddLlyr force-pushed the jess/list-address-field-owners branch from 7219a7b to ad24946 Compare January 18, 2025 12:34

test: Bump timeouts

b77f302

DafyddLlyr force-pushed the jess/list-address-field-owners branch from ec8886f to b77f302 Compare January 20, 2025 12:22

jessicamcinchak commented Jan 20, 2025

View reviewed changes

jessicamcinchak marked this pull request as ready for review January 20, 2025 21:38

jessicamcinchak requested a review from a team January 20, 2025 21:38

DafyddLlyr reviewed Jan 21, 2025

View reviewed changes

editor.planx.uk/src/@planx/components/shared/Schema/model.ts Show resolved Hide resolved

RODO94 approved these changes Jan 21, 2025

View reviewed changes

jamdelion reviewed Jan 21, 2025

View reviewed changes

DafyddLlyr added 2 commits January 22, 2025 11:05

fix: Remove old validation schema in names in favour of a standard na…

70fc546

…ming pattern

fix: Add numberical seperators to timeouts

5435bf9

DafyddLlyr force-pushed the jess/list-address-field-owners branch from 93edced to 5435bf9 Compare January 22, 2025 11:05

DafyddLlyr merged commit ce1e687 into main Jan 22, 2025
12 checks passed

DafyddLlyr deleted the jess/list-address-field-owners branch January 22, 2025 15:12

DafyddLlyr mentioned this pull request Jan 24, 2025

feat: Allow optional fields in List and Page schemas #4166

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support "address" fields in List schemas #4020

feat: support "address" fields in List schemas #4020

jessicamcinchak commented Nov 29, 2024 •

edited

Loading

github-actions bot commented Nov 29, 2024 •

edited

Loading

jessicamcinchak Jan 20, 2025 •

edited

Loading

jessicamcinchak Jan 20, 2025

jessicamcinchak commented Jan 20, 2025

RODO94 left a comment

RODO94 Jan 21, 2025

RODO94 Jan 21, 2025

RODO94 Jan 21, 2025

RODO94 Jan 21, 2025

DafyddLlyr Jan 21, 2025 •

edited

Loading

DafyddLlyr Jan 22, 2025 •

edited

Loading

jamdelion Jan 21, 2025

jamdelion Jan 21, 2025

jamdelion Jan 21, 2025 •

edited

Loading

jamdelion Jan 21, 2025

DafyddLlyr Jan 22, 2025

jamdelion Jan 21, 2025

feat: support "address" fields in List schemas #4020

feat: support "address" fields in List schemas #4020

Conversation

jessicamcinchak commented Nov 29, 2024 • edited Loading

github-actions bot commented Nov 29, 2024 • edited Loading

jessicamcinchak Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessicamcinchak commented Jan 20, 2025

RODO94 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DafyddLlyr Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

DafyddLlyr Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamdelion Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessicamcinchak commented Nov 29, 2024 •

edited

Loading

github-actions bot commented Nov 29, 2024 •

edited

Loading

jessicamcinchak Jan 20, 2025 •

edited

Loading

DafyddLlyr Jan 21, 2025 •

edited

Loading

DafyddLlyr Jan 22, 2025 •

edited

Loading

jamdelion Jan 21, 2025 •

edited

Loading