-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix(Calendar): remove explicit readonly semantics from grid and gridcells #20324
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
📊 Bundle size report🤖 This report was generated against 25bf940e8e5512b0168999a7ae4c6994e4976129 |
Asset size changes
Baseline commit: 25bf940e8e5512b0168999a7ae4c6994e4976129 (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1031 | 1010 | 5000 | |
| BaseButton | mount | 980 | 1017 | 5000 | |
| Breadcrumb | mount | 2639 | 2595 | 1000 | |
| ButtonNext | mount | 515 | 524 | 5000 | |
| Checkbox | mount | 1644 | 1670 | 5000 | |
| CheckboxBase | mount | 1403 | 1454 | 5000 | |
| ChoiceGroup | mount | 5030 | 4968 | 5000 | |
| ComboBox | mount | 1007 | 1074 | 1000 | |
| CommandBar | mount | 10258 | 10322 | 1000 | |
| ContextualMenu | mount | 6322 | 6572 | 1000 | |
| DefaultButton | mount | 1199 | 1212 | 5000 | |
| DetailsRow | mount | 3942 | 3933 | 5000 | |
| DetailsRowFast | mount | 3977 | 3897 | 5000 | |
| DetailsRowNoStyles | mount | 3781 | 3753 | 5000 | |
| Dialog | mount | 2605 | 2522 | 1000 | |
| DocumentCardTitle | mount | 185 | 185 | 1000 | |
| Dropdown | mount | 3388 | 3443 | 5000 | |
| FluentProviderNext | mount | 3212 | 3311 | 5000 | |
| FluentProviderWithTheme | mount | 213 | 211 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 114 | 102 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 238 | 238 | 10 | |
| FocusTrapZone | mount | 1921 | 1908 | 5000 | |
| FocusZone | mount | 1872 | 1790 | 5000 | |
| IconButton | mount | 1839 | 1889 | 5000 | |
| Label | mount | 371 | 355 | 5000 | |
| Layer | mount | 3184 | 3117 | 5000 | |
| Link | mount | 513 | 522 | 5000 | |
| MakeStyles | mount | 1756 | 1825 | 50000 | |
| MenuButton | mount | 1544 | 1582 | 5000 | |
| MessageBar | mount | 2024 | 1985 | 5000 | |
| Nav | mount | 3466 | 3418 | 1000 | |
| OverflowSet | mount | 1117 | 1157 | 5000 | |
| Panel | mount | 2437 | 2467 | 1000 | |
| Persona | mount | 888 | 885 | 1000 | |
| Pivot | mount | 1553 | 1487 | 1000 | |
| PrimaryButton | mount | 1354 | 1340 | 5000 | |
| Rating | mount | 8348 | 8244 | 5000 | |
| SearchBox | mount | 1515 | 1427 | 5000 | |
| Shimmer | mount | 2692 | 2679 | 5000 | |
| Slider | mount | 2092 | 2064 | 5000 | |
| SpinButton | mount | 5211 | 5205 | 5000 | |
| Spinner | mount | 438 | 453 | 5000 | |
| SplitButton | mount | 3331 | 3348 | 5000 | |
| Stack | mount | 540 | 524 | 5000 | |
| StackWithIntrinsicChildren | mount | 1813 | 1814 | 5000 | |
| StackWithTextChildren | mount | 5160 | 5086 | 5000 | |
| SwatchColorPicker | mount | 11135 | 11102 | 5000 | |
| Tabs | mount | 1518 | 1553 | 1000 | |
| TagPicker | mount | 2773 | 2814 | 5000 | |
| TeachingBubble | mount | 13040 | 13313 | 5000 | |
| Text | mount | 456 | 452 | 5000 | |
| TextField | mount | 1445 | 1479 | 5000 | |
| ThemeProvider | mount | 1198 | 1235 | 5000 | |
| ThemeProvider | virtual-rerender | 628 | 618 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1984 | 1991 | 5000 | |
| Toggle | mount | 877 | 852 | 5000 | |
| buttonNative | mount | 131 | 133 | 5000 |
Perf Analysis (@fluentui/react-northstar)
Perf tests with no regressions
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| AccordionMinimalPerf.default | 199 | 176 | 1.13:1 |
| ListNestedPerf.default | 639 | 583 | 1.1:1 |
| ListMinimalPerf.default | 584 | 535 | 1.09:1 |
| TableMinimalPerf.default | 467 | 432 | 1.08:1 |
| BoxMinimalPerf.default | 389 | 363 | 1.07:1 |
| PortalMinimalPerf.default | 184 | 172 | 1.07:1 |
| VideoMinimalPerf.default | 692 | 646 | 1.07:1 |
| AvatarMinimalPerf.default | 216 | 203 | 1.06:1 |
| IconMinimalPerf.default | 683 | 645 | 1.06:1 |
| TextMinimalPerf.default | 381 | 358 | 1.06:1 |
| ListCommonPerf.default | 718 | 682 | 1.05:1 |
| DropdownManyItemsPerf.default | 755 | 728 | 1.04:1 |
| GridMinimalPerf.default | 376 | 363 | 1.04:1 |
| InputMinimalPerf.default | 1399 | 1363 | 1.03:1 |
| ProviderMinimalPerf.default | 1216 | 1178 | 1.03:1 |
| RadioGroupMinimalPerf.default | 492 | 476 | 1.03:1 |
| RefMinimalPerf.default | 241 | 233 | 1.03:1 |
| TreeMinimalPerf.default | 858 | 830 | 1.03:1 |
| AlertMinimalPerf.default | 324 | 318 | 1.02:1 |
| AttachmentSlotsPerf.default | 1161 | 1138 | 1.02:1 |
| CarouselMinimalPerf.default | 494 | 482 | 1.02:1 |
| DividerMinimalPerf.default | 402 | 393 | 1.02:1 |
| HeaderMinimalPerf.default | 392 | 384 | 1.02:1 |
| LayoutMinimalPerf.default | 406 | 399 | 1.02:1 |
| MenuMinimalPerf.default | 902 | 888 | 1.02:1 |
| PopupMinimalPerf.default | 608 | 598 | 1.02:1 |
| StatusMinimalPerf.default | 739 | 723 | 1.02:1 |
| AnimationMinimalPerf.default | 469 | 463 | 1.01:1 |
| DropdownMinimalPerf.default | 3322 | 3279 | 1.01:1 |
| ItemLayoutMinimalPerf.default | 1311 | 1297 | 1.01:1 |
| SplitButtonMinimalPerf.default | 4602 | 4566 | 1.01:1 |
| TextAreaMinimalPerf.default | 557 | 554 | 1.01:1 |
| TooltipMinimalPerf.default | 1109 | 1102 | 1.01:1 |
| TreeWith60ListItems.default | 197 | 195 | 1.01:1 |
| ButtonMinimalPerf.default | 207 | 207 | 1:1 |
| ButtonOverridesMissPerf.default | 1870 | 1875 | 1:1 |
| ChatMinimalPerf.default | 717 | 717 | 1:1 |
| CheckboxMinimalPerf.default | 2868 | 2870 | 1:1 |
| FormMinimalPerf.default | 459 | 459 | 1:1 |
| ListWith60ListItems.default | 683 | 685 | 1:1 |
| CustomToolbarPrototype.default | 4320 | 4305 | 1:1 |
| DialogMinimalPerf.default | 809 | 815 | 0.99:1 |
| FlexMinimalPerf.default | 300 | 304 | 0.99:1 |
| LabelMinimalPerf.default | 416 | 419 | 0.99:1 |
| ProviderMergeThemesPerf.default | 1717 | 1741 | 0.99:1 |
| SegmentMinimalPerf.default | 376 | 380 | 0.99:1 |
| SliderMinimalPerf.default | 1796 | 1809 | 0.99:1 |
| TableManyItemsPerf.default | 2046 | 2058 | 0.99:1 |
| ToolbarMinimalPerf.default | 975 | 985 | 0.99:1 |
| ButtonSlotsPerf.default | 568 | 577 | 0.98:1 |
| ChatWithPopoverPerf.default | 417 | 424 | 0.98:1 |
| EmbedMinimalPerf.default | 4520 | 4592 | 0.98:1 |
| LoaderMinimalPerf.default | 713 | 725 | 0.98:1 |
| MenuButtonMinimalPerf.default | 1738 | 1766 | 0.98:1 |
| ReactionMinimalPerf.default | 421 | 428 | 0.98:1 |
| CardMinimalPerf.default | 612 | 629 | 0.97:1 |
| DatepickerMinimalPerf.default | 5732 | 5911 | 0.97:1 |
| HeaderSlotsPerf.default | 818 | 855 | 0.96:1 |
| RosterPerf.default | 1258 | 1322 | 0.95:1 |
| ChatDuplicateMessagesPerf.default | 331 | 352 | 0.94:1 |
| ImageMinimalPerf.default | 417 | 443 | 0.94:1 |
| AttachmentMinimalPerf.default | 180 | 196 | 0.92:1 |
| SkeletonMinimalPerf.default | 389 | 426 | 0.91:1 |
lorejoh12
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.
Thanks for following up on this! That is indeed the reason we originally had to add this readonly tag, so it's great to hear that it's been fixed and we can have a little more straightforward semantics here.
…ells (microsoft#20324) * remove explicit readonly semantics from grid * Change files
Pull request checklist
$ yarn changeDescription of changes
This change comes from a past spec ambiguity on whether
role="grid"was editable by default. NVDA used to read "editable" for any gridcell unless it was explicitly markedreadonly. The issue is that Talkback is now treating any use ofreadonlyasdisabled, whether on form controls or grid.Before making this change, I tested with NVDA 2021 on Chrome and Edge 94, and FF 93, and it appears NVDA is no longer announcing "editable" by default, so removing the
aria-readonlyattributes seems like the best choice now.