GrafanaUI: Add indeterminate state to Checkbox #67312
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JoaoSilvaGrafana
added
add to changelog
area/grafana/ui
JoaoSilvaGrafana
requested review from
jackw and
eledobleefe
and removed request for
a team
joshhunt
approved these changes
Apr 26, 2023
| - [ ] B-series | ||
| - [ ] C-series | ||
|
|
||
| The indeterminate state of the checkbox should be used when there is a group of child checkboxes that are in a mix of checked and unchecked states. For instance when you have a list of checkboxes representing the columns of a table, and you want to allow the user to select which columns to display. If some of the columns are checked, and some are unchecked, then the parent checkbox should be in an indeterminate state. If all the columns are checked, then the parent should be checked. If none of the columns are checked, then the parent should be unchecked. |
There was a problem hiding this comment.
I would mention here that <Checkbox checked indeterminate /> is discouraged as a checkboxs emit boolean values which can only ever be TRUE or FALSE, and if something is partially true, then it is false.
|
Hi! I run this branch in my local and I found the
|
eledobleefe
reviewed
Apr 27, 2023
| right: 3px; | ||
| top: calc(50% - 1.5px); | ||
| height: 3px; | ||
| border: solid ${theme.colors.primary.contrastText}; |
There was a problem hiding this comment.
@JoaoSilvaGrafana If I get rid of this border the indeterminate state looks good, otherwise, it looks like the screenshots I added to my comments.
There was a problem hiding this comment.
Firefox! :shakes-fist:
Thanks for checking Laura 😄 I will see how to solve this, that line is there solving another edge case problem...
eledobleefe
approved these changes
Apr 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this feature?
Adds an indeterminate state to checkboxes. It picks up from this PR #65463 and adds some more styling to it, including disabled.
Why do we need this feature?
Indeterminate is a visual only state typically used to show that 'sub options' are in a mixed state. Setting
indeterminate={true}on a checkbox does not affect the value of the checkbox, but only the visual appearance.Who is this feature for?
Everyone
Which issue(s) does this PR fix?:
Fixes #63335
Special notes for your reviewer:
Regarding a question on if we need to set indeterminate property to the checkbox via JS, I don't believe we need it, at least from a a11y perspective all that is required is that we set
aria-checked='mixed'.Please check that: