fix(Indeterminate Checkbox): Fixed issue with voice over not recognizing the mixed state when using keyboard navigation. #1652
Conversation
chris-cedrone-cengage
requested review from
silvalaura and
a team
as code owners
🦋 Changeset detectedLatest commit: bace366 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 |
chris-cedrone-cengage
requested review from
nikitaorliak-cengage and
silvalaura
and removed request for
a team and
silvalaura
|
it does read mixed, I was able to check the disabled checkbox in the storybook using my keyboard
|
| ) : isChecked ? ( | ||
| <CheckBoxIcon size={theme.iconSizes.medium} /> | ||
| <div | ||
| tabIndex={!disabled && 0} |
There was a problem hiding this comment.
I'm not sure about this tabindex here. It's changing the keyboard navigation behavior in TreeView which we don't want. Speaking of which, we should note in the testing notes to consider TreeView and DataGrid
There was a problem hiding this comment.
I removed the div and moved these attributes to StyledFakeInput.
| <div | ||
| tabIndex={!disabled && 0} | ||
| role="checkbox" | ||
| aria-checked="mixed" |
There was a problem hiding this comment.
When we add this, we now have an input with aria-checked mixed AND a div with it. Is this okay? How do other systems handle this?
There was a problem hiding this comment.
Also, this mixed status is hardcoded so even when it becomes checked, mixed is still in the dom.
There was a problem hiding this comment.
-
Having tabIndex here still breaks the keyboard navigation in TreeView. With TreeView, we expect
Tab key moves to the tree and then out of the treeand the tree should be navigated with up and down arrows. -
The focus around the indeterminate checkbox looks weird/different to the focus around regular checkboxes
-
Have you found a good example in the wild that does what we are trying to do? That will help us ensure we are doing this the right way.
| @@ -172,6 +185,10 @@ export const IndeterminateCheckbox = React.forwardRef< | |||
| style={inputStyle} | |||
| theme={theme} | |||
| aria-hidden="true" | |||
| tabIndex={0} | |||
| role="checkbox" | |||
| aria-checked={isIndeterminate ? 'mixed' : isChecked} | |||
There was a problem hiding this comment.
Is it okay to have aria-checked twice? If it is, we can copy line 153 aria-checked={ariaCheckedValue}.
We have type="checkbox" on the hidden input so I worry this is repetitive??
Closes #1231
🐢 What I did 🐢
Restructured DOM to recognize the "mixed" state when Voice Over tabs over indeterminate checkboxes
📺 Screenshots: 📺
✅ Checklist ✅
🐎 How to test 🐎