Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changeset/empty-laws-boil.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Ensure inputs are properly connected to feedback messages via `aria-describedby` usage.
3 changes: 1 addition & 2 deletions packages/clerk-js/src/ui/elements/FieldControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,11 @@ const FieldLabelRow = (props: PropsWithChildren) => {
};

const FieldFeedback = (props: Pick<FormFeedbackProps, 'elementDescriptors' | 'center'>) => {
const { fieldId, debouncedFeedback, errorMessageId } = useFormField();
const { fieldId, debouncedFeedback } = useFormField();

return (
<FormFeedback
center={props.center}
errorMessageId={errorMessageId}
{...{
...debouncedFeedback,
elementDescriptors: props.elementDescriptors,
Expand Down
13 changes: 6 additions & 7 deletions packages/clerk-js/src/ui/elements/FormControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function useFormTextAnimation() {
transitionTimingFunction: t.transitionTiming.$common,
});
},
[prefersReducedMotion],
[prefersReducedMotion, appearanceAnimations],
);

return {
Expand Down Expand Up @@ -71,14 +71,13 @@ export type FormFeedbackDescriptorsKeys = 'error' | 'warning' | 'info' | 'succes
type Feedback = { feedback?: string; feedbackType?: FeedbackType; shouldEnter: boolean };

export type FormFeedbackProps = Partial<ReturnType<typeof useFormControlFeedback>['debounced'] & { id: FieldId }> & {
errorMessageId?: string;
elementDescriptors?: Partial<Record<FormFeedbackDescriptorsKeys, ElementDescriptor>>;
center?: boolean;
sx?: ThemableCssProp;
};

export const FormFeedback = (props: FormFeedbackProps) => {
const { id, elementDescriptors, sx, feedback, feedbackType = 'info', center = false, errorMessageId } = props;
const { id, elementDescriptors, sx, feedback, feedbackType = 'info', center = false } = props;
const feedbacksRef = useRef<{
a?: Feedback;
b?: Feedback;
Expand Down Expand Up @@ -143,10 +142,8 @@ export const FormFeedback = (props: FormFeedbackProps) => {
return {
elementDescriptor: descriptor,
elementId: id ? descriptor?.setId?.(id) : undefined,
// We only want the id applied when the feedback type is an error
// to avoid having multiple elements in the dom with the same id attribute.
// We also only have aria-describedby applied to the input when it is an error.
id: type === 'error' ? errorMessageId : undefined,
// Use legacy pattern for errors (backwards compatible), new pattern for other types
id: type === 'error' ? `error-${id}` : `${id}-${type}-feedback`,
};
};

Expand Down Expand Up @@ -182,6 +179,7 @@ export const FormFeedback = (props: FormFeedbackProps) => {
getFormTextAnimation(!!feedbacks.a?.shouldEnter, { inDelay: true }),
]}
localizationKey={titleize(feedbacks.a?.feedback)}
aria-live={feedbacks.a?.shouldEnter ? 'polite' : 'off'}
/>
<InfoComponentB
{...getElementProps(feedbacks.b?.feedbackType)}
Expand All @@ -193,6 +191,7 @@ export const FormFeedback = (props: FormFeedbackProps) => {
getFormTextAnimation(!!feedbacks.b?.shouldEnter, { inDelay: true }),
]}
localizationKey={titleize(feedbacks.b?.feedback)}
aria-live={feedbacks.b?.shouldEnter ? 'polite' : 'off'}
/>
</Flex>
);
Expand Down
104 changes: 100 additions & 4 deletions packages/clerk-js/src/ui/elements/__tests__/PlainInput.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ const createField = (...params: Parameters<typeof useFormControl>) => {
{...props}
/>
<button onClick={() => field.setError('some error')}>set error</button>
<button onClick={() => field.setSuccess('some success')}>set success</button>
<button onClick={() => field.setWarning('some warning')}>set warning</button>
<button onClick={() => field.setInfo('some info')}>set info</button>
</>
);
});
Expand Down Expand Up @@ -132,15 +135,20 @@ describe('PlainInput', () => {
placeholder: 'some placeholder',
});

const { getByRole, getByLabelText, findByText } = render(<Field />, { wrapper });
const { getByRole, getByLabelText, findByText, container } = render(<Field />, { wrapper });

await userEvent.click(getByRole('button', { name: /set error/i }));

expect(await findByText(/Some Error/i)).toBeInTheDocument();

const label = getByLabelText(/some label/i);
expect(label).toHaveAttribute('aria-invalid', 'true');
expect(label).toHaveAttribute('aria-describedby', 'error-firstname');
const input = getByLabelText(/some label/i);
expect(input).toHaveAttribute('aria-invalid', 'true');
expect(input).toHaveAttribute('aria-describedby', 'error-firstname');

// Verify the error message element has the correct ID
const errorElement = container.querySelector('#error-firstname');
expect(errorElement).toBeInTheDocument();
expect(errorElement).toHaveTextContent(/Some Error/i);
});

it('with info', async () => {
Expand All @@ -157,4 +165,92 @@ describe('PlainInput', () => {
fireEvent.focus(await findByLabelText(/some label/i));
expect(await findByText(/some info/i)).toBeInTheDocument();
});

it('with success feedback and aria-describedby', async () => {
const { wrapper } = await createFixtures();
const { Field } = createField('firstname', 'init value', {
type: 'text',
label: 'some label',
placeholder: 'some placeholder',
});

const { getByRole, getByLabelText, findByText, container } = render(<Field />, { wrapper });

await userEvent.click(getByRole('button', { name: /set success/i }));

expect(await findByText(/Some Success/i)).toBeInTheDocument();

const input = getByLabelText(/some label/i);
expect(input).toHaveAttribute('aria-invalid', 'false');
expect(input).toHaveAttribute('aria-describedby', 'firstname-success-feedback');

// Verify the success message element has the correct ID
const successElement = container.querySelector('#firstname-success-feedback');
expect(successElement).toBeInTheDocument();
expect(successElement).toHaveTextContent(/Some Success/i);
});

it('transitions between error and success feedback types', async () => {
const { wrapper } = await createFixtures();
const { Field } = createField('firstname', 'init value', {
type: 'text',
label: 'some label',
placeholder: 'some placeholder',
});

const { getByRole, getByLabelText, findByText, container } = render(<Field />, { wrapper });

// Start with error
await userEvent.click(getByRole('button', { name: /set error/i }));
expect(await findByText(/Some Error/i)).toBeInTheDocument();

let input = getByLabelText(/some label/i);
expect(input).toHaveAttribute('aria-invalid', 'true');
expect(input).toHaveAttribute('aria-describedby', 'error-firstname');

// Transition to success
await userEvent.click(getByRole('button', { name: /set success/i }));
expect(await findByText(/Some Success/i)).toBeInTheDocument();

input = getByLabelText(/some label/i);
expect(input).toHaveAttribute('aria-invalid', 'false');
expect(input).toHaveAttribute('aria-describedby', 'firstname-success-feedback');

// Verify success element exists with proper ID
const successElement = container.querySelector('#firstname-success-feedback');
expect(successElement).toBeInTheDocument();
expect(successElement).toHaveTextContent(/Some Success/i);
});

it('aria-live attribute is correctly applied', async () => {
const { wrapper } = await createFixtures();
const { Field } = createField('firstname', 'init value', {
type: 'text',
label: 'some label',
placeholder: 'some placeholder',
});

const { getByRole, findByText, container } = render(<Field />, { wrapper });

// Set error feedback
await userEvent.click(getByRole('button', { name: /set error/i }));
expect(await findByText(/Some Error/i)).toBeInTheDocument();

// Verify the visible error message has aria-live="polite"
const errorElement = container.querySelector('#error-firstname');
expect(errorElement).toHaveAttribute('aria-live', 'polite');

// Transition to success
await userEvent.click(getByRole('button', { name: /set success/i }));
expect(await findByText(/Some Success/i)).toBeInTheDocument();

// Verify the visible success message has aria-live="polite"
const successElement = container.querySelector('#firstname-success-feedback');
expect(successElement).toHaveAttribute('aria-live', 'polite');

// The previous error message should now have aria-live="off" (though it might still exist in DOM but hidden)
// Verify exactly one element has aria-live="polite" at a time
const allAriaLivePolite = container.querySelectorAll('[aria-live="polite"]');
expect(allAriaLivePolite.length).toBe(1);
});
});
4 changes: 1 addition & 3 deletions packages/clerk-js/src/ui/primitives/FormErrorText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const { applyVariants } = createVariants(theme => ({
type FormErrorTextProps = React.PropsWithChildren<StyleVariants<typeof applyVariants>>;

export const FormErrorText = forwardRef<HTMLElement, FormErrorTextProps>((props, ref) => {
const { hasError, errorMessageId } = useFormField() || {};
const { hasError } = useFormField() || {};

if (!hasError && !props.children) {
return null;
Expand All @@ -35,8 +35,6 @@ export const FormErrorText = forwardRef<HTMLElement, FormErrorTextProps>((props,
<Text
ref={ref}
colorScheme='danger'
aria-live='polite'
id={errorMessageId}
{...rest}
css={applyVariants(props)}
>
Expand Down
4 changes: 1 addition & 3 deletions packages/clerk-js/src/ui/primitives/FormInfoText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { useFormField } from './hooks/useFormField';
import { Text } from './Text';

export const FormInfoText = forwardRef<HTMLElement, FormTextProps>((props, ref) => {
const { hasError, errorMessageId } = useFormField() || {};
const { hasError } = useFormField() || {};

if (!hasError && !props.children) {
return null;
Expand All @@ -16,8 +16,6 @@ export const FormInfoText = forwardRef<HTMLElement, FormTextProps>((props, ref)
<Text
ref={ref}
colorScheme='secondary'
aria-live='polite'
id={errorMessageId}
{...props}
css={applyVariants(props)}
/>
Expand Down
1 change: 0 additions & 1 deletion packages/clerk-js/src/ui/primitives/FormSuccessText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export const FormSuccessText = forwardRef<HTMLElement, FormTextProps>((props, re
<Text
ref={ref}
colorScheme='secondary'
aria-live='polite'
{...rest}
css={applyVariants(props) as any}
>
Expand Down
1 change: 0 additions & 1 deletion packages/clerk-js/src/ui/primitives/FormWarningText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export const FormWarningText = forwardRef<HTMLElement, FormTextProps>((props, re
<Text
ref={ref}
colorScheme='secondary'
aria-live='polite'
{...rest}
css={applyVariants(props) as any}
>
Expand Down
8 changes: 4 additions & 4 deletions packages/clerk-js/src/ui/primitives/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ export type InputProps = PrimitiveProps<'input'> & StyleVariants<typeof applyVar

export const Input = React.forwardRef<HTMLInputElement, InputProps>((props, ref) => {
const fieldControl = useFormField() || {};
// @ts-expect-error Typescript is complaining that `errorMessageId` does not exist. We are clearly passing them from above.
const { errorMessageId, ignorePasswordManager, feedbackType, ...fieldControlProps } = sanitizeInputProps(
// @ts-expect-error Typescript is complaining that `feedbackMessageId` does not exist. We are clearly passing them from above.
const { feedbackMessageId, ignorePasswordManager, feedbackType, ...fieldControlProps } = sanitizeInputProps(
fieldControl,
['errorMessageId', 'ignorePasswordManager', 'feedbackType'],
['feedbackMessageId', 'ignorePasswordManager', 'feedbackType'],
);

const propsWithoutVariants = filterProps({
Expand Down Expand Up @@ -106,7 +106,7 @@ export const Input = React.forwardRef<HTMLInputElement, InputProps>((props, ref)
required={_required}
id={props.id || fieldControlProps.id}
aria-invalid={_hasError}
aria-describedby={errorMessageId ? errorMessageId : undefined}
aria-describedby={feedbackMessageId || undefined}
aria-required={_required}
aria-disabled={_disabled}
data-feedback={feedbackType}
Expand Down
19 changes: 12 additions & 7 deletions packages/clerk-js/src/ui/primitives/hooks/useFormField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ type FormFieldProviderProps = ReturnType<typeof useFormControlUtil<FieldId>>['pr
};

type FormFieldContextValue = Omit<FormFieldProviderProps, 'id'> & {
errorMessageId?: string;
feedbackMessageId?: string;
id?: string;
fieldId?: FieldId;
hasError: boolean;
Expand Down Expand Up @@ -49,17 +49,22 @@ export const FormFieldContextProvider = (props: React.PropsWithChildren<FormFiel
// Both html attributes (e.g. data-invalid) and css styles depend on hasError being debounced
const hasError = debounced.feedbackType === 'error';

// Track whether the `FormErrorText` has been rendered.
// Track whether any feedback message has been rendered.
// We use this to append its id the `aria-describedby` of the `input`.
const errorMessageId = hasError ? `error-${propsId}` : '';
// Use legacy pattern for errors (backwards compatible), new pattern for other types
const feedbackMessageId = debounced.feedback
? debounced.feedbackType === 'error'
? `error-${propsId}`
: `${propsId}-${debounced.feedbackType}-feedback`
: '';
const value = React.useMemo(
() => ({
isRequired,
isDisabled,
hasError,
id,
fieldId: propsId,
errorMessageId,
feedbackMessageId,
setError,
setSuccess,
setWarning,
Expand All @@ -75,7 +80,7 @@ export const FormFieldContextProvider = (props: React.PropsWithChildren<FormFiel
hasError,
id,
propsId,
errorMessageId,
feedbackMessageId,
isDisabled,
setError,
setSuccess,
Expand Down Expand Up @@ -128,7 +133,7 @@ export const sanitizeInputProps = (
setSuccess,
setError,
setInfo,
errorMessageId,
feedbackMessageId,
fieldId,
label,
clearFeedback,
Expand All @@ -143,7 +148,7 @@ export const sanitizeInputProps = (
/**
* Ignore error for the index type as we have defined it explicitly above
*/
// @ts-ignore
// @ts-ignore - Dynamic property access for form field props
inputProps[key] = obj[key];
});

Expand Down
Loading