From daf459a1bb078571c02c97651129fa5ae230b6f8 Mon Sep 17 00:00:00 2001 From: Bea Esguerra Date: Thu, 3 Oct 2024 11:50:56 -0500 Subject: [PATCH 01/10] LabeledField (part 1): Initial set up for LabeledField component based on FieldHeading (#2322) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: - The `FieldHeading` component internal to the `wonder-blocks-form package` was copied to `wonder-blocks-labeled-field` package and renamed to the `LabeledField` component - Export `LabeledField` as a named export - Added base stories for `LabeledField` and updated docs Issue: WB-1503 ## Test plan: - `LabeledField` component is reviewed (`?path=/docs/packages-labeledfield-labeledfield--docs`) Note: Minimal changes have been made to the FieldHeading implementation and tests (just renaming). Next, I will be working on updating the component so it follows the implementation spec and exploring validation more, so the api may change! I'll be merging PRs to the `feature/labeled-field` branch until it's all ready ๐Ÿ˜„ Author: beaesguerra Reviewers: beaesguerra, jandrade Required Reviewers: Approved By: jandrade Checks: โœ… Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), โœ… Check build sizes (ubuntu-latest, 20.x), โœ… Test (ubuntu-latest, 20.x, 2/2), โœ… Test (ubuntu-latest, 20.x, 1/2), โœ… Lint (ubuntu-latest, 20.x), โœ… Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), โญ๏ธ Chromatic - Skip on Release PR (changesets), ๐Ÿšซ Chromatic - Get results on regular PRs, โœ… Publish npm snapshot (ubuntu-latest, 20.x), โœ… Check for .changeset entries for all changed files (ubuntu-latest, 20.x), โœ… Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), โœ… gerald, โœ… Test (ubuntu-latest, 20.x, 2/2), โœ… Test (ubuntu-latest, 20.x, 1/2), โœ… Check build sizes (ubuntu-latest, 20.x), โœ… Lint (ubuntu-latest, 20.x), ๐Ÿšซ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), โœ… Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), โญ๏ธ Chromatic - Skip on Release PR (changesets), โœ… Publish npm snapshot (ubuntu-latest, 20.x), โœ… gerald, โœ… Check for .changeset entries for all changed files (ubuntu-latest, 20.x), โญ๏ธ dependabot Pull Request URL: https://github.com/Khan/wonder-blocks/pull/2322 --- .changeset/two-pens-remember.md | 5 + .../labeled-field.argtypes.ts | 49 ++++ .../labeled-field.stories.tsx | 171 +++++++++++++ .../__tests__/labeled-field.test.tsx | 225 ++++++++++++++++++ .../src/components/labeled-field.tsx | 171 ++++++++++++- .../wonder-blocks-labeled-field/src/index.ts | 2 +- .../tsconfig-build.json | 2 + 7 files changed, 621 insertions(+), 4 deletions(-) create mode 100644 .changeset/two-pens-remember.md create mode 100644 __docs__/wonder-blocks-labeled-field/labeled-field.argtypes.ts create mode 100644 __docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx create mode 100644 packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx diff --git a/.changeset/two-pens-remember.md b/.changeset/two-pens-remember.md new file mode 100644 index 000000000..6db6ad966 --- /dev/null +++ b/.changeset/two-pens-remember.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-labeled-field": patch +--- + +Sets up the initial implementation for LabeledField based on the internal FieldHeading component in the form package diff --git a/__docs__/wonder-blocks-labeled-field/labeled-field.argtypes.ts b/__docs__/wonder-blocks-labeled-field/labeled-field.argtypes.ts new file mode 100644 index 000000000..45f35707b --- /dev/null +++ b/__docs__/wonder-blocks-labeled-field/labeled-field.argtypes.ts @@ -0,0 +1,49 @@ +export default { + label: { + table: { + type: { + summary: "string | ReactNode", + }, + }, + }, + description: { + table: { + type: { + summary: "string | ReactNode", + }, + }, + }, + error: { + table: { + type: { + summary: "string | ReactNode", + }, + }, + }, + field: { + table: { + type: { + summary: "ReactNode", + }, + }, + }, + /** + * Visual Style + */ + style: { + table: { + type: { + summary: "StyleType", + }, + category: "Visual style", + }, + control: { + type: "null", + }, + }, + light: { + table: { + category: "Visual style", + }, + }, +}; diff --git a/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx b/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx new file mode 100644 index 000000000..7b2bfe16d --- /dev/null +++ b/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx @@ -0,0 +1,171 @@ +import * as React from "react"; +import type {Meta, StoryObj} from "@storybook/react"; +import ComponentInfo from "../../.storybook/components/component-info"; +import packageConfig from "../../packages/wonder-blocks-labeled-field/package.json"; +import {LabeledField} from "@khanacademy/wonder-blocks-labeled-field"; +import {TextArea, TextField} from "@khanacademy/wonder-blocks-form"; +import LabeledFieldArgTypes from "./labeled-field.argtypes"; +import {PropsFor, View} from "@khanacademy/wonder-blocks-core"; +import {spacing} from "@khanacademy/wonder-blocks-tokens"; +import { + MultiSelect, + OptionItem, + SingleSelect, +} from "@khanacademy/wonder-blocks-dropdown"; +import SearchField from "@khanacademy/wonder-blocks-search-field"; + +/** + * The `LabeledField` component provides common elements for a form field such + * as the label, required indicator, description, and error message. + * + * It is highly recommended that all form fields should be used with the + * `LabeledField` component so that our form fields are consistent and accessible. + */ +export default { + title: "Packages / LabeledField / LabeledField", + component: LabeledField, + parameters: { + componentSubtitle: ( + + ), + }, + argTypes: LabeledFieldArgTypes, +} as Meta; + +type StoryComponentType = StoryObj; + +export const Default: StoryComponentType = { + args: { + field: {}} />, + label: "Name", + description: "Helpful description text", + error: "Error message", + required: true, + }, +}; + +const AllFields = (args: PropsFor) => { + const [textFieldValue, setTextFieldValue] = React.useState(""); + const [textAreaValue, setTextAreaValue] = React.useState(""); + const [singleSelectValue, setSingleSelectValue] = React.useState(""); + const [multiSelectValue, setMultiSelectValue] = React.useState( + [], + ); + const [searchValue, setSearchValue] = React.useState(""); + + return ( + + + } + /> + + } + /> + + + + + + + } + /> + + + + + + + } + /> + + + } + /> + + ); +}; + +/** + * The `LabeledField` component can be used with form field components such as: + * - `TextField` + * - `TextArea` + * - `SingleSelect` + * - `MultiSelect` + * - `SearchField` + */ +export const Fields: StoryComponentType = { + args: { + description: "Helpful description text", + required: true, + }, + render: AllFields, +}; + +export const Error: StoryComponentType = { + args: { + description: "Helpful description text", + error: "Error message", + required: true, + }, + render: AllFields, +}; + +/** + * If the labeled field is used on a dark background, the `light` prop can be + * set to `true`. When abled, the text in the component (label, required + * indicator, description, and error message) are modified to work on a dark + * background. + */ +export const Light: StoryComponentType = { + args: { + description: "Helpful description text", + error: "Error message", + required: true, + light: true, + }, + parameters: { + backgrounds: {default: "darkBlue"}, + }, + render: AllFields, +}; diff --git a/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx b/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx new file mode 100644 index 000000000..de5b9a385 --- /dev/null +++ b/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx @@ -0,0 +1,225 @@ +import * as React from "react"; +import {render, screen} from "@testing-library/react"; +import {StyleSheet} from "aphrodite"; + +import {I18nInlineMarkup} from "@khanacademy/wonder-blocks-i18n"; +import {Body} from "@khanacademy/wonder-blocks-typography"; + +import {TextField} from "@khanacademy/wonder-blocks-form"; +import LabeledField from "../labeled-field"; + +describe("LabeledField", () => { + it("LabeledField renders the label text", () => { + // Arrange + const label = "Label"; + + // Act + render( + {}} />} + label={label} + />, + ); + + // Assert + expect(screen.getByText(label)).toBeInTheDocument(); + }); + + it("LabeledField renders the description text", () => { + // Arrange + const description = "Description"; + + // Act + render( + {}} />} + label="Label" + description={description} + />, + ); + + // Assert + expect(screen.getByText(description)).toBeInTheDocument(); + }); + + it("LabeledField renders the error text", () => { + // Arrange + const error = "Error"; + + // Act + render( + {}} />} + label="Label" + error={error} + />, + ); + + // Assert + expect(screen.getByRole("alert")).toBeInTheDocument(); + }); + + it("LabeledField adds testId to label", () => { + // Arrange + const testId = "testid"; + + // Act + render( + {}} />} + label="Label" + testId={testId} + />, + ); + + // Assert + const label = screen.getByTestId(`${testId}-label`); + expect(label).toBeInTheDocument(); + }); + + it("LabeledField adds testId to description", () => { + // Arrange + const testId = "testid"; + + // Act + render( + {}} />} + label="Label" + description="Description" + testId={testId} + />, + ); + + // Assert + const description = screen.getByTestId(`${testId}-description`); + expect(description).toBeInTheDocument(); + }); + + it("LabeledField adds testId to error", () => { + // Arrange + const testId = "testid"; + + // Act + render( + {}} />} + label="Label" + error="Error" + testId={testId} + />, + ); + + // Assert + const error = screen.getByTestId(`${testId}-error`); + expect(error).toBeInTheDocument(); + }); + + it("LabeledField adds the correctly formatted id to label's htmlFor", () => { + // Arrange + const id = "exampleid"; + const testId = "testid"; + + // Act + render( + {}} />} + label="Label" + id={id} + testId={testId} + />, + ); + + // Assert + const label = screen.getByTestId(`${testId}-label`); + expect(label).toHaveAttribute("for", `${id}-field`); + }); + + it("LabeledField adds the correctly formatted id to error's id", () => { + // Arrange + const id = "exampleid"; + const testId = "testid"; + + // Act + render( + {}} />} + label="Label" + error="Error" + id={id} + testId={testId} + />, + ); + + // Assert + const error = screen.getByRole("alert"); + expect(error).toHaveAttribute("id", `${id}-error`); + }); + + it("stype prop applies to the LabeledField container", () => { + // Arrange + const styles = StyleSheet.create({ + style1: { + flexGrow: 1, + background: "blue", + }, + }); + + // Act + const {container} = render( + {}} />} + label="Label" + error="Error" + style={styles.style1} + />, + ); + + // Assert + const labeledField = container.childNodes[0]; + expect(labeledField).toHaveStyle("background: blue"); + }); + + it("should render a LabelMedium when the 'label' prop is a I18nInlineMarkup", () => { + // Arrange + + // Act + render( + {}} />} + label={ + {s}}> + {"Test Hello, world!"} + + } + />, + ); + + // Assert + const label = screen.getByText("Hello, world!"); + // LabelMedium has a font-size of 16px + expect(label).toHaveStyle("font-size: 16px"); + }); + + it("should render a LabelSmall when the 'description' prop is a I18nInlineMarkup", () => { + // Arrange + + // Act + render( + {}} />} + label={Hello, world} + description={ + {s}}> + {"Test description"} + + } + />, + ); + + // Assert + const description = screen.getByText("description"); + // LabelSmall has a font-size of 16px + expect(description).toHaveStyle("font-size: 14px"); + }); +}); diff --git a/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx b/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx index bcacb42f1..a69c6aeb7 100644 --- a/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx +++ b/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx @@ -1,4 +1,169 @@ -// Something so that the file is not empty -const LabeledField = "LabeledField"; +import * as React from "react"; +import {StyleSheet} from "aphrodite"; -export default LabeledField; +import {View, addStyle, StyleType} from "@khanacademy/wonder-blocks-core"; +import {Strut} from "@khanacademy/wonder-blocks-layout"; +import {color, spacing} from "@khanacademy/wonder-blocks-tokens"; +import {LabelMedium, LabelSmall} from "@khanacademy/wonder-blocks-typography"; + +type Props = { + /** + * The form field component. + */ + field: React.ReactNode; + /** + * The title for the label element. + */ + label: React.ReactNode; + /** + * The text for the description element. + */ + description?: React.ReactNode; + /** + * Whether this field is required to continue. + */ + required?: boolean; + /** + * The message for the error element. + */ + error?: React.ReactNode; + /** + * Custom styles for the labeled field container. + */ + style?: StyleType; + /** + * A unique id to link the label (and optional error) to the field. + * + * The label will assume that the field will have its id formatted as `${id}-field`. + * The field can assume that the error will have its id formatted as `${id}-error`. + */ + id?: string; + /** + * Optional test ID for e2e testing. + */ + testId?: string; + /** + * Change the fieldโ€™s sub-components to fit a dark background. + */ + light?: boolean; +}; + +const StyledSpan = addStyle("span"); + +/** + * A LabeledField is an element that provides a label, description, and error element + * to present better context and hints to any type of form field component. + */ +export default class LabeledField extends React.Component { + renderLabel(): React.ReactNode { + const {label, id, required, testId, light} = this.props; + + const requiredIcon = ( + + {" "} + * + + ); + + return ( + + + {label} + {required && requiredIcon} + + + + ); + } + + maybeRenderDescription(): React.ReactNode | null | undefined { + const {description, testId, light} = this.props; + + if (!description) { + return null; + } + + return ( + + + {description} + + + + ); + } + + maybeRenderError(): React.ReactNode | null | undefined { + const {error, id, testId, light} = this.props; + + if (!error) { + return null; + } + + return ( + + + + {error} + + + ); + } + + render(): React.ReactNode { + const {field, style} = this.props; + + return ( + + {this.renderLabel()} + {this.maybeRenderDescription()} + + {field} + {this.maybeRenderError()} + + ); + } +} + +const styles = StyleSheet.create({ + label: { + color: color.offBlack, + }, + lightLabel: { + color: color.white, + }, + description: { + color: color.offBlack64, + }, + lightDescription: { + color: color.white64, + }, + error: { + color: color.red, + }, + lightError: { + color: color.fadedRed, + }, + required: { + color: color.red, + }, + lightRequired: { + color: color.fadedRed, + }, +}); diff --git a/packages/wonder-blocks-labeled-field/src/index.ts b/packages/wonder-blocks-labeled-field/src/index.ts index 01f326352..1f90ff46a 100644 --- a/packages/wonder-blocks-labeled-field/src/index.ts +++ b/packages/wonder-blocks-labeled-field/src/index.ts @@ -1,3 +1,3 @@ import LabeledField from "./components/labeled-field"; -export default LabeledField; +export {LabeledField}; diff --git a/packages/wonder-blocks-labeled-field/tsconfig-build.json b/packages/wonder-blocks-labeled-field/tsconfig-build.json index a837c5376..38cf68ebb 100644 --- a/packages/wonder-blocks-labeled-field/tsconfig-build.json +++ b/packages/wonder-blocks-labeled-field/tsconfig-build.json @@ -10,5 +10,7 @@ {"path": "../wonder-blocks-layout/tsconfig-build.json"}, {"path": "../wonder-blocks-tokens/tsconfig-build.json"}, {"path": "../wonder-blocks-typography/tsconfig-build.json"}, + {"path": "../wonder-blocks-i18n/tsconfig-build.json"}, + {"path": "../wonder-blocks-form/tsconfig-build.json"}, ] } \ No newline at end of file From fd29f864836fb043c2d0caf271a9790668d46842 Mon Sep 17 00:00:00 2001 From: Bea Esguerra Date: Fri, 4 Oct 2024 14:35:45 -0500 Subject: [PATCH 02/10] LabeledField(part2): refactor to a function component (#2338) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Refactoring the LabeledField component to a function component. The LabeledField is based on the FieldHeading component and will be updated in more PRs to meet the implementation spec Issue: WB-1503 ## Test plan: Confirm tests still pass and component looks and functions the same way Author: beaesguerra Reviewers: jandrade Required Reviewers: Approved By: jandrade Checks: โœ… Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), โœ… Test (ubuntu-latest, 20.x, 2/2), โœ… Test (ubuntu-latest, 20.x, 1/2), โœ… Check build sizes (ubuntu-latest, 20.x), โœ… Lint (ubuntu-latest, 20.x), โœ… Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), โœ… Publish npm snapshot (ubuntu-latest, 20.x), โญ๏ธ Chromatic - Skip on Release PR (changesets), โœ… Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), โœ… Check for .changeset entries for all changed files (ubuntu-latest, 20.x), โœ… gerald, โญ๏ธ dependabot Pull Request URL: https://github.com/Khan/wonder-blocks/pull/2338 --- .changeset/big-starfishes-worry.md | 5 ++ .../src/components/labeled-field.tsx | 46 ++++++++++--------- 2 files changed, 29 insertions(+), 22 deletions(-) create mode 100644 .changeset/big-starfishes-worry.md diff --git a/.changeset/big-starfishes-worry.md b/.changeset/big-starfishes-worry.md new file mode 100644 index 000000000..ce702cf8f --- /dev/null +++ b/.changeset/big-starfishes-worry.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-labeled-field": patch +--- + +LabeledField: Refactor from class component to function component diff --git a/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx b/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx index a69c6aeb7..d0a2b4c69 100644 --- a/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx +++ b/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx @@ -54,10 +54,20 @@ const StyledSpan = addStyle("span"); * A LabeledField is an element that provides a label, description, and error element * to present better context and hints to any type of form field component. */ -export default class LabeledField extends React.Component { - renderLabel(): React.ReactNode { - const {label, id, required, testId, light} = this.props; +export default function LabeledField(props: Props) { + const { + field, + style, + label, + id, + required, + testId, + light, + description, + error, + } = props; + function renderLabel(): React.ReactNode { const requiredIcon = ( { ); } - maybeRenderDescription(): React.ReactNode | null | undefined { - const {description, testId, light} = this.props; - + function maybeRenderDescription(): React.ReactNode | null | undefined { if (!description) { return null; } @@ -104,9 +112,7 @@ export default class LabeledField extends React.Component { ); } - maybeRenderError(): React.ReactNode | null | undefined { - const {error, id, testId, light} = this.props; - + function maybeRenderError(): React.ReactNode | null | undefined { if (!error) { return null; } @@ -126,19 +132,15 @@ export default class LabeledField extends React.Component { ); } - render(): React.ReactNode { - const {field, style} = this.props; - - return ( - - {this.renderLabel()} - {this.maybeRenderDescription()} - - {field} - {this.maybeRenderError()} - - ); - } + return ( + + {renderLabel()} + {maybeRenderDescription()} + + {field} + {maybeRenderError()} + + ); } const styles = StyleSheet.create({ From dea7c71ecf43e49d1b49be06786c65d118aa4428 Mon Sep 17 00:00:00 2001 From: Bea Esguerra Date: Wed, 9 Oct 2024 17:25:12 -0500 Subject: [PATCH 03/10] LabeledField(part3): Wire up attributes (#2339) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: - Use an auto-generated unique id if the `id` prop is not provided - Wire up elements within the component - the `label` element's `for` attribute is the field `id` - `aria-describedby` on the field has the ids of the description and/or error message elements - Apply attributes to the field element: - `id` - `aria-describedby` - `required` if the `required` prop is set to `true` - `testId` Issue: WB-1503 ## Test plan: - Tests are passing - Updated documentation makes sense: - `?path=/docs/packages-labeledfield-labeledfield--docs` - `?path=/docs/packages-labeledfield-labeledfield-accessibility--docs` - There are no a11y warnings in the Storybook Accessibility add on for the LabeledField stories - Note: There are warnings for the LabeledField stories with the different field examples. They are related to an issue with the SearchField component and will be addressed in WB-1771! Author: beaesguerra Reviewers: beaesguerra, marcysutton, jandrade Required Reviewers: Approved By: jandrade Checks: โœ… Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), โœ… Test (ubuntu-latest, 20.x, 2/2), โœ… Test (ubuntu-latest, 20.x, 1/2), โœ… Lint (ubuntu-latest, 20.x), โœ… Check build sizes (ubuntu-latest, 20.x), โœ… Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), โœ… Publish npm snapshot (ubuntu-latest, 20.x), โญ๏ธ Chromatic - Skip on Release PR (changesets), โœ… Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), โœ… Check for .changeset entries for all changed files (ubuntu-latest, 20.x), โœ… gerald, โญ๏ธ dependabot Pull Request URL: https://github.com/Khan/wonder-blocks/pull/2339 --- .changeset/pink-turtles-sleep.md | 5 + .../accessibility.mdx | 20 ++ .../labeled-field.stories.tsx | 61 +++- .../__tests__/labeled-field.test.tsx | 265 ++++++++++++++++++ .../src/components/labeled-field.tsx | 57 +++- 5 files changed, 398 insertions(+), 10 deletions(-) create mode 100644 .changeset/pink-turtles-sleep.md create mode 100644 __docs__/wonder-blocks-labeled-field/accessibility.mdx diff --git a/.changeset/pink-turtles-sleep.md b/.changeset/pink-turtles-sleep.md new file mode 100644 index 000000000..ffce9a348 --- /dev/null +++ b/.changeset/pink-turtles-sleep.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-labeled-field": minor +--- + +LabeledField: Wire up attributes for elements and apply attributes to the field element diff --git a/__docs__/wonder-blocks-labeled-field/accessibility.mdx b/__docs__/wonder-blocks-labeled-field/accessibility.mdx new file mode 100644 index 000000000..b2341e8c0 --- /dev/null +++ b/__docs__/wonder-blocks-labeled-field/accessibility.mdx @@ -0,0 +1,20 @@ +import {Meta} from "@storybook/blocks"; + + +# LabeledField Accessibility + +The `LabeledField` component should be used in forms whenever possible. Benefits +for using this component include: +- Consistent styling for the label, description, and error message +- HTML attributes are wired up automatically for accessibility: + - A `label` element is used with the `for` attribute set to the + `id` of the field element + - The field element will have `aria-describedby` set to the element ids for + the description and error + +## Guidelines +- Make sure the `id` prop for `LabeledField` is unique to the page. If the `id` +prop is not provided, a unique id will be auto-generated! +- The `LabeledField` component does not need to be used with the `CheckboxGroup` + or `RadioGroup` components since those components already have accessible labels + built-in when the `label` prop is used. diff --git a/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx b/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx index 7b2bfe16d..5c0431071 100644 --- a/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx +++ b/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx @@ -22,7 +22,7 @@ import SearchField from "@khanacademy/wonder-blocks-search-field"; * `LabeledField` component so that our form fields are consistent and accessible. */ export default { - title: "Packages / LabeledField / LabeledField", + title: "Packages / LabeledField", component: LabeledField, parameters: { componentSubtitle: ( @@ -169,3 +169,62 @@ export const Light: StoryComponentType = { }, render: AllFields, }; + +/** + * The following story shows what the LabeledField looks like when different + * props are set. + */ +export const Scenarios = (args: PropsFor) => { + const [textFieldValue, setTextFieldValue] = React.useState(""); + + return ( + + + } + /> + + } + /> + {}} + validate={() => "Error message"} + /> + } + /> + {}} + validate={() => "Error message"} + /> + } + /> + + ); +}; diff --git a/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx b/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx index de5b9a385..2102e39ac 100644 --- a/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx +++ b/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx @@ -6,8 +6,13 @@ import {I18nInlineMarkup} from "@khanacademy/wonder-blocks-i18n"; import {Body} from "@khanacademy/wonder-blocks-typography"; import {TextField} from "@khanacademy/wonder-blocks-form"; +import {RenderStateRoot} from "@khanacademy/wonder-blocks-core"; import LabeledField from "../labeled-field"; +const defaultOptions = { + wrapper: RenderStateRoot, +}; + describe("LabeledField", () => { it("LabeledField renders the label text", () => { // Arrange @@ -19,6 +24,7 @@ describe("LabeledField", () => { field={ {}} />} label={label} />, + defaultOptions, ); // Assert @@ -36,6 +42,7 @@ describe("LabeledField", () => { label="Label" description={description} />, + defaultOptions, ); // Assert @@ -53,6 +60,7 @@ describe("LabeledField", () => { label="Label" error={error} />, + defaultOptions, ); // Assert @@ -70,6 +78,7 @@ describe("LabeledField", () => { label="Label" testId={testId} />, + defaultOptions, ); // Assert @@ -89,6 +98,7 @@ describe("LabeledField", () => { description="Description" testId={testId} />, + defaultOptions, ); // Assert @@ -108,6 +118,7 @@ describe("LabeledField", () => { error="Error" testId={testId} />, + defaultOptions, ); // Assert @@ -128,6 +139,7 @@ describe("LabeledField", () => { id={id} testId={testId} />, + defaultOptions, ); // Assert @@ -149,6 +161,7 @@ describe("LabeledField", () => { id={id} testId={testId} />, + defaultOptions, ); // Assert @@ -173,6 +186,7 @@ describe("LabeledField", () => { error="Error" style={styles.style1} />, + defaultOptions, ); // Assert @@ -193,6 +207,7 @@ describe("LabeledField", () => { } />, + defaultOptions, ); // Assert @@ -215,6 +230,7 @@ describe("LabeledField", () => { } />, + defaultOptions, ); // Assert @@ -222,4 +238,253 @@ describe("LabeledField", () => { // LabelSmall has a font-size of 16px expect(description).toHaveStyle("font-size: 14px"); }); + + describe("Attributes", () => { + const id = "example-id"; + const label = "Label"; + const description = "Description of the field"; + const error = "Error message"; + const testId = "test-id"; + + const getLabel = () => screen.getByText(label); + const getDescription = () => screen.getByText(description); + const getField = () => screen.getByRole("textbox"); + const getError = () => screen.getByRole("alert"); + + describe("id", () => { + it.each([ + ["label", `${id}-label`, getLabel], + ["description", `${id}-description`, getDescription], + ["field", `${id}-field`, getField], + ["error", `${id}-error`, getError], + ])( + "should have the id for the %s element set to %s", + ( + _elementType: string, // unused directly but is used for the test name using %s + expectedId: string, + getElement: () => HTMLElement, + ) => { + // Arrange + render( + {}} />} + id={id} + label={label} + description={description} + error={error} + />, + defaultOptions, + ); + + // Act + const el = getElement(); + + // Assert + expect(el.id).toBe(expectedId); + }, + ); + + it.each([ + ["label", "-label", getLabel], + ["description", "-description", getDescription], + ["field", "-field", getField], + ["error", "-error", getError], + ])( + "should have an auto-generated id for the %s element that ends with %s", + ( + _elementType: string, // unused directly but is used for the test name using %s + expectedPostfix: string, + getElement: () => HTMLElement, + ) => { + // Arrange + render( + {}} />} + label={label} + description={description} + error={error} + />, + defaultOptions, + ); + + // Act + const el = getElement(); + + // Assert + expect(el.id).toEndWith(expectedPostfix); + }, + ); + }); + + describe("testId", () => { + it.each([ + ["label", `${testId}-label`, getLabel], + ["description", `${testId}-description`, getDescription], + ["field", `${testId}-field`, getField], + ["error", `${testId}-error`, getError], + ])( + "should use the testId prop to set the %s element's data-testid attribute to %s", + ( + _elementType: string, // unused directly but is used for the test name using %s + expectedTestId: string, + getElement: () => HTMLElement, + ) => { + // Arrange + render( + {}} />} + testId={testId} + label={label} + description={description} + error={error} + />, + defaultOptions, + ); + + // Act + const el = getElement(); + + // Assert + expect(el).toHaveAttribute("data-testid", expectedTestId); + }, + ); + + it.each([ + ["label", getLabel], + ["description", getDescription], + ["field", getField], + ["error", getError], + ])( + "should not set the data-testid attribute on the %s element if the testId prop is not set", + ( + _elementType: string, // unused directly but is used for the test name using %s + getElement: () => HTMLElement, + ) => { + // Arrange + render( + {}} />} + label={label} + description={description} + error={error} + />, + defaultOptions, + ); + + // Act + const el = getElement(); + + // Assert + expect(el).not.toHaveAttribute("data-testid"); + }, + ); + }); + + it("should persist original attributes on the field if it is not overridden", () => { + // Arrange + render( + {}} + name="name-example" + /> + } + label="Label" + />, + defaultOptions, + ); + + // Act + const fieldEl = screen.getByRole("textbox"); + + // Assert + expect(fieldEl).toHaveAttribute("name", "name-example"); + }); + }); + + describe("Accessibility", () => { + describe("Axe", () => { + it("should have no accessibility violations", async () => { + // Arrange + const {container} = render( + {}} />} + label="Label" + description="Description for the field" + error="Error message" + />, + defaultOptions, + ); + // Act + + // Assert + await expect(container).toHaveNoA11yViolations(); + }); + }); + describe("ARIA", () => { + it("should render a label tag with the for attribute set to the id of the field", () => { + // Arrange + const label = "Label"; + render( + {}} />} + label={label} + />, + defaultOptions, + ); + + // Act + const labelEl = screen.getByText(label); + const inputEl = screen.getByRole("textbox"); + + // Assert + expect(labelEl).toHaveAttribute("for", inputEl.id); + }); + + it("should set aria-describedby on the field to the id of the description", () => { + // Arrange + const description = "Description of the field"; + render( + {}} />} + label="Label" + description={description} + />, + defaultOptions, + ); + + // Act + const descriptionEl = screen.getByText(description); + const inputEl = screen.getByRole("textbox"); + + // Assert + expect(inputEl).toHaveAttribute( + "aria-describedby", + descriptionEl.id, + ); + }); + + it("should set the aria-describedby on the field to the id of the error", () => { + // Arrange + const error = "Error message"; + render( + {}} />} + label="Label" + error={error} + />, + defaultOptions, + ); + + // Act + const errorEl = screen.getByRole("alert"); + const inputEl = screen.getByRole("textbox"); + + // Assert + expect(inputEl).toHaveAttribute("aria-describedby", errorEl.id); + }); + }); + }); }); diff --git a/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx b/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx index d0a2b4c69..2c4510837 100644 --- a/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx +++ b/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx @@ -1,7 +1,12 @@ import * as React from "react"; import {StyleSheet} from "aphrodite"; -import {View, addStyle, StyleType} from "@khanacademy/wonder-blocks-core"; +import { + View, + addStyle, + StyleType, + useUniqueIdWithMock, +} from "@khanacademy/wonder-blocks-core"; import {Strut} from "@khanacademy/wonder-blocks-layout"; import {color, spacing} from "@khanacademy/wonder-blocks-tokens"; import {LabelMedium, LabelSmall} from "@khanacademy/wonder-blocks-typography"; @@ -10,7 +15,7 @@ type Props = { /** * The form field component. */ - field: React.ReactNode; + field: React.ReactElement; /** * The title for the label element. */ @@ -32,14 +37,28 @@ type Props = { */ style?: StyleType; /** - * A unique id to link the label (and optional error) to the field. + * A unique id to use as the base of the ids for the elements within the component. + * Here is how the id is used for the different elements in the component: + * - The label will have an id formatted as `${id}-label` + * - The description will have an id formatted as `${id}-description` + * - The field will have an id formatted as `${id}-field` + * - The error will have an id formatted as `${id}-error` * - * The label will assume that the field will have its id formatted as `${id}-field`. - * The field can assume that the error will have its id formatted as `${id}-error`. + * If the `id` prop is not provided, a base unique id will be auto-generated. + * This is important so that the different elements can be wired up together + * for accessibility! + * + * Note: When using the `LabeledField` component, an `id` provided to the + * field component (ex: a TextField component) will be overridden. */ id?: string; /** - * Optional test ID for e2e testing. + * Optional test id for e2e testing. Here is how the test id is used for the + * different elements in the component: + * - The label will have a testId formatted as `${testId}-label` + * - The description will have a testId formatted as `${testId}-description` + * - The field will have a testId formatted as `${testId}-field` + * - The error will have a testId formatted as `${testId}-error` */ testId?: string; /** @@ -67,6 +86,13 @@ export default function LabeledField(props: Props) { error, } = props; + const ids = useUniqueIdWithMock("labeled-field"); + const uniqueId = id ?? ids.get("id"); + const labelId = `${uniqueId}-label`; + const descriptionId = `${uniqueId}-description`; + const fieldId = `${uniqueId}-field`; + const errorId = `${uniqueId}-error`; + function renderLabel(): React.ReactNode { const requiredIcon = ( {label} {required && requiredIcon} @@ -104,6 +131,7 @@ export default function LabeledField(props: Props) { {description} @@ -123,7 +151,7 @@ export default function LabeledField(props: Props) { {error} @@ -132,12 +160,23 @@ export default function LabeledField(props: Props) { ); } + function renderField() { + return React.cloneElement(field, { + id: fieldId, + "aria-describedby": [description && descriptionId, error && errorId] + .filter(Boolean) + .join(" "), + required, + testId: testId && `${testId}-field`, + }); + } + return ( {renderLabel()} {maybeRenderDescription()} - {field} + {renderField()} {maybeRenderError()} ); From 0869c5ee8724a7e9b79457c2249888f9f4cc670c Mon Sep 17 00:00:00 2001 From: Bea Esguerra Date: Fri, 1 Nov 2024 15:36:10 -0500 Subject: [PATCH 04/10] LabeledField(part4): styling and error icon (#2344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: - Add error icon to the error message based on the [Figma specs](https://www.figma.com/design/VbVu3h2BpBhH80niq101MHHE/%F0%9F%92%A0-Main-Components?m=auto&node-id=13497-10015&t=BL8QsHXT6zAhJf3S-1) - This will address accessibility issues reported by Level Access related to "Ensure color is not the sole means of communicating information" for error messages ([example finding from the report](https://khanacademy.hub.essentia11y.com/short-link/uX7pyw_Y0AZ3Z7tz)) - Add a `labels` prop so that a translated label for the error icon can be passed in - Use semantic color tokens where possible to match what's in Figma - Handle long text with and without word breaks Issue: WB-1503 ## Test plan: - The styling in the LabeledField stories match the design specs (`?path=/docs/packages-labeledfield--docs`) - The error icon has a default aria-label (`?path=/story/packages-labeledfield--default`) Screenshot 2024-10-10 at 3 27 18โ€ฏPM - The error icon uses the labels prop for the aria-label (if provided) (`?path=/story/packages-labeledfield--default&args=labels.errorIconAriaLabel:placeholder+for+translated+aria+label`) Screenshot 2024-10-10 at 3 20 59โ€ฏPM - Overflowing text cases wrap properly (`?path=/story/packages-labeledfield--scenarios`) image - Confirm that when a user focuses on an invalid field, the error icon aria label and the error message are both read out (tested with VO + Safari, NVDA + Firefox, NVDA + Chrome) https://github.com/user-attachments/assets/230fe5ad-36f8-448b-9031-c46ef250ad97 Author: beaesguerra Reviewers: beaesguerra, marcysutton, jandrade Required Reviewers: Approved By: jandrade Checks: โœ… Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), โœ… Lint / Lint (ubuntu-latest, 20.x), โœ… Test / Test (ubuntu-latest, 20.x, 2/2), โœ… Test / Test (ubuntu-latest, 20.x, 1/2), โœ… Check build sizes (ubuntu-latest, 20.x), โœ… Publish npm snapshot (ubuntu-latest, 20.x), โœ… Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), โœ… Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), โญ๏ธ Chromatic - Skip on Release PR (changesets), โœ… Check for .changeset entries for all changed files (ubuntu-latest, 20.x), โœ… gerald, โญ๏ธ dependabot Pull Request URL: https://github.com/Khan/wonder-blocks/pull/2344 --- .changeset/dull-elephants-march.md | 8 + .../accessibility.mdx | 7 + .../labeled-field.stories.tsx | 104 ++++++++-- .../__tests__/labeled-field.test.tsx | 190 ++++++++++++------ .../src/components/labeled-field.tsx | 107 ++++++++-- 5 files changed, 325 insertions(+), 91 deletions(-) create mode 100644 .changeset/dull-elephants-march.md diff --git a/.changeset/dull-elephants-march.md b/.changeset/dull-elephants-march.md new file mode 100644 index 000000000..c0bdb7626 --- /dev/null +++ b/.changeset/dull-elephants-march.md @@ -0,0 +1,8 @@ +--- +"@khanacademy/wonder-blocks-labeled-field": minor +--- + +- Update `LabeledField` styling to use semantic tokens to match Figma specs +- Add error icon to the error message. This addresses accessibility issues related to color being the only way to communicate information +- Add a labels prop so that a translated label for the error icon can be passed in +- Handle long text overflow with and without word breaks diff --git a/__docs__/wonder-blocks-labeled-field/accessibility.mdx b/__docs__/wonder-blocks-labeled-field/accessibility.mdx index b2341e8c0..c3eb7b9c5 100644 --- a/__docs__/wonder-blocks-labeled-field/accessibility.mdx +++ b/__docs__/wonder-blocks-labeled-field/accessibility.mdx @@ -11,6 +11,13 @@ for using this component include: `id` of the field element - The field element will have `aria-describedby` set to the element ids for the description and error +- An error icon for the error message so that color isn't the only way to +communicate information. The error icon has an aria-label of `Error:` by default +to prefix the provided error message. +- The error section which includes the icon and text has `aria-live=assertive` +and `aria-atomic=true` so that any updates to the error message are announced to +screen readers. + ## Guidelines - Make sure the `id` prop for `LabeledField` is unique to the page. If the `id` diff --git a/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx b/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx index 5c0431071..668c51966 100644 --- a/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx +++ b/__docs__/wonder-blocks-labeled-field/labeled-field.stories.tsx @@ -13,6 +13,13 @@ import { SingleSelect, } from "@khanacademy/wonder-blocks-dropdown"; import SearchField from "@khanacademy/wonder-blocks-search-field"; +import { + HeadingLarge, + HeadingMedium, + HeadingSmall, +} from "@khanacademy/wonder-blocks-typography"; +import {Strut} from "@khanacademy/wonder-blocks-layout"; +import Button from "@khanacademy/wonder-blocks-button"; /** * The `LabeledField` component provides common elements for a form field such @@ -41,8 +48,8 @@ export const Default: StoryComponentType = { args: { field: {}} />, label: "Name", - description: "Helpful description text", - error: "Error message", + description: "Helpful description text.", + error: "Message about the error", required: true, }, }; @@ -136,7 +143,7 @@ const AllFields = (args: PropsFor) => { */ export const Fields: StoryComponentType = { args: { - description: "Helpful description text", + description: "Helpful description text.", required: true, }, render: AllFields, @@ -144,8 +151,8 @@ export const Fields: StoryComponentType = { export const Error: StoryComponentType = { args: { - description: "Helpful description text", - error: "Error message", + description: "Helpful description text.", + error: "Message about the error", required: true, }, render: AllFields, @@ -159,8 +166,8 @@ export const Error: StoryComponentType = { */ export const Light: StoryComponentType = { args: { - description: "Helpful description text", - error: "Error message", + description: "Helpful description text.", + error: "Message about the error", required: true, light: true, }, @@ -170,15 +177,57 @@ export const Light: StoryComponentType = { render: AllFields, }; +/** + * When this story is used with a screen reader, any updates to an existing + * error message will be announced. + */ +export const ChangingErrors: StoryComponentType = () => { + const errorMsg1 = "First error message"; + const errorMsg2 = "Second error message"; + + const [errorMessage, setErrorMessage] = React.useState(errorMsg1); + + return ( + + {}} />} + error={errorMessage} + /> + + + + ); +}; + +ChangingErrors.parameters = { + chromatic: { + // Disabling because this doesn't test anything visual. + disableSnapshot: true, + }, +}; + /** * The following story shows what the LabeledField looks like when different * props are set. */ export const Scenarios = (args: PropsFor) => { const [textFieldValue, setTextFieldValue] = React.useState(""); - + const longText = + "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur."; + const longTextWithNoBreak = + "LoremipsumdolorsitametconsecteturadipiscingelitseddoeiusmodtemporincididuntutlaboreetdoloremagnaaliquaUtenimadminimveniamquisnostrudexercitationullamcolaborisnisiutaliquipexeacommodoconsequatDuisauteiruredolorinreprehenderitinvoluptatevelitessecillumdoloreeufugiatnullapariatur"; return ( + Scenarios ) => { {}} - validate={() => "Error message"} + validate={() => "Message about the error"} /> } /> {}} - validate={() => "Error message"} + validate={() => "Message about the error"} + /> + } + /> + Text Scenarios + With Long Text + {}} + validate={() => "Message about the error"} + /> + } + /> + With Long Text and No Word Break + {}} + validate={() => "Message about the error"} /> } /> diff --git a/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx b/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx index 2102e39ac..44c541422 100644 --- a/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx +++ b/packages/wonder-blocks-labeled-field/src/components/__tests__/labeled-field.test.tsx @@ -1,5 +1,5 @@ import * as React from "react"; -import {render, screen} from "@testing-library/react"; +import {render, screen, within} from "@testing-library/react"; import {StyleSheet} from "aphrodite"; import {I18nInlineMarkup} from "@khanacademy/wonder-blocks-i18n"; @@ -14,6 +14,17 @@ const defaultOptions = { }; describe("LabeledField", () => { + const id = "example-id"; + const label = "Label"; + const description = "Description of the field"; + const error = "Error message"; + const testId = "test-id"; + + const getLabel = () => screen.getByText(label); + const getDescription = () => screen.getByText(description); + const getField = () => screen.getByRole("textbox"); + const getError = () => screen.getByTestId("test-id-error"); + it("LabeledField renders the label text", () => { // Arrange const label = "Label"; @@ -64,7 +75,7 @@ describe("LabeledField", () => { ); // Assert - expect(screen.getByRole("alert")).toBeInTheDocument(); + expect(screen.getByText(error)).toBeInTheDocument(); }); it("LabeledField adds testId to label", () => { @@ -126,49 +137,6 @@ describe("LabeledField", () => { expect(error).toBeInTheDocument(); }); - it("LabeledField adds the correctly formatted id to label's htmlFor", () => { - // Arrange - const id = "exampleid"; - const testId = "testid"; - - // Act - render( - {}} />} - label="Label" - id={id} - testId={testId} - />, - defaultOptions, - ); - - // Assert - const label = screen.getByTestId(`${testId}-label`); - expect(label).toHaveAttribute("for", `${id}-field`); - }); - - it("LabeledField adds the correctly formatted id to error's id", () => { - // Arrange - const id = "exampleid"; - const testId = "testid"; - - // Act - render( - {}} />} - label="Label" - error="Error" - id={id} - testId={testId} - />, - defaultOptions, - ); - - // Assert - const error = screen.getByRole("alert"); - expect(error).toHaveAttribute("id", `${id}-error`); - }); - it("stype prop applies to the LabeledField container", () => { // Arrange const styles = StyleSheet.create({ @@ -239,18 +207,50 @@ describe("LabeledField", () => { expect(description).toHaveStyle("font-size: 14px"); }); - describe("Attributes", () => { - const id = "example-id"; - const label = "Label"; - const description = "Description of the field"; - const error = "Error message"; - const testId = "test-id"; + describe("Labels prop", () => { + it("should use the errorIconAriaLabel for the error icon aria label", () => { + // Arrange + const errorIconAriaLabel = "Placeholder for translated aria label"; + render( + {}} />} + label="Label" + error="Error message" + labels={{errorIconAriaLabel}} + />, + defaultOptions, + ); + + // Act + const errorIcon = screen.getByRole("img"); + + // Assert + expect(errorIcon).toHaveAttribute("aria-label", errorIconAriaLabel); + }); + + it("should use a default aria-label on the error icon if the errorIconAriaLabel is not provided", () => { + // Arrange + render( + {}} />} + label="Label" + error="Error message" + testId="labeled-field" + />, + defaultOptions, + ); + + // Act + // Get the icon within the error section + const error = screen.getByTestId("labeled-field-error"); + const errorIcon = within(error).getByRole("img"); - const getLabel = () => screen.getByText(label); - const getDescription = () => screen.getByText(description); - const getField = () => screen.getByRole("textbox"); - const getError = () => screen.getByRole("alert"); + // Assert + expect(errorIcon).toHaveAttribute("aria-label", "Error:"); + }); + }); + describe("Attributes", () => { describe("id", () => { it.each([ ["label", `${id}-label`, getLabel], @@ -272,6 +272,7 @@ describe("LabeledField", () => { label={label} description={description} error={error} + testId={testId} />, defaultOptions, ); @@ -303,6 +304,7 @@ describe("LabeledField", () => { label={label} description={description} error={error} + testId={testId} />, defaultOptions, ); @@ -353,7 +355,22 @@ describe("LabeledField", () => { ["label", getLabel], ["description", getDescription], ["field", getField], - ["error", getError], + [ + "error", + () => { + // In order to get the error section (icon + message) + // without using testId, we get the parent of the error + // text + // eslint-disable-next-line testing-library/no-node-access + const el = screen.getByText(error).parentElement; + if (!el) { + throw Error( + "Error section in LabeledField not found", + ); + } + return el; + }, + ], ])( "should not set the data-testid attribute on the %s element if the testId prop is not set", ( @@ -466,7 +483,7 @@ describe("LabeledField", () => { ); }); - it("should set the aria-describedby on the field to the id of the error", () => { + it("should set the aria-describedby on the field to the id of the error section", () => { // Arrange const error = "Error message"; render( @@ -474,16 +491,69 @@ describe("LabeledField", () => { field={ {}} />} label="Label" error={error} + testId="labeled-field" />, defaultOptions, ); // Act - const errorEl = screen.getByRole("alert"); - const inputEl = screen.getByRole("textbox"); + const errorSectionEl = screen.getByTestId( + "labeled-field-error", + ); + const inputEl = getField(); + + // Assert + expect(inputEl).toHaveAttribute( + "aria-describedby", + errorSectionEl.id, + ); + }); + + it("should have aria-live=assertive set on the error section", () => { + // Arrange + const error = "Error message"; + render( + {}} />} + label="Label" + error={error} + testId="labeled-field" + />, + defaultOptions, + ); + + // Act + const errorSectionEl = screen.getByTestId( + "labeled-field-error", + ); + + // Assert + expect(errorSectionEl).toHaveAttribute( + "aria-live", + "assertive", + ); + }); + + it("should have aria-atomic=true set on the error section", () => { + // Arrange + const error = "Error message"; + render( + {}} />} + label="Label" + error={error} + testId="labeled-field" + />, + defaultOptions, + ); + + // Act + const errorSectionEl = screen.getByTestId( + "labeled-field-error", + ); // Assert - expect(inputEl).toHaveAttribute("aria-describedby", errorEl.id); + expect(errorSectionEl).toHaveAttribute("aria-atomic", "true"); }); }); }); diff --git a/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx b/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx index 2c4510837..ec8f0aa4a 100644 --- a/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx +++ b/packages/wonder-blocks-labeled-field/src/components/labeled-field.tsx @@ -1,5 +1,6 @@ import * as React from "react"; import {StyleSheet} from "aphrodite"; +import WarningCircle from "@phosphor-icons/core/bold/warning-circle-bold.svg"; import { View, @@ -8,8 +9,9 @@ import { useUniqueIdWithMock, } from "@khanacademy/wonder-blocks-core"; import {Strut} from "@khanacademy/wonder-blocks-layout"; -import {color, spacing} from "@khanacademy/wonder-blocks-tokens"; +import {color, semanticColor, spacing} from "@khanacademy/wonder-blocks-tokens"; import {LabelMedium, LabelSmall} from "@khanacademy/wonder-blocks-typography"; +import {PhosphorIcon} from "@khanacademy/wonder-blocks-icon"; type Props = { /** @@ -30,6 +32,10 @@ type Props = { required?: boolean; /** * The message for the error element. + * + * Note: Since the error icon has an aria-label, screen readers will + * prefix the error message with "Error:" (or the value provided to the + * errorIconAriaLabel in the `labels` prop) */ error?: React.ReactNode; /** @@ -65,6 +71,20 @@ type Props = { * Change the fieldโ€™s sub-components to fit a dark background. */ light?: boolean; + /** + * The object containing the custom labels used inside this component. + * + * This is useful for internationalization. Defaults to English. + */ + labels?: LabeledFieldLabels; +}; + +export type LabeledFieldLabels = { + errorIconAriaLabel: string; +}; + +const defaultLabeledFieldLabels: LabeledFieldLabels = { + errorIconAriaLabel: "Error:", }; const StyledSpan = addStyle("span"); @@ -84,6 +104,7 @@ export default function LabeledField(props: Props) { light, description, error, + labels = defaultLabeledFieldLabels, } = props; const ids = useUniqueIdWithMock("labeled-field"); @@ -96,7 +117,10 @@ export default function LabeledField(props: Props) { function renderLabel(): React.ReactNode { const requiredIcon = ( {" "} @@ -107,7 +131,10 @@ export default function LabeledField(props: Props) { return ( @@ -141,21 +171,47 @@ export default function LabeledField(props: Props) { } function maybeRenderError(): React.ReactNode | null | undefined { - if (!error) { - return null; - } - return ( - - {error} - + {error && ( + <> + + + {error} + + + )} + ); } @@ -163,7 +219,7 @@ export default function LabeledField(props: Props) { function renderField() { return React.cloneElement(field, { id: fieldId, - "aria-describedby": [description && descriptionId, error && errorId] + "aria-describedby": [error && errorId, description && descriptionId] .filter(Boolean) .join(" "), required, @@ -184,27 +240,40 @@ export default function LabeledField(props: Props) { const styles = StyleSheet.create({ label: { - color: color.offBlack, + color: semanticColor.text.primary, }, lightLabel: { - color: color.white, + color: semanticColor.text.inverse, }, description: { - color: color.offBlack64, + color: semanticColor.text.secondary, }, lightDescription: { color: color.white64, }, + errorSection: { + flexDirection: "row", + gap: spacing.xSmall_8, + }, error: { - color: color.red, + color: semanticColor.status.critical.foreground, }, lightError: { color: color.fadedRed, }, + errorIcon: { + marginTop: "1px", // This vertically aligns the icon with the text + }, + errorMessage: { + minWidth: "0", // This enables the wrapping behaviour on the error message + }, required: { - color: color.red, + color: semanticColor.status.critical.foreground, }, lightRequired: { color: color.fadedRed, }, + textWordBreak: { + overflowWrap: "break-word", + }, }); From d9bc865b924aea41f1effd7f88c6e6da0cb8185f Mon Sep 17 00:00:00 2001 From: Bea Esguerra Date: Thu, 19 Dec 2024 14:43:16 -0700 Subject: [PATCH 05/10] LabeledField: integrate with field components and fixes (#2399) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Putting LabeledField work together with field updates and addressing some issues. Fixes include: TextField and TextArea: - Set `aria-required` based on `required` prop - Always clear error state onChange if instantValidation is false so that external error messages set using the `error` prop can still be cleared (This is useful for clearing any backend errors when the input value changes) LabeledField updates: - Let `required` prop be a boolean or string so it can be passed down to the field prop - Conditionally set spacing above the error message depending on if there is an error message - Set `required`, `error` and `light` props for LabeledField and field component if it is set on either LabeledField or field component - Rename `error` prop to `errorMessage` (This technically isn't a breaking change since the batch of labeled field work hasn't been released yet, labeled field work is currently in the `feature/labeled-field` branch) Stories/docs: - Add docs on best practices for forms and fields - Add docs around LabeledTextField deprecation in favour of LabeledField + TextField - Update LabeledField examples and docs. Includes examples for validation and moving focus to the first field that has an error message after form submission Issue: WB-1783 ## Test plan: 1. Review new docs: - LabeledField (`?path=/docs/packages-labeledfield--docs`) - LabeledTextField (`/?path=/docs/packages-form-labeledtextfield-deprecated--docs`) - Form Best Practices (`?path=/docs/packages-form-overview--docs`) 2. Test LabeledField with Field Validation: Confirm validation works as expected - LabeledField Required story (`?path=/story/packages-labeledfield--required`) - Tabbing through fields without entering anything - Submitting without filling in fields - Inputting/selecting a value and removing it - LabeledField Validation story (`?path=/story/packages-labeledfield--validation`) - Triggering validation errors - Clearing validation errors when selected value changes - Submitting the form to see an example for backend errors Note: Did some preliminary testing with LabeledField and new validation features. Different screen reader and browser combinations had different behaviours around announcing errors so I created WB-1842 for more testing + documentation. Author: beaesguerra Reviewers: beaesguerra, jandrade Required Reviewers: Approved By: jandrade Checks: โœ… Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), โœ… Lint / Lint (ubuntu-latest, 20.x), โœ… Test / Test (ubuntu-latest, 20.x, 2/2), โœ… Test / Test (ubuntu-latest, 20.x, 1/2), โœ… Check build sizes (ubuntu-latest, 20.x), โœ… Publish npm snapshot (ubuntu-latest, 20.x), โœ… Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), โญ๏ธ Chromatic - Skip on Release PR (changesets), โœ… Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), โœ… Check for .changeset entries for all changed files (ubuntu-latest, 20.x), โœ… gerald, โญ๏ธ dependabot Pull Request URL: https://github.com/Khan/wonder-blocks/pull/2399 --- .changeset/chilly-mirrors-add.md | 5 + .changeset/metal-maps-move.md | 5 + .changeset/slow-otters-crash.md | 5 + .changeset/smart-grapes-serve.md | 5 + .changeset/spicy-rivers-marry.md | 5 + __docs__/wonder-blocks-form/_overview_.mdx | 52 ++- __docs__/wonder-blocks-form/accessibility.mdx | 34 -- .../accessibility.stories.tsx | 2 +- .../labeled-text-field.stories.tsx | 74 +++- .../labeled-field.stories.tsx | 334 ++++++++++++++++-- .../components/__tests__/text-area.test.tsx | 72 +++- .../components/__tests__/text-field.test.tsx | 68 +++- .../src/components/text-area.tsx | 1 + .../src/components/text-field.tsx | 1 + .../src/hooks/use-field-validation.ts | 14 +- .../__tests__/labeled-field.test.tsx | 219 ++++++++---- .../src/components/labeled-field.tsx | 89 +++-- .../tsconfig-build.json | 1 - 18 files changed, 791 insertions(+), 195 deletions(-) create mode 100644 .changeset/chilly-mirrors-add.md create mode 100644 .changeset/metal-maps-move.md create mode 100644 .changeset/slow-otters-crash.md create mode 100644 .changeset/smart-grapes-serve.md create mode 100644 .changeset/spicy-rivers-marry.md delete mode 100644 __docs__/wonder-blocks-form/accessibility.mdx diff --git a/.changeset/chilly-mirrors-add.md b/.changeset/chilly-mirrors-add.md new file mode 100644 index 000000000..4d82d7b9d --- /dev/null +++ b/.changeset/chilly-mirrors-add.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-form": patch +--- + +TextField and TextArea: Set `aria-required` if it is required diff --git a/.changeset/metal-maps-move.md b/.changeset/metal-maps-move.md new file mode 100644 index 000000000..f4a0999de --- /dev/null +++ b/.changeset/metal-maps-move.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-form": patch +--- + +TextField and TextArea validation: Always clear error message onChange if instantValidation=false so externally set error state can still be cleared diff --git a/.changeset/slow-otters-crash.md b/.changeset/slow-otters-crash.md new file mode 100644 index 000000000..0888206d7 --- /dev/null +++ b/.changeset/slow-otters-crash.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-labeled-field": patch +--- + +Set required, error and light props for LabeledField and field component if it is set on either LabeledField or field component diff --git a/.changeset/smart-grapes-serve.md b/.changeset/smart-grapes-serve.md new file mode 100644 index 000000000..5835a09e5 --- /dev/null +++ b/.changeset/smart-grapes-serve.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-labeled-field": patch +--- + +Use `errorMessage` prop instead of `error` prop for consistency (`error` prop is used for boolean props in form field components). diff --git a/.changeset/spicy-rivers-marry.md b/.changeset/spicy-rivers-marry.md new file mode 100644 index 000000000..be94ec3c4 --- /dev/null +++ b/.changeset/spicy-rivers-marry.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/wonder-blocks-labeled-field": patch +--- + +LabeledField: Let `required` prop be a boolean or string so it can be passed down to the field prop diff --git a/__docs__/wonder-blocks-form/_overview_.mdx b/__docs__/wonder-blocks-form/_overview_.mdx index 78c7e733d..cdece2c84 100644 --- a/__docs__/wonder-blocks-form/_overview_.mdx +++ b/__docs__/wonder-blocks-form/_overview_.mdx @@ -1,4 +1,6 @@ -import {Meta} from "@storybook/blocks"; +import {Meta, Story, Canvas} from "@storybook/blocks"; +import * as AccessibilityStories from './accessibility.stories'; +import * as LabeledFieldStories from '../wonder-blocks-labeled-field/labeled-field.stories'; ` and +`` elements instead of a `