Skip to content

Commit 80e45d7

Browse files
fix(clerk-js): Ensure input feedback messages are mapped to the input (#6914)
1 parent 4531215 commit 80e45d7

File tree

10 files changed

+130
-32
lines changed

10 files changed

+130
-32
lines changed

.changeset/empty-laws-boil.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/clerk-js': patch
3+
---
4+
5+
Ensure inputs are properly connected to feedback messages via `aria-describedby` usage.

packages/clerk-js/src/ui/elements/FieldControl.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,11 @@ const FieldLabelRow = (props: PropsWithChildren) => {
154154
};
155155

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

159159
return (
160160
<FormFeedback
161161
center={props.center}
162-
errorMessageId={errorMessageId}
163162
{...{
164163
...debouncedFeedback,
165164
elementDescriptors: props.elementDescriptors,

packages/clerk-js/src/ui/elements/FormControl.tsx

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ function useFormTextAnimation() {
4040
transitionTimingFunction: t.transitionTiming.$common,
4141
});
4242
},
43-
[prefersReducedMotion],
43+
[prefersReducedMotion, appearanceAnimations],
4444
);
4545

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

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

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

@@ -182,6 +179,7 @@ export const FormFeedback = (props: FormFeedbackProps) => {
182179
getFormTextAnimation(!!feedbacks.a?.shouldEnter, { inDelay: true }),
183180
]}
184181
localizationKey={titleize(feedbacks.a?.feedback)}
182+
aria-live={feedbacks.a?.shouldEnter ? 'polite' : 'off'}
185183
/>
186184
<InfoComponentB
187185
{...getElementProps(feedbacks.b?.feedbackType)}
@@ -193,6 +191,7 @@ export const FormFeedback = (props: FormFeedbackProps) => {
193191
getFormTextAnimation(!!feedbacks.b?.shouldEnter, { inDelay: true }),
194192
]}
195193
localizationKey={titleize(feedbacks.b?.feedback)}
194+
aria-live={feedbacks.b?.shouldEnter ? 'polite' : 'off'}
196195
/>
197196
</Flex>
198197
);

packages/clerk-js/src/ui/elements/__tests__/PlainInput.test.tsx

Lines changed: 100 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ const createField = (...params: Parameters<typeof useFormControl>) => {
2121
{...props}
2222
/>
2323
<button onClick={() => field.setError('some error')}>set error</button>
24+
<button onClick={() => field.setSuccess('some success')}>set success</button>
25+
<button onClick={() => field.setWarning('some warning')}>set warning</button>
26+
<button onClick={() => field.setInfo('some info')}>set info</button>
2427
</>
2528
);
2629
});
@@ -132,15 +135,20 @@ describe('PlainInput', () => {
132135
placeholder: 'some placeholder',
133136
});
134137

135-
const { getByRole, getByLabelText, findByText } = render(<Field />, { wrapper });
138+
const { getByRole, getByLabelText, findByText, container } = render(<Field />, { wrapper });
136139

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

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

141-
const label = getByLabelText(/some label/i);
142-
expect(label).toHaveAttribute('aria-invalid', 'true');
143-
expect(label).toHaveAttribute('aria-describedby', 'error-firstname');
144+
const input = getByLabelText(/some label/i);
145+
expect(input).toHaveAttribute('aria-invalid', 'true');
146+
expect(input).toHaveAttribute('aria-describedby', 'error-firstname');
147+
148+
// Verify the error message element has the correct ID
149+
const errorElement = container.querySelector('#error-firstname');
150+
expect(errorElement).toBeInTheDocument();
151+
expect(errorElement).toHaveTextContent(/Some Error/i);
144152
});
145153

146154
it('with info', async () => {
@@ -157,4 +165,92 @@ describe('PlainInput', () => {
157165
fireEvent.focus(await findByLabelText(/some label/i));
158166
expect(await findByText(/some info/i)).toBeInTheDocument();
159167
});
168+
169+
it('with success feedback and aria-describedby', async () => {
170+
const { wrapper } = await createFixtures();
171+
const { Field } = createField('firstname', 'init value', {
172+
type: 'text',
173+
label: 'some label',
174+
placeholder: 'some placeholder',
175+
});
176+
177+
const { getByRole, getByLabelText, findByText, container } = render(<Field />, { wrapper });
178+
179+
await userEvent.click(getByRole('button', { name: /set success/i }));
180+
181+
expect(await findByText(/Some Success/i)).toBeInTheDocument();
182+
183+
const input = getByLabelText(/some label/i);
184+
expect(input).toHaveAttribute('aria-invalid', 'false');
185+
expect(input).toHaveAttribute('aria-describedby', 'firstname-success-feedback');
186+
187+
// Verify the success message element has the correct ID
188+
const successElement = container.querySelector('#firstname-success-feedback');
189+
expect(successElement).toBeInTheDocument();
190+
expect(successElement).toHaveTextContent(/Some Success/i);
191+
});
192+
193+
it('transitions between error and success feedback types', async () => {
194+
const { wrapper } = await createFixtures();
195+
const { Field } = createField('firstname', 'init value', {
196+
type: 'text',
197+
label: 'some label',
198+
placeholder: 'some placeholder',
199+
});
200+
201+
const { getByRole, getByLabelText, findByText, container } = render(<Field />, { wrapper });
202+
203+
// Start with error
204+
await userEvent.click(getByRole('button', { name: /set error/i }));
205+
expect(await findByText(/Some Error/i)).toBeInTheDocument();
206+
207+
let input = getByLabelText(/some label/i);
208+
expect(input).toHaveAttribute('aria-invalid', 'true');
209+
expect(input).toHaveAttribute('aria-describedby', 'error-firstname');
210+
211+
// Transition to success
212+
await userEvent.click(getByRole('button', { name: /set success/i }));
213+
expect(await findByText(/Some Success/i)).toBeInTheDocument();
214+
215+
input = getByLabelText(/some label/i);
216+
expect(input).toHaveAttribute('aria-invalid', 'false');
217+
expect(input).toHaveAttribute('aria-describedby', 'firstname-success-feedback');
218+
219+
// Verify success element exists with proper ID
220+
const successElement = container.querySelector('#firstname-success-feedback');
221+
expect(successElement).toBeInTheDocument();
222+
expect(successElement).toHaveTextContent(/Some Success/i);
223+
});
224+
225+
it('aria-live attribute is correctly applied', async () => {
226+
const { wrapper } = await createFixtures();
227+
const { Field } = createField('firstname', 'init value', {
228+
type: 'text',
229+
label: 'some label',
230+
placeholder: 'some placeholder',
231+
});
232+
233+
const { getByRole, findByText, container } = render(<Field />, { wrapper });
234+
235+
// Set error feedback
236+
await userEvent.click(getByRole('button', { name: /set error/i }));
237+
expect(await findByText(/Some Error/i)).toBeInTheDocument();
238+
239+
// Verify the visible error message has aria-live="polite"
240+
const errorElement = container.querySelector('#error-firstname');
241+
expect(errorElement).toHaveAttribute('aria-live', 'polite');
242+
243+
// Transition to success
244+
await userEvent.click(getByRole('button', { name: /set success/i }));
245+
expect(await findByText(/Some Success/i)).toBeInTheDocument();
246+
247+
// Verify the visible success message has aria-live="polite"
248+
const successElement = container.querySelector('#firstname-success-feedback');
249+
expect(successElement).toHaveAttribute('aria-live', 'polite');
250+
251+
// The previous error message should now have aria-live="off" (though it might still exist in DOM but hidden)
252+
// Verify exactly one element has aria-live="polite" at a time
253+
const allAriaLivePolite = container.querySelectorAll('[aria-live="polite"]');
254+
expect(allAriaLivePolite.length).toBe(1);
255+
});
160256
});

packages/clerk-js/src/ui/primitives/FormErrorText.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const { applyVariants } = createVariants(theme => ({
2323
type FormErrorTextProps = React.PropsWithChildren<StyleVariants<typeof applyVariants>>;
2424

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

2828
if (!hasError && !props.children) {
2929
return null;
@@ -35,8 +35,6 @@ export const FormErrorText = forwardRef<HTMLElement, FormErrorTextProps>((props,
3535
<Text
3636
ref={ref}
3737
colorScheme='danger'
38-
aria-live='polite'
39-
id={errorMessageId}
4038
{...rest}
4139
css={applyVariants(props)}
4240
>

packages/clerk-js/src/ui/primitives/FormInfoText.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { useFormField } from './hooks/useFormField';
66
import { Text } from './Text';
77

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

1111
if (!hasError && !props.children) {
1212
return null;
@@ -16,8 +16,6 @@ export const FormInfoText = forwardRef<HTMLElement, FormTextProps>((props, ref)
1616
<Text
1717
ref={ref}
1818
colorScheme='secondary'
19-
aria-live='polite'
20-
id={errorMessageId}
2119
{...props}
2220
css={applyVariants(props)}
2321
/>

packages/clerk-js/src/ui/primitives/FormSuccessText.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ export const FormSuccessText = forwardRef<HTMLElement, FormTextProps>((props, re
2828
<Text
2929
ref={ref}
3030
colorScheme='secondary'
31-
aria-live='polite'
3231
{...rest}
3332
css={applyVariants(props) as any}
3433
>

packages/clerk-js/src/ui/primitives/FormWarningText.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ export const FormWarningText = forwardRef<HTMLElement, FormTextProps>((props, re
1313
<Text
1414
ref={ref}
1515
colorScheme='secondary'
16-
aria-live='polite'
1716
{...rest}
1817
css={applyVariants(props) as any}
1918
>

packages/clerk-js/src/ui/primitives/Input.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,10 @@ export type InputProps = PrimitiveProps<'input'> & StyleVariants<typeof applyVar
6363

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

7272
const propsWithoutVariants = filterProps({
@@ -106,7 +106,7 @@ export const Input = React.forwardRef<HTMLInputElement, InputProps>((props, ref)
106106
required={_required}
107107
id={props.id || fieldControlProps.id}
108108
aria-invalid={_hasError}
109-
aria-describedby={errorMessageId ? errorMessageId : undefined}
109+
aria-describedby={feedbackMessageId || undefined}
110110
aria-required={_required}
111111
aria-disabled={_disabled}
112112
data-feedback={feedbackType}

packages/clerk-js/src/ui/primitives/hooks/useFormField.tsx

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ type FormFieldProviderProps = ReturnType<typeof useFormControlUtil<FieldId>>['pr
1010
};
1111

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

52-
// Track whether the `FormErrorText` has been rendered.
52+
// Track whether any feedback message has been rendered.
5353
// We use this to append its id the `aria-describedby` of the `input`.
54-
const errorMessageId = hasError ? `error-${propsId}` : '';
54+
// Use legacy pattern for errors (backwards compatible), new pattern for other types
55+
const feedbackMessageId = debounced.feedback
56+
? debounced.feedbackType === 'error'
57+
? `error-${propsId}`
58+
: `${propsId}-${debounced.feedbackType}-feedback`
59+
: '';
5560
const value = React.useMemo(
5661
() => ({
5762
isRequired,
5863
isDisabled,
5964
hasError,
6065
id,
6166
fieldId: propsId,
62-
errorMessageId,
67+
feedbackMessageId,
6368
setError,
6469
setSuccess,
6570
setWarning,
@@ -75,7 +80,7 @@ export const FormFieldContextProvider = (props: React.PropsWithChildren<FormFiel
7580
hasError,
7681
id,
7782
propsId,
78-
errorMessageId,
83+
feedbackMessageId,
7984
isDisabled,
8085
setError,
8186
setSuccess,
@@ -128,7 +133,7 @@ export const sanitizeInputProps = (
128133
setSuccess,
129134
setError,
130135
setInfo,
131-
errorMessageId,
136+
feedbackMessageId,
132137
fieldId,
133138
label,
134139
clearFeedback,
@@ -143,7 +148,7 @@ export const sanitizeInputProps = (
143148
/**
144149
* Ignore error for the index type as we have defined it explicitly above
145150
*/
146-
// @ts-ignore
151+
// @ts-ignore - Dynamic property access for form field props
147152
inputProps[key] = obj[key];
148153
});
149154

0 commit comments

Comments
 (0)