va-checkbox: add indeterminate prop #1426
Conversation
| private updateIndeterminateInput() { | ||
| const input = this.el.shadowRoot.querySelector('input'); | ||
| if (this.indeterminate && !this.checked) { | ||
| input.indeterminate = true; |
There was a problem hiding this comment.
We are adding this indeterminate attribute manually with JavaScript based on Ryan's a11y findings here. When I tested with VO, I confirmed that this is what announces "mixed" as the checkbox state when it's active.
| @@ -230,14 +256,15 @@ export class VaCheckbox { | |||
| aria-describedby={ariaDescribedbyIds} | |||
| aria-invalid={error ? 'true' : 'false'} | |||
| disabled={disabled} | |||
| data-indeterminate={indeterminate && !checked} | |||
There was a problem hiding this comment.
The USWDS release notes say:
when you set
input.indeterminate = truevia JavaScript or add thedata-indeterminateattribute
...but I couldn't get everything working for both the styles and screenreader without adding both indeterminate using JS and also adding this data-indeterminate attribute. Maybe this is a shadow dom limitation?
| onChange={this.handleChange} | ||
| /> | ||
| <label | ||
| htmlFor="checkbox-element" | ||
| class="usa-checkbox__label" | ||
| part="label" | ||
| role="checkbox" | ||
| aria-checked={ariaChecked} | ||
| aria-checked={indeterminate && !checked ? 'mixed' : ariaChecked} |
There was a problem hiding this comment.
I'm adding this because otherwise the aria-checked state would be either true or false but it also accepts "mixed" according to MDN.
Setting indeterminate on the input element though seems to be what is making the screenreader announcement. I'm not sure exactly what aria-checked is doing in this scenario if anything.
| @@ -257,4 +257,39 @@ describe('va-checkbox', () => { | |||
| await checkboxLabelEl.click(); | |||
| expect(await checkboxEl.getProperty('checked')).toBeTruthy(); | |||
| }); | |||
|
|
|||
| it('sets the data-indeterminate attribute on the input element', async () => { | |||
There was a problem hiding this comment.
I tried also to get a test working that checked if the input element was getting the .indeterminate property from being set in JS but I could not get it to work. :/
This is kind of a strange property though so either it's some kind of testing limitation with Stencil or I'm just not writing the test in a way that it can pick up that value.
|
For the Storybook example, I took inspiration from this resource but there are different ways to compose a scenario that uses this checkbox state. There are also different ways to style it (like indented options). Are design guidelines something we want to put more thought into? Or is this example going to be ok for now?
cc @department-of-veterans-affairs/platform-design-system-designers |
There was a problem hiding this comment.
Everything looked good in my testing. I tested in all of our screen reader combos that I have access to (everything except iOS) and it worked as expected.
Something to consider in the example is updating it to better show a parent-child relationship by changing the label to "Select All Historical Figures". I think that would better indicate that it controls the child checkboxes.
Thanks!
jamigibbs
changed the title
Chromatic
https://2652-va-checkbox-indeterminate--65a6e2ed2314f7b8f98609d8.chromatic.com/?path=/docs/uswds-va-checkbox--docs#indeterminate
Description
This will add an
indeterminateprop tova-checkbox. When active, the checkbox will display the "mixed" icon in the checkbox which indicates that it's neither checked nor unchecked.Closes department-of-veterans-affairs/vets-design-system-documentation#2652
QA Checklist
Screenshots
Acceptance criteria
Definition of done