-
-
Notifications
You must be signed in to change notification settings - Fork 969
fix(Checkbox): set indeterminate property for screen reader support #4448
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
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4448 +/- ##
=======================================
Coverage 94.48% 94.49%
=======================================
Files 559 560 +1
Lines 13797 13803 +6
Branches 4106 4107 +1
=======================================
+ Hits 13036 13043 +7
+ Misses 689 688 -1
Partials 72 72
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 a screen reader accessibility issue by ensuring the native checkbox indeterminate property is set via JavaScript. Screen readers like NVDA, JAWS, and VoiceOver require the native DOM property to properly announce the "mixed" or "partially checked" state, not just the ARIA attribute.
- Creates a reusable
useIndeterminateCheckboxhook that manages the indeterminate state viauseLayoutEffect - Refactors
Checkboxcomponent to use the new hook instead of directly managing refs - Adds comprehensive test coverage for both
CheckboxandCheckTreecomponents
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Checkbox/hooks/useIndeterminateCheckbox.ts |
New hook that synchronously sets the native checkbox indeterminate property using useLayoutEffect |
src/Checkbox/Checkbox.tsx |
Replaces direct useRef with the new hook, removes unused import |
src/Checkbox/test/Checkbox.spec.tsx |
Adds test to verify the native indeterminate property is set correctly |
src/CheckTree/test/CheckTree.spec.tsx |
Adds comprehensive test suite for indeterminate state in parent-child checkbox relationships |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Deployment failed with the following error: |
|
Deployment failed with the following error: |
- Create useIndeterminateCheckbox hook to manage indeterminate state - Set native checkbox indeterminate property via useLayoutEffect - Add test to verify indeterminate property is set correctly - Add comprehensive CheckTree tests for indeterminate state Fixes #4345 Screen readers (NVDA, JAWS, VoiceOver) require the native checkbox indeterminate property to be set via JavaScript, not just aria-checked. This ensures proper announcement of 'mixed' or 'partially checked' state.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
bf9ff0b to
cfe2495
Compare
Fixes #4345
Screen readers (NVDA, JAWS, VoiceOver) require the native checkbox indeterminate property to be set via JavaScript, not just aria-checked. This ensures proper announcement of 'mixed' or 'partially checked' state.