Skip to content
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

react-button 1st rule of ARIA refactor #18582

Closed

Conversation

bsunderhus
Copy link
Contributor

Pull request checklist

  • Include a change request file using $ yarn change

Description of changes

Refactors react-button to use react-aria to ensure 1st rule of ARIA

@bsunderhus bsunderhus requested a review from khmakoto June 16, 2021 11:55
@bsunderhus bsunderhus self-assigned this Jun 16, 2021
@bsunderhus bsunderhus added this to In progress in Teams Prague - @microsoft/teams-prg via automation Jun 16, 2021
@bsunderhus bsunderhus requested a review from a team as a code owner June 16, 2021 11:55
@bsunderhus bsunderhus requested a review from a team June 16, 2021 11:55
@size-auditor
Copy link

size-auditor bot commented Jun 16, 2021

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-components-ToggleButton 35.42 kB 36.081 kB ExceedsBaseline     661 bytes
office-ui-fabric-react fluentui-react-components-Button 25.194 kB 25.854 kB ExceedsBaseline     660 bytes
office-ui-fabric-react fluentui-react-components-CompoundButton 29.946 kB 30.606 kB ExceedsBaseline     660 bytes
office-ui-fabric-react fluentui-react-components-MenuButton 26.78 kB 27.44 kB ExceedsBaseline     660 bytes

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

Baseline commit: f945172f2b3904cf993a6a7f65a13bbe44f31fbe (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Jun 16, 2021

Perf Analysis (@fluentui/react)

Scenario Render type Master Ticks PR Ticks Iterations Status
ButtonNext mount 520 522 5000 Possible regression
Panel mount 2057 1349 1000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 808 788 5000
BaseButton mount 875 885 5000
Breadcrumb mount 2651 2629 1000
ButtonNext mount 520 522 5000 Possible regression
Checkbox mount 1536 1507 5000
CheckboxBase mount 1277 1262 5000
ChoiceGroup mount 4691 4626 5000
ComboBox mount 951 943 1000
CommandBar mount 10092 10067 1000
ContextualMenu mount 6223 6214 1000
DefaultButton mount 1124 1126 5000
DetailsRow mount 3697 3583 5000
DetailsRowFast mount 3647 3657 5000
DetailsRowNoStyles mount 3466 3542 5000
Dialog mount 2164 2139 1000
DocumentCardTitle mount 136 137 1000
Dropdown mount 3178 3193 5000
FluentProviderNext mount 7159 7225 5000
FocusTrapZone mount 1770 1766 5000
FocusZone mount 1791 1794 5000
IconButton mount 1707 1686 5000
Label mount 331 334 5000
Layer mount 1748 1760 5000
Link mount 459 470 5000
MakeStyles mount 1793 1797 50000
MenuButton mount 1426 1411 5000
MessageBar mount 2012 2022 5000
Nav mount 3177 3251 1000
OverflowSet mount 1024 1051 5000
Panel mount 2057 1349 1000 Possible regression
Persona mount 816 808 1000
Pivot mount 1403 1355 1000
PrimaryButton mount 1236 1268 5000
Rating mount 7307 7432 5000
SearchBox mount 1294 1295 5000
Shimmer mount 2496 2510 5000
Slider mount 1913 1919 5000
SpinButton mount 4832 4766 5000
Spinner mount 406 418 5000
SplitButton mount 3133 3061 5000
Stack mount 481 498 5000
StackWithIntrinsicChildren mount 1449 1460 5000
StackWithTextChildren mount 4393 4356 5000
SwatchColorPicker mount 9806 9911 5000
Tabs mount 1361 1367 1000
TagPicker mount 2320 2312 5000
TeachingBubble mount 11664 11642 5000
Text mount 398 419 5000
TextField mount 1359 1377 5000
ThemeProvider mount 1162 1133 5000
ThemeProvider virtual-rerender 592 580 5000
Toggle mount 796 784 5000
buttonNative mount 114 104 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 179 138 1.3:1
AccordionMinimalPerf.default 152 140 1.09:1
ButtonMinimalPerf.default 174 160 1.09:1
PortalMinimalPerf.default 179 167 1.07:1
ListNestedPerf.default 558 527 1.06:1
SegmentMinimalPerf.default 346 331 1.05:1
ButtonSlotsPerf.default 554 531 1.04:1
ChatDuplicateMessagesPerf.default 284 274 1.04:1
ChatMinimalPerf.default 641 618 1.04:1
ListCommonPerf.default 626 602 1.04:1
AvatarMinimalPerf.default 197 191 1.03:1
DividerMinimalPerf.default 360 349 1.03:1
LabelMinimalPerf.default 381 369 1.03:1
PopupMinimalPerf.default 566 552 1.03:1
ProviderMinimalPerf.default 957 928 1.03:1
RefMinimalPerf.default 240 232 1.03:1
TableMinimalPerf.default 385 375 1.03:1
VideoMinimalPerf.default 608 592 1.03:1
ListWith60ListItems.default 641 630 1.02:1
LoaderMinimalPerf.default 691 678 1.02:1
RadioGroupMinimalPerf.default 430 423 1.02:1
TextMinimalPerf.default 343 337 1.02:1
ToolbarMinimalPerf.default 918 901 1.02:1
TreeMinimalPerf.default 790 772 1.02:1
TreeWith60ListItems.default 174 170 1.02:1
AlertMinimalPerf.default 270 268 1.01:1
CardMinimalPerf.default 530 526 1.01:1
CarouselMinimalPerf.default 453 448 1.01:1
HeaderMinimalPerf.default 353 351 1.01:1
ItemLayoutMinimalPerf.default 1201 1184 1.01:1
ListMinimalPerf.default 496 490 1.01:1
TableManyItemsPerf.default 1864 1848 1.01:1
TextAreaMinimalPerf.default 491 488 1.01:1
CustomToolbarPrototype.default 3708 3669 1.01:1
AnimationMinimalPerf.default 406 408 1:1
AttachmentSlotsPerf.default 1035 1036 1:1
BoxMinimalPerf.default 345 345 1:1
CheckboxMinimalPerf.default 2671 2665 1:1
DatepickerMinimalPerf.default 5274 5277 1:1
DialogMinimalPerf.default 744 741 1:1
DropdownMinimalPerf.default 3069 3054 1:1
EmbedMinimalPerf.default 4058 4046 1:1
InputMinimalPerf.default 1236 1238 1:1
MenuMinimalPerf.default 825 825 1:1
MenuButtonMinimalPerf.default 1527 1523 1:1
SliderMinimalPerf.default 1497 1501 1:1
SplitButtonMinimalPerf.default 3636 3623 1:1
StatusMinimalPerf.default 668 665 1:1
TooltipMinimalPerf.default 952 955 1:1
FlexMinimalPerf.default 268 272 0.99:1
FormMinimalPerf.default 395 398 0.99:1
GridMinimalPerf.default 328 332 0.99:1
LayoutMinimalPerf.default 355 360 0.99:1
RosterPerf.default 1165 1173 0.99:1
ProviderMergeThemesPerf.default 1604 1619 0.99:1
ReactionMinimalPerf.default 368 371 0.99:1
ButtonOverridesMissPerf.default 1655 1687 0.98:1
ImageMinimalPerf.default 363 370 0.98:1
SkeletonMinimalPerf.default 335 342 0.98:1
DropdownManyItemsPerf.default 654 677 0.97:1
IconMinimalPerf.default 581 596 0.97:1
HeaderSlotsPerf.default 729 769 0.95:1
ChatWithPopoverPerf.default 333 361 0.92:1

ev.currentTarget.click();
}
});
// TODO: this handles Spacebar on keyup, but conformance test is breaking
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a follow-up issue opened to fix conformance tests regarding this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. there should be a discussion about updating conformance after implementing this #18587

but for the moment, I just converged react-aria to support things as they are at the moment, so nothing breaks.

@khmakoto
Copy link
Member

I'm ok with the PR as it is now, I just want to merge #18563 first.

@bsunderhus bsunderhus added the Status: Blocked Resolution blocked by another issue label Jun 30, 2021
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 30, 2021

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 f7aa616:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@bsunderhus
Copy link
Contributor Author

I'm ok with the PR as it is now, I just want to merge #18563 first.

No problem @khmakoto ! I'll block this for now, and probably wait to merge #18721 first also

@bsunderhus
Copy link
Contributor Author

This PR implementation will be covered by #18814

@bsunderhus bsunderhus closed this Sep 21, 2021
Teams Prague - @microsoft/teams-prg automation moved this from In progress to Done Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Button Status: Blocked Resolution blocked by another issue
Development

Successfully merging this pull request may close these issues.

None yet

4 participants