-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(accessibility): Field announce error message for focused field #8329
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
base: main
Are you sure you want to change the base?
Conversation
db1dec4
to
71e40dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the error message announcement behavior for focused fields by updating the logic in the Field component to announce errors via the live announcer. Key changes include:
- Adding a useEffect in Field.tsx to announce error messages when the field is focused.
- Renaming the help text flag to hasErrorMessage for clarity.
- Adding the @react-aria/live-announcer dependency in package.json.
- Extending InlineAlert stories with a new DynamicWithAriaLivePolite variant.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/@react-spectrum/label/src/Field.tsx | Updates error message announcement logic and variable naming |
packages/@react-spectrum/label/package.json | Adds dependency for live announcer |
packages/@react-spectrum/inlinealert/stories/InlineAlert.stories.tsx | Adds a new story variant showcasing aria-live behavior |
71e40dc
to
3484b2f
Compare
fee6570
to
af43a05
Compare
Build successful! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves accessibility by ensuring that error messages are announced when the associated field is focused.
- Introduces an effect in Field.tsx to announce error messages when the field is active.
- Adds a dependency on @react-aria/live-announcer in package.json.
- Updates stories and tests to better handle error message rendering.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/@react-spectrum/label/src/Field.tsx | Implements useEffect to announce error messages for focused fields. |
packages/@react-spectrum/label/package.json | Adds the live announcer dependency to support new accessibility logic. |
packages/@react-spectrum/inlinealert/stories/InlineAlert.stories.tsx | Introduces a new story variant with aria-live explicitly set. |
packages/@react-spectrum/datepicker/test/DateField.test.js | Modifies test to access the first instance of the error message element. |
Comments suppressed due to low confidence (2)
packages/@react-spectrum/datepicker/test/DateField.test.js:239
- Verify that multiple occurrences of the 'Date unavailable.' text are expected. If only one error message should appear, consider updating this to use a more specific selector to avoid masking potential duplication issues.
expect(tree.getAllByText('Date unavailable.')[0]).toBeInTheDocument();
packages/@react-spectrum/label/src/Field.tsx:71
- Ensure that the ref passed to Field is always a RefObject. The current cast to RefObject assumes a stable object ref; if callback refs are used, this might lead to unexpected behavior when announcing error messages.
React.useEffect(() => {
Build successful! 🎉 |
fd00b91
to
fbd7356
Compare
Build successful! 🎉 |
Build successful! 🎉 |
3573692
to
bd37367
Compare
Build successful! 🎉 |
Build successful! 🎉 |
Co-authored-by: Copilot <[email protected]>
f862c17
to
72b183c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances error message announcements to improve accessibility by ensuring that error messages for focused fields are announced appropriately. Key changes include:
- Removing an extraneous newline in Field.tsx.
- Adding dependencies on @react-aria/live-announcer in both label and form packages to enable live announcements.
- Updating tests and stories to reflect improved error announcement logic and ARIA live behaviors.
- Refactoring useFormValidation in the form package to delay and trigger error message announcements on blur.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
packages/@react-spectrum/label/src/Field.tsx | Removed an unnecessary blank line. |
packages/@react-spectrum/label/package.json | Added @react-aria/live-announcer dependency. |
packages/@react-spectrum/inlinealert/stories/InlineAlert.stories.tsx | Introduced a new story variant with aria-live="polite". |
packages/@react-spectrum/datepicker/test/DateField.test.js | Updated error message test to use getAllByText instead of getByText. |
packages/@react-aria/form/src/useFormValidation.ts | Implemented delayed error announcement logic and adjusted event listeners. |
packages/@react-aria/form/package.json | Added @react-aria/live-announcer dependency. |
justBlurredRef.current | ||
) | ||
) { | ||
timeoutIdRef.current = setTimeout(() => announce(errorMessage, 'polite'), 250); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider extracting the hardcoded 250ms delay into a named constant to improve clarity and ease future adjustments.
Copilot uses AI. Check for mistakes.
input!.removeEventListener('invalid', onInvalid); | ||
input!.removeEventListener('change', onChange); | ||
form?.removeEventListener('reset', onReset); | ||
}; | ||
}, [ref, onInvalid, onChange, onReset, validationBehavior]); | ||
}, [justBlurredRef, onBlur, onChange, onInvalid, onReset, ref, validationBehavior]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Since ref objects are stable, including justBlurredRef and ref in the dependency array may be unnecessary; consider removing them to simplify the dependencies.
}, [justBlurredRef, onBlur, onChange, onInvalid, onReset, ref, validationBehavior]); | |
}, [onBlur, onChange, onInvalid, onReset, validationBehavior]); |
Copilot uses AI. Check for mistakes.
Build successful! 🎉 |
Closes #8328
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project:
Accessibility/SUSI/Adobe