-
Notifications
You must be signed in to change notification settings - Fork 303
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 indeterminate checkbox edge cases #2458
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 07467ef 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 |
packages/ui/components/checkbox-group/src/LionCheckboxIndeterminate.js
Outdated
Show resolved
Hide resolved
packages/ui/components/checkbox-group/src/LionCheckboxIndeterminate.js
Outdated
Show resolved
Hide resolved
packages/ui/components/checkbox-group/src/LionCheckboxIndeterminate.js
Outdated
Show resolved
Hide resolved
fixes: #1967 |
Could you add a test with a use case that covers |
When there is one of children checkboxes disabled checked and the other child checkbox is unchecked, the parent should be unchecked. Now it is checked |
What I did
There were a few problems in the LionCheckboxIndeterminate:
_setOwnCheckedState()
was called byform-element-register
event before the same event was handled by the checkbox group which adds the element emitting the event to theformElements
false
although, in practice, it was fully checkedindeterminate
property wastrue
and the same property of the input node was alsotrue
, and clicking it should keep it indeterminated (like when one of the children is disabled and the others are mixed with checked and unchecked), theindeterminate
property of the custom element doesn't change but the same property of the input node changes tofalse
(seems like browser's default behavior) because no syncing could happen without the custome element'sindeteminate
property changing.This PR fixes those issues.