Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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/puny-places-shine.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Add aria live region to ensure feedback messages are read to screen readers when feedback changes.
79 changes: 45 additions & 34 deletions packages/clerk-js/src/ui/elements/FormControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ import {
FormInfoText,
FormSuccessText,
FormWarningText,
Span,
useAppearance,
} from '../customizables';
import type { ElementDescriptor } from '../customizables/elementDescriptors';
import { usePrefersReducedMotion } from '../hooks';
import type { ThemableCssProp } from '../styledSystem';
import { animations } from '../styledSystem';
import { animations, common } from '../styledSystem';
import type { FeedbackType, useFormControlFeedback } from '../utils/useFormControl';

function useFormTextAnimation() {
Expand Down Expand Up @@ -161,38 +162,48 @@ export const FormFeedback = (props: FormFeedbackProps) => {
const InfoComponentB = FormInfoComponent[feedbacks.b?.feedbackType || 'info'];

return (
<Flex
style={{
height: feedback ? maxHeight : 0, // dynamic height
position: 'relative',
}}
center={center}
sx={[getFormTextAnimation(!!feedback), sx]}
>
<InfoComponentA
{...getElementProps(feedbacks.a?.feedbackType)}
ref={calculateHeightA}
sx={[
() => ({
visibility: feedbacks.a?.shouldEnter ? 'visible' : 'hidden',
}),
getFormTextAnimation(!!feedbacks.a?.shouldEnter, { inDelay: true }),
]}
localizationKey={titleize(feedbacks.a?.feedback)}
aria-live={feedbacks.a?.shouldEnter ? 'polite' : 'off'}
/>
<InfoComponentB
{...getElementProps(feedbacks.b?.feedbackType)}
ref={calculateHeightB}
sx={[
() => ({
visibility: feedbacks.b?.shouldEnter ? 'visible' : 'hidden',
}),
getFormTextAnimation(!!feedbacks.b?.shouldEnter, { inDelay: true }),
]}
localizationKey={titleize(feedbacks.b?.feedback)}
aria-live={feedbacks.b?.shouldEnter ? 'polite' : 'off'}
/>
</Flex>
<>
{/* Screen reader only live region that updates when feedback changes */}
<Span
aria-live='polite'
aria-atomic='true'
sx={{
...common.visuallyHidden(),
}}
>
{feedback ? titleize(feedback) : ''}
</Span>
<Flex
style={{
height: feedback ? maxHeight : 0, // dynamic height
position: 'relative',
}}
center={center}
sx={[getFormTextAnimation(!!feedback), sx]}
>
<InfoComponentA
{...getElementProps(feedbacks.a?.feedbackType)}
ref={calculateHeightA}
sx={[
() => ({
visibility: feedbacks.a?.shouldEnter ? 'visible' : 'hidden',
}),
getFormTextAnimation(!!feedbacks.a?.shouldEnter, { inDelay: true }),
]}
localizationKey={titleize(feedbacks.a?.feedback)}
/>
<InfoComponentB
{...getElementProps(feedbacks.b?.feedbackType)}
ref={calculateHeightB}
sx={[
() => ({
visibility: feedbacks.b?.shouldEnter ? 'visible' : 'hidden',
}),
getFormTextAnimation(!!feedbacks.b?.shouldEnter, { inDelay: true }),
]}
localizationKey={titleize(feedbacks.b?.feedback)}
/>
</Flex>
</>
);
};
33 changes: 23 additions & 10 deletions packages/clerk-js/src/ui/elements/__tests__/PlainInput.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ describe('PlainInput', () => {
expect(successElement).toHaveTextContent(/Some Success/i);
});

it('aria-live attribute is correctly applied', async () => {
it('screen reader only live region announces feedback changes', async () => {
const { wrapper } = await createFixtures();
const { Field } = createField('firstname', 'init value', {
type: 'text',
Expand All @@ -236,21 +236,34 @@ describe('PlainInput', () => {
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');
// Verify there's a screen-reader-only aria-live region with the error content
const ariaLiveRegions = container.querySelectorAll('[aria-live="polite"]');
expect(ariaLiveRegions.length).toBeGreaterThanOrEqual(1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: If I understood the bug correctly, the announcement did not work because aria-live only announces changes to the DOM element, so maybe this test should also assert that this element is already in the DOM without errors before the set error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ephem I updated the tests, let me know what you think!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! They feel tightly scoped now, and like they would catch any regression. 👍


// Find the screen reader only region (it will have the visually hidden styles)
const srOnlyRegion = Array.from(ariaLiveRegions).find(el => {
const style = window.getComputedStyle(el);
return style.position === 'absolute' && style.width === '1px';
});
expect(srOnlyRegion).toBeDefined();
expect(srOnlyRegion).toHaveTextContent(/Some Error/i);

// 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"
// Verify the screen reader only region updated its content
expect(srOnlyRegion).toHaveTextContent(/Some Success/i);

// Verify the visible error/success elements exist with proper IDs for aria-describedby
const errorElement = container.querySelector('#error-firstname');
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);
// One should be visible, the other hidden (for animation)
const errorVisible = errorElement && window.getComputedStyle(errorElement).visibility === 'visible';
const successVisible = successElement && window.getComputedStyle(successElement).visibility === 'visible';

// At least one should be visible (might be both during transition)
expect(errorVisible || successVisible).toBe(true);
});
});