Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate accessible field labels in development #1591

Merged
merged 2 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/shiny-peas-love.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'braid-design-system': patch
---

---
updated:
- Autosuggest
- Dropdown
- MonthPicker
- PasswordField
- RadioGroup
- TextField
- Textarea
---

Validate accessible field labels in development

Introduce development-time validation for the `label` prop on form fields to guard against blank values and guide towards the alternative labelling options that are available.
This validation helps ensure that all fields are accessible to screen readers and other assistive technologies.
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,7 @@ export const Autosuggest = forwardRef(function <Value>(
<Box position="relative" ref={rootRef}>
<Field
{...restProps}
componentName="Autosuggest"
id={id}
value={value.text}
prefix={undefined}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const Dropdown = forwardRef<HTMLSelectElement, DropdownProps>(
return (
<Field
{...restProps}
componentName="Dropdown"
disabled={disabled}
prefix={undefined}
secondaryMessage={null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ const MonthPicker = ({
disabled={disabled}
value={customValueToString(currentValue)}
{...restProps}
componentName="MonthPicker"
icon={undefined}
prefix={undefined}
name={undefined}
Expand All @@ -227,7 +228,13 @@ const MonthPicker = ({
);

const customFieldGroup = (
<FieldGroup id={id} tone={tone} disabled={disabled} {...restProps}>
<FieldGroup
id={id}
tone={tone}
disabled={disabled}
componentName="MonthPicker"
{...restProps}
>
{(fieldGroupProps) => (
<Columns space="xsmall">
<Column>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export const PasswordField = forwardRef<HTMLInputElement, PasswordFieldProps>(
return (
<Field
{...restProps}
componentName="PasswordField"
id={id}
value={value}
icon={undefined}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const RadioGroup = ({
<FieldGroup
id={id}
{...props}
componentName="RadioGroup"
disabled={disabled}
tone={tone}
role="radiogroup"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export const TextField = forwardRef<HTMLInputElement, TextFieldProps>(
return (
<Field
{...restProps}
componentName="TextField"
id={id}
value={value}
secondaryMessage={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export const Textarea = forwardRef<HTMLTextAreaElement, TextareaProps>(
return (
<Field
{...restProps}
componentName="Textarea"
tone={tone}
value={value}
icon={undefined}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import dedent from 'dedent';
import assert from 'assert';
import React, { type ReactNode, type AllHTMLAttributes, Fragment } from 'react';
import clsx from 'clsx';
Expand Down Expand Up @@ -88,6 +89,7 @@ type InternalFieldProps = FieldBaseProps &
secondaryIcon: ReactNode,
prefix: ReactNode,
): ReactNode;
componentName: string;
};

export const Field = ({
Expand All @@ -110,13 +112,34 @@ export const Field = ({
icon,
prefix,
required,
componentName,
...restProps
}: InternalFieldProps) => {
assert(
prefix === undefined || typeof prefix === 'string',
'Prefix must be a string',
);

if (process.env.NODE_ENV !== 'production') {
if (
'label' in restProps &&
typeof restProps.label === 'string' &&
restProps.label.trim().length === 0 &&
!('aria-label' in restProps) &&
!('aria-labelledby' in restProps)
) {
// eslint-disable-next-line no-console
console.warn(
dedent`
The "label" prop is required as the accessible name for a ${componentName}.
If you are labelling the ${componentName} with another element or using a non-visual label please provide the appropriate props, either "aria-label" or "aria-labelledby".

See the ${componentName} documentation for more information: https://seek-oss.github.io/braid-design-system/components/${componentName}#indirect-or-hidden-field-labels
`,
);
}
}

const messageId = `${id}-message`;
const descriptionId = description ? `${id}-description` : undefined;
const fieldBackground: BoxProps['background'] = disabled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import buildDataAttributes, {
} from '../buildDataAttributes';
import { mergeIds } from '../mergeIds';
import type { ReactNodeNoStrings } from '../ReactNodeNoStrings';
import dedent from 'dedent';

type FormElementProps = AllHTMLAttributes<HTMLFormElement>;

Expand Down Expand Up @@ -52,6 +53,7 @@ type InternalFieldGroupProps = FieldGroupBaseProps &
role?: FormElementProps['role'];
messageSpace?: StackProps['space'];
children(props: FieldGroupRenderProps): ReactNodeNoStrings;
componentName: string;
};

export const FieldGroup = ({
Expand All @@ -68,6 +70,7 @@ export const FieldGroup = ({
required,
role,
data,
componentName,
...restProps
}: InternalFieldGroupProps) => {
const labelId = `${id}-label`;
Expand All @@ -84,6 +87,26 @@ export const FieldGroup = ({
ariaLabel = restProps['aria-label'];
}

if (process.env.NODE_ENV !== 'production') {
if (
'label' in restProps &&
typeof restProps.label === 'string' &&
restProps.label.trim().length === 0 &&
!('aria-label' in restProps) &&
!('aria-labelledby' in restProps)
) {
// eslint-disable-next-line no-console
console.warn(
dedent`
The "label" prop is required as the accessible name for a ${componentName}.
If you are labelling the ${componentName} with another element or using a non-visual label please provide the appropriate props, either "aria-label" or "aria-labelledby".

See the ${componentName} documentation for more information: https://seek-oss.github.io/braid-design-system/components/${componentName}#indirect-or-hidden-field-labels
`,
);
}
}

return (
<Box
component="fieldset"
Expand Down