-
Notifications
You must be signed in to change notification settings - Fork 507
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
fix(Checkbox): mirror aria-checked
to checked property of checkbox input
#4263
Conversation
🦋 Changeset detectedLatest commit: 7aff51a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
bump @pksjce if you have a sec to review! |
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.
Verify that the axe issue from the original issue is no longer present
Yeah, all gone!
Do you have any ideas for better handling this change? 👀 Unfortunately, defaultChecked was the root of the issue here and I couldn't think of a better way to keep things in sync since this component can be both controlled and uncontrolled
I think this is great. The way I thought is very similar to yours, I think yours is much cleaner and certainly avoids nested ternaries 😄
checked={indeterminate ? false : defaultChecked || checked}
aria-checked={indeterminate ? 'mixed' : defaultChecked ? defaultChecked : checked ? 'true' : 'false'}
Thanks for fixing this 🚀
…ditional-attr-violation
…input (#4263) * fix(Checkbox): mirror `aria-checked` to checked property of checkbox input * chore: add changeset * fix: add back in useEffect import --------- Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Closes #4262
Changelog
New
Changed
aria-checked
attribute to thechecked
property on the checkbox inputRemoved
Rollout strategy
Testing & Reviewing
defaultChecked
was the root of the issue here and I couldn't think of a better way to keep things in sync since this component can be both controlled and uncontrolled