From 0b7e466b813b96613afc4b13f68833e39f7f461d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Wed, 15 May 2024 11:48:38 +0100 Subject: [PATCH 1/3] feat(a11y): Remove disabled button from question component --- .../components/Question/Public/Question.tsx | 141 +++++++++--------- 1 file changed, 72 insertions(+), 69 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx b/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx index 8f5d86c195..c40e3f45ab 100644 --- a/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx +++ b/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx @@ -15,6 +15,8 @@ import { handleSubmit } from "pages/Preview/Node"; import React from "react"; import FormWrapper from "ui/public/FormWrapper"; import FullWidthWrapper from "ui/public/FullWidthWrapper"; +import ErrorWrapper from "ui/shared/ErrorWrapper"; +import { number, object, string } from "yup"; export interface IQuestion { id?: string; @@ -57,15 +59,15 @@ const Question: React.FC = (props) => { a: previousResponseKey ?? undefined, }, }, - onSubmit: (values) => { - setTimeout( - () => props.handleSubmit({ answers: [values.selected.id] }), - theme.transitions.duration.standard, - ); - }, - validate: () => { - // do nothing - }, + onSubmit: (values) => props.handleSubmit({ answers: [values.selected.id] }), + validateOnBlur: false, + validateOnChange: false, + validationSchema: object({ + selected: object({ + id: string().required("Select your answer before continuing"), + a: number().required(), + }) + }), }); let layout = QuestionLayout.Basic; @@ -80,7 +82,6 @@ const Question: React.FC = (props) => { return ( = (props) => { > {props.text} - - + - {props.responses?.map((response) => { - const onChange = () => { - formik.setFieldValue("selected.id", response.id); - formik.setFieldValue("selected.a", response.responseKey); - }; - const buttonProps = { - onChange, - }; + + {props.responses?.map((response) => { + const onChange = () => { + formik.setFieldValue("selected.id", response.id); + formik.setFieldValue("selected.a", response.responseKey); + }; + const buttonProps = { + onChange, + }; - switch (layout) { - case QuestionLayout.Basic: - return ( - - - + switch (layout) { + case QuestionLayout.Basic: + return ( + + + + + + ); + case QuestionLayout.Descriptions: + return ( + + + + ); + case QuestionLayout.Images: + return ( + + - - ); - case QuestionLayout.Descriptions: - return ( - - - - ); - case QuestionLayout.Images: - return ( - - - - ); - } - })} - - + ); + } + })} + + + From ece17bd2be8ccd6bd264828603b753a04fb9f35a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Wed, 15 May 2024 14:24:55 +0100 Subject: [PATCH 2/3] test: Fix tests - Add validation for `a` - Get test functions from `setup()` - Add `describe()` blocks --- .../components/Question/Public.test.tsx | 161 ++++++++++-------- .../components/Question/Public/Question.tsx | 6 +- 2 files changed, 96 insertions(+), 71 deletions(-) diff --git a/editor.planx.uk/src/@planx/components/Question/Public.test.tsx b/editor.planx.uk/src/@planx/components/Question/Public.test.tsx index 3006b0ce26..caf1f12062 100644 --- a/editor.planx.uk/src/@planx/components/Question/Public.test.tsx +++ b/editor.planx.uk/src/@planx/components/Question/Public.test.tsx @@ -52,75 +52,98 @@ describe("Question component", () => { QuestionLayout.Images, QuestionLayout.Descriptions, ].forEach((type) => { - it(`renders the ${QuestionLayout[type]} layout correctly`, async () => { - const handleSubmit = jest.fn(); - - const { user } = setup( - , - ); - - const continueButton = screen.getByTestId("continue-button"); - - expect(screen.getByRole("heading")).toHaveTextContent("Best food"); - - expect(continueButton).toBeDisabled(); - - await user.click(screen.getByText("Pizza")); - - expect(continueButton).not.toBeDisabled(); - - await user.click(continueButton); - - await waitFor(() => - expect(handleSubmit).toHaveBeenCalledWith({ answers: ["pizza_id"] }), - ); - }); - - it(`should display previously selected answer on back or change in the ${QuestionLayout[type]} layout`, async () => { - const handleSubmit = jest.fn(); - const { user } = setup( - , - ); - - expect(screen.getByRole("heading")).toHaveTextContent("Best food"); - - const celeryRadio = screen.getByRole("radio", { name: /Celery/ }); - const pizzaRadio = screen.getByRole("radio", { name: /Pizza/ }); - - // State is preserved... - expect(celeryRadio).toBeChecked(); - - // ...and can be updated - await user.click(pizzaRadio); - expect(pizzaRadio).toBeChecked(); - - const continueButton = screen.getByTestId("continue-button"); - expect(continueButton).toBeEnabled(); - }); - - it(`should not have any accessibility violations in the ${QuestionLayout[type]} layout`, async () => { - const handleSubmit = jest.fn(); - const { container } = setup( - , - ); - const results = await axe(container); - expect(results).toHaveNoViolations(); + describe(`${QuestionLayout[type]} layout`, () => { + it(`renders the layout correctly`, async () => { + const handleSubmit = jest.fn(); + + const { user, getByTestId, getByRole, getByText } = setup( + , + ); + + const continueButton = getByTestId("continue-button"); + + expect(getByRole("heading")).toHaveTextContent("Best food"); + + await user.click(getByText("Pizza")); + + await user.click(continueButton); + + await waitFor(() => + expect(handleSubmit).toHaveBeenCalledWith({ answers: ["pizza_id"] }), + ); + }); + + it(`should display previously selected answer on back or change`, async () => { + const handleSubmit = jest.fn(); + const { user, getByRole, getByTestId } = setup( + , + ); + + expect(getByRole("heading")).toHaveTextContent("Best food"); + + const celeryRadio = getByRole("radio", { name: /Celery/ }); + const pizzaRadio = getByRole("radio", { name: /Pizza/ }); + + // State is preserved... + expect(celeryRadio).toBeChecked(); + + // ...and can be updated + await user.click(pizzaRadio); + expect(pizzaRadio).toBeChecked(); + + const continueButton = getByTestId("continue-button"); + expect(continueButton).toBeEnabled(); + }); + + it(`should not have any accessibility violations`, async () => { + const handleSubmit = jest.fn(); + const { container } = setup( + , + ); + const results = await axe(container); + expect(results).toHaveNoViolations(); + }); + + it(`should display an error message if no option is selected`, async () => { + const handleSubmit = jest.fn(); + const errorMessage = /Select your answer before continuing/; + + const { user, getByTestId, getByText, queryByText } = setup( + , + ); + + const continueButton = getByTestId("continue-button"); + + expect(continueButton).not.toBeDisabled(); + expect(queryByText(errorMessage)).not.toBeInTheDocument(); + + await user.click(continueButton); + + await waitFor(() => { + expect(handleSubmit).not.toHaveBeenCalled(); + expect(getByText(errorMessage)).toBeVisible(); + }); + }); }); }); }); diff --git a/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx b/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx index c40e3f45ab..0d16379c64 100644 --- a/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx +++ b/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx @@ -16,7 +16,7 @@ import React from "react"; import FormWrapper from "ui/public/FormWrapper"; import FullWidthWrapper from "ui/public/FullWidthWrapper"; import ErrorWrapper from "ui/shared/ErrorWrapper"; -import { number, object, string } from "yup"; +import { mixed, object, string } from "yup"; export interface IQuestion { id?: string; @@ -65,7 +65,9 @@ const Question: React.FC = (props) => { validationSchema: object({ selected: object({ id: string().required("Select your answer before continuing"), - a: number().required(), + a: mixed().required().test(value => + typeof value === "number" || + typeof value === "string") }) }), }); From e0dadb74b2c0084a8f651cdc08175dccba370d8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Thu, 16 May 2024 08:32:18 +0100 Subject: [PATCH 3/3] fix: Error wrapper text --- .../src/@planx/components/Question/Public/Question.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx b/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx index 0d16379c64..bfa855e8e5 100644 --- a/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx +++ b/editor.planx.uk/src/@planx/components/Question/Public/Question.tsx @@ -103,7 +103,7 @@ const Question: React.FC = (props) => { > {props.text} - +