Skip to content

fix(Calendar): remove explicit readonly semantics from grid and gridcells#20324

Merged
lorejoh12 merged 2 commits intomicrosoft:masterfrom
smhigley:calendar-readonly
Oct 28, 2021
Merged

fix(Calendar): remove explicit readonly semantics from grid and gridcells#20324
lorejoh12 merged 2 commits intomicrosoft:masterfrom
smhigley:calendar-readonly

Conversation

@smhigley
Copy link
Contributor

@smhigley smhigley commented Oct 23, 2021

Pull request checklist

  • Addresses an existing issue: Fixes 12950
  • Include a change request file using $ yarn change

Description 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 marked readonly. The issue is that Talkback is now treating any use of readonly as disabled, 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-readonly attributes seems like the best choice now.

@smhigley smhigley requested a review from a team October 23, 2021 02:18
@smhigley smhigley requested a review from lorejoh12 as a code owner October 23, 2021 02:18
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 23, 2021

📊 Bundle size report

🤖 This report was generated against 25bf940e8e5512b0168999a7ae4c6994e4976129

@size-auditor
Copy link

size-auditor bot commented Oct 23, 2021

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-WeeklyDayPicker 94.502 kB 94.46 kB BelowBaseline     -42 bytes
office-ui-fabric-react fluentui-react-Calendar 113.129 kB 113.049 kB BelowBaseline     -80 bytes
office-ui-fabric-react fluentui-react-DatePicker 169.642 kB 169.562 kB BelowBaseline     -80 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 25bf940e8e5512b0168999a7ae4c6994e4976129 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 23, 2021

Perf Analysis (@fluentui/react)

No significant results to display.

All results

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
lorejoh12 approved these changes Oct 28, 2021
View reviewed changes
Copy link
Contributor

@lorejoh12 lorejoh12 left a 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.

@lorejoh12 lorejoh12 merged commit 6fa3723 into microsoft:master Oct 28, 2021
mlp73 pushed a commit to mlp73/fluentui that referenced this pull request Jan 17, 2022
@smhigley
fix(Calendar): remove explicit readonly semantics from grid and gridc…
6a2cf0c 
…ells (microsoft#20324)

* remove explicit readonly semantics from grid

* Change files
@smhigley smhigley mentioned this pull request Jan 21, 2022
Merged
3 tasks
@smhigley smhigley deleted the calendar-readonly branch July 19, 2022 18:38
@smhigley smhigley mentioned this pull request Jul 19, 2022
Merged
This was referenced Aug 30, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@micahgodbolt micahgodbolt micahgodbolt approved these changes

@khmakoto khmakoto khmakoto approved these changes

@GeoffCoxMSFT GeoffCoxMSFT GeoffCoxMSFT approved these changes

+1 more reviewer

@lorejoh12 lorejoh12 lorejoh12 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@bsunderhus bsunderhus

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@smhigley @fabricteam @micahgodbolt @khmakoto @lorejoh12 @GeoffCoxMSFT @bsunderhus