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.
This PR improves the Combobox because there are some differences compared to the Listbox that we didn't properly account for.
Big thanks to @afercia for providing us with a wonderful issue report that talks about the differences.
Edit (24/01/2023): Going to merge this PR with partial fixes but not everything. This PR will fix the following items:
aria-autocomplete
attributesaria-activedescendant
on theCombobox.Options
(it is only required on theCombobox.Input
)For the
aria-selected
prop it is a bit confusing to apply it based on thevisual
state only which means that there is no difference between a checked active value and a non-checked active value. I also created a table below with odd findings...We will tackle this in a different PR. But want to merge these changes/fixes already.
This commit fixes a few issues:
aria-selected
value was always based on theselected
value (which is correct for theListbox
) but it should be based on the visually selected value (in our case, this is theactive
value).aria-activedescendant
attribute from theComboboxOptions
, it should only exist on theComboboxInput
.aria-autocomplete
to theComboboxInput
, with a value oflist
so that assistive technology better understands what kind of Combobox we are dealing with.ComboboxInput
is made. Now we will trigger a change to the input when we open theCombobox
, we will set the value to''
and re-set to whatever value was present. We also make sure that the cursor position / selected range is reset so that the end user doesn't feel any differences (except for a better working VoiceOver on macOS).Some additional findings:
I also think that this is impossible to get right, so we have to chose the "best" way we think. Should we start announcing these changes in state manually instead of relying on on VoiceOver? 🤔
Some fun differences between macOS Safari + VoiceOver and macOS Chrome + VoiceOver:
So while using the
active
state foraria-selected
gives you at least consistent announcements, it also means that there is no way to differentiate betweenselected
andactive
.Single Value Mode
aria-selected
based on the visual selected value.aria-selected: active ? true : undefined
aria-selected: active
aria-selected
based on the actual selected value.aria-selected: selected ? true : undefined
aria-selected: selected
aria-checked
based on the visual selected value.aria-checked: active ? true : undefined
aria-checked: active
aria-checked
based on the actual selected value.aria-checked: selected ? true : undefined
aria-selected: selected
Multi Value Mode
aria-selected
based on the visual selected value.aria-selected: active ? true : undefined
aria-selected: active
aria-selected
based on the actual selected value.aria-selected: selected ? true : undefined
aria-selected: selected
aria-checked
based on the visual selected value.aria-checked: active ? true : undefined
aria-checked: active
aria-checked
based on the actual selected value.aria-checked: selected ? true : undefined
aria-selected: selected
Fixes: #2129
Co-authored-by: Andrea Fercia a.fercia@gmail.com