-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix: v8 Combobox role and accname for non-hidden icon button #26905
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
a65babe to
1398df9
Compare
📊 Bundle size report🤖 This report was generated against fbee0f3858ee67d53ee2540531a0cfb6497165cb |
Asset size changes
Baseline commit: 29ed7fb57a05a6595835f947193460886e80cfc5 (build) |
|
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 054ea62:
|
🕵 fluentuiv8 No visual regressions between this PR and main |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 1187 | 1197 | 5000 | |
| Breadcrumb | mount | 2813 | 2826 | 1000 | |
| Checkbox | mount | 2675 | 2668 | 5000 | |
| CheckboxBase | mount | 2421 | 2369 | 5000 | |
| ChoiceGroup | mount | 4307 | 4300 | 5000 | |
| ComboBox | mount | 1155 | 1156 | 1000 | |
| CommandBar | mount | 9345 | 9301 | 1000 | |
| ContextualMenu | mount | 12364 | 12456 | 1000 | |
| DefaultButton | mount | 1369 | 1367 | 5000 | |
| DetailsRow | mount | 3238 | 3242 | 5000 | |
| DetailsRowFast | mount | 3237 | 3246 | 5000 | |
| DetailsRowNoStyles | mount | 3118 | 3117 | 5000 | |
| Dialog | mount | 2980 | 2972 | 1000 | |
| DocumentCardTitle | mount | 581 | 598 | 1000 | |
| Dropdown | mount | 3150 | 3175 | 5000 | |
| FocusTrapZone | mount | 1921 | 1980 | 5000 | |
| FocusZone | mount | 1925 | 1950 | 5000 | |
| GroupedList | mount | 47325 | 52815 | 2 | |
| GroupedList | virtual-rerender | 22210 | 22392 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 81122 | 82130 | 2 | |
| GroupedListV2 | mount | 554 | 576 | 2 | |
| GroupedListV2 | virtual-rerender | 542 | 557 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 552 | 564 | 2 | |
| IconButton | mount | 1895 | 1799 | 5000 | |
| Label | mount | 761 | 749 | 5000 | |
| Layer | mount | 4178 | 4192 | 5000 | |
| Link | mount | 862 | 842 | 5000 | |
| MenuButton | mount | 1605 | 1613 | 5000 | |
| MessageBar | mount | 2352 | 2321 | 5000 | |
| Nav | mount | 3070 | 3082 | 1000 | |
| OverflowSet | mount | 1416 | 1436 | 5000 | |
| Panel | mount | 2492 | 2498 | 1000 | |
| Persona | mount | 1254 | 1264 | 1000 | |
| Pivot | mount | 1537 | 1524 | 1000 | |
| PrimaryButton | mount | 1483 | 1471 | 5000 | |
| Rating | mount | 6981 | 6943 | 5000 | |
| SearchBox | mount | 1488 | 1501 | 5000 | |
| Shimmer | mount | 2889 | 2900 | 5000 | |
| Slider | mount | 2137 | 2110 | 5000 | |
| SpinButton | mount | 4230 | 4351 | 5000 | |
| Spinner | mount | 827 | 824 | 5000 | |
| SplitButton | mount | 2794 | 2843 | 5000 | |
| Stack | mount | 864 | 853 | 5000 | |
| StackWithIntrinsicChildren | mount | 2220 | 2231 | 5000 | |
| StackWithTextChildren | mount | 4929 | 4943 | 5000 | |
| SwatchColorPicker | mount | 9432 | 9507 | 5000 | |
| TagPicker | mount | 2344 | 2343 | 5000 | |
| TeachingBubble | mount | 75488 | 75018 | 5000 | |
| Text | mount | 826 | 827 | 5000 | |
| TextField | mount | 1539 | 1563 | 5000 | |
| ThemeProvider | mount | 1446 | 1443 | 5000 | |
| ThemeProvider | virtual-rerender | 1152 | 1160 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 2015 | 1997 | 5000 | |
| Toggle | mount | 1132 | 1121 | 5000 | |
| buttonNative | mount | 532 | 524 | 5000 |
micahgodbolt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other than Sean's suggestion, this looks good
0db5c69 to
054ea62
Compare
* master: (93 commits) chore: migrate to jest 27 (microsoft#26835) chore: make lint task run without need of build (microsoft#26872) chore(react-table): exports UseTableSelectionOptions (microsoft#26892) applying package updates fix(react-card): allow elements to grow to fill the available space (microsoft#26616) fix: Popover without focus trap should not be aria-hidden (microsoft#26932) applying package updates applying package updates fix(react-combobox): Remove _getAriaActiveDescendantValue, compute aria-activedescendantvalue in state, and update currentPendingValue when the options change (microsoft#26574) fix: v8 Combobox role and accname for non-hidden icon button (microsoft#26905) fix: Removing possible recursive loop in Coachmark (microsoft#26934) Combobox: Fix cursor jumping to the end of input (microsoft#26931) Fix missing icons on website (microsoft#26797) fix: Fix the width of Input's focus border with appearance=underline (microsoft#26881) chore: Clean up Input's interactive styles (microsoft#26865) Remove codeowners from change files (microsoft#26935) chore: add splitbutton error warning to docs, remove button ariaDescription example (microsoft#26904) docs: Remove testing code from MenuList example (microsoft#26929) chore: refactor SpinButton to use makeResetStyles (microsoft#26867) feat: Set overflow on positioned element when `autosize` is applied (microsoft#26868) ...
…r-component * feat/drawer-base-component: (141 commits) remove DrawerContainer feat: WIP add initial draft for Drawer and DrawerContainer chore: migrate to jest 27 (microsoft#26835) chore: make lint task run without need of build (microsoft#26872) chore(react-table): exports UseTableSelectionOptions (microsoft#26892) applying package updates fix(react-card): allow elements to grow to fill the available space (microsoft#26616) fix: Popover without focus trap should not be aria-hidden (microsoft#26932) applying package updates applying package updates fix(react-combobox): Remove _getAriaActiveDescendantValue, compute aria-activedescendantvalue in state, and update currentPendingValue when the options change (microsoft#26574) fix: v8 Combobox role and accname for non-hidden icon button (microsoft#26905) fix: Removing possible recursive loop in Coachmark (microsoft#26934) Combobox: Fix cursor jumping to the end of input (microsoft#26931) Fix missing icons on website (microsoft#26797) fix: Fix the width of Input's focus border with appearance=underline (microsoft#26881) chore: Clean up Input's interactive styles (microsoft#26865) Remove codeowners from change files (microsoft#26935) chore: add splitbutton error warning to docs, remove button ariaDescription example (microsoft#26904) docs: Remove testing code from MenuList example (microsoft#26929) ...
When the Combobox icon button isn't hidden, it should have an appropriate role and accname