-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix: v8 ComboBox needs to update aria-activedescendant value #27457
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
Conversation
Co-authored-by: Makoto Morimoto <[email protected]>
📊 Bundle size report🤖 This report was generated against 1071a2a382741b1328228b336c2a94de388a3963 |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit c19d115:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: af3c827937dcb0a32f52ef8a93b91c1276956b56 (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 818 | 827 | 5000 | |
| Breadcrumb | mount | 2180 | 2227 | 1000 | |
| Checkbox | mount | 2157 | 2109 | 5000 | |
| CheckboxBase | mount | 1959 | 1906 | 5000 | |
| ChoiceGroup | mount | 3757 | 3713 | 5000 | |
| ComboBox | mount | 884 | 900 | 1000 | |
| CommandBar | mount | 8035 | 7990 | 1000 | |
| ContextualMenu | mount | 17450 | 17292 | 1000 | |
| DefaultButton | mount | 967 | 984 | 5000 | |
| DetailsRow | mount | 2901 | 2905 | 5000 | |
| DetailsRowFast | mount | 2855 | 2882 | 5000 | |
| DetailsRowNoStyles | mount | 2648 | 2607 | 5000 | |
| Dialog | mount | 3440 | 3545 | 1000 | |
| DocumentCardTitle | mount | 329 | 325 | 1000 | |
| Dropdown | mount | 2575 | 2534 | 5000 | |
| FocusTrapZone | mount | 1484 | 1474 | 5000 | |
| FocusZone | mount | 1445 | 1426 | 5000 | |
| GroupedList | mount | 50209 | 58642 | 2 | |
| GroupedList | virtual-rerender | 24363 | 24311 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 75406 | 74630 | 2 | |
| GroupedListV2 | mount | 317 | 312 | 2 | |
| GroupedListV2 | virtual-rerender | 296 | 312 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 315 | 324 | 2 | |
| IconButton | mount | 1410 | 1399 | 5000 | |
| Label | mount | 444 | 456 | 5000 | |
| Layer | mount | 3570 | 3617 | 5000 | |
| Link | mount | 534 | 525 | 5000 | |
| MenuButton | mount | 1234 | 1217 | 5000 | |
| MessageBar | mount | 27294 | 27325 | 5000 | |
| Nav | mount | 2528 | 2561 | 1000 | |
| OverflowSet | mount | 1022 | 1019 | 5000 | |
| Panel | mount | 2264 | 2211 | 1000 | |
| Persona | mount | 968 | 980 | 1000 | |
| Pivot | mount | 1184 | 1175 | 1000 | |
| PrimaryButton | mount | 1103 | 1097 | 5000 | |
| Rating | mount | 5833 | 5748 | 5000 | |
| SearchBox | mount | 1133 | 1149 | 5000 | |
| Shimmer | mount | 2302 | 2347 | 5000 | |
| Slider | mount | 1713 | 1718 | 5000 | |
| SpinButton | mount | 3743 | 3799 | 5000 | |
| Spinner | mount | 528 | 525 | 5000 | |
| SplitButton | mount | 2430 | 2435 | 5000 | |
| Stack | mount | 536 | 530 | 5000 | |
| StackWithIntrinsicChildren | mount | 1129 | 1159 | 5000 | |
| StackWithTextChildren | mount | 3214 | 3205 | 5000 | |
| SwatchColorPicker | mount | 8132 | 8057 | 5000 | |
| TagPicker | mount | 2014 | 2006 | 5000 | |
| Text | mount | 498 | 503 | 5000 | |
| TextField | mount | 1193 | 1199 | 5000 | |
| ThemeProvider | mount | 1189 | 1134 | 5000 | |
| ThemeProvider | virtual-rerender | 772 | 761 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1749 | 1729 | 5000 | |
| Toggle | mount | 795 | 811 | 5000 | |
| buttonNative | mount | 289 | 284 | 5000 |
Previous Behavior
This is a regression introduced in #26574, and causes the
aria-activedescendantvalue to not update when arrowing through options, which makes the control fully broken for screen reader users.The root cause is that the
currentPendingValueValidIndexneeds to take precedence as the index of the active option, but the PR updated the logic to useselectedIndices[0]as the primary value, while it should be the fallback value.New Behavior
aria-activedescendantis now calculated off ofcurrentPendingValueValidIndex, falling back toselectedIndices[0]. This is what the behavior was prior to the PR.