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
MenuButton: Removing legacy patterns from converged component #18498
Conversation
Asset size changes
Over Tolerance (1024 B) Over Baseline Below Baseline New Removed 1 kB = 1000 B Baseline commit: 3a08bcacbd059fbc1d528607639576c4512433c7 (build) |
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
Avatar | mount | 888 | 885 | 5000 | |
BaseButton | mount | 1009 | 1008 | 5000 | |
Breadcrumb | mount | 2793 | 2857 | 1000 | |
ButtonNext | mount | 597 | 567 | 5000 | |
Checkbox | mount | 1710 | 1712 | 5000 | |
CheckboxBase | mount | 1446 | 1465 | 5000 | |
ChoiceGroup | mount | 5326 | 5279 | 5000 | |
ComboBox | mount | 1141 | 1051 | 1000 | |
CommandBar | mount | 10995 | 11151 | 1000 | |
ContextualMenu | mount | 6553 | 6691 | 1000 | |
DefaultButton | mount | 1283 | 1286 | 5000 | |
DetailsRow | mount | 4098 | 4140 | 5000 | |
DetailsRowFast | mount | 4216 | 4095 | 5000 | |
DetailsRowNoStyles | mount | 3875 | 3912 | 5000 | |
Dialog | mount | 2313 | 2286 | 1000 | |
DocumentCardTitle | mount | 154 | 161 | 1000 | |
Dropdown | mount | 3581 | 3644 | 5000 | |
FocusTrapZone | mount | 1943 | 2006 | 5000 | |
FocusZone | mount | 1963 | 1928 | 5000 | |
IconButton | mount | 1956 | 2001 | 5000 | |
Label | mount | 360 | 358 | 5000 | |
Layer | mount | 2000 | 2007 | 5000 | |
Link | mount | 513 | 502 | 5000 | |
MakeStyles | mount | 1946 | 1958 | 50000 | |
MenuButton | mount | 1679 | 1657 | 5000 | |
MessageBar | mount | 2180 | 2162 | 5000 | |
Nav | mount | 3635 | 3609 | 1000 | |
OverflowSet | mount | 1146 | 1134 | 5000 | |
Panel | mount | 2208 | 2242 | 1000 | |
Persona | mount | 882 | 945 | 1000 | |
Pivot | mount | 1584 | 1553 | 1000 | |
PrimaryButton | mount | 1427 | 1445 | 5000 | |
Rating | mount | 8686 | 8714 | 5000 | |
SearchBox | mount | 1458 | 1487 | 5000 | |
Shimmer | mount | 2856 | 2874 | 5000 | |
Slider | mount | 2199 | 2265 | 5000 | |
SpinButton | mount | 5410 | 5571 | 5000 | |
Spinner | mount | 457 | 460 | 5000 | |
SplitButton | mount | 3583 | 3458 | 5000 | |
Stack | mount | 544 | 572 | 5000 | |
StackWithIntrinsicChildren | mount | 1726 | 1772 | 5000 | |
StackWithTextChildren | mount | 5223 | 5186 | 5000 | |
SwatchColorPicker | mount | 11413 | 11153 | 5000 | |
Tabs | mount | 1547 | 1517 | 1000 | |
TagPicker | mount | 2661 | 2733 | 5000 | |
TeachingBubble | mount | 12706 | 12774 | 5000 | |
Text | mount | 460 | 463 | 5000 | |
TextField | mount | 1497 | 1509 | 5000 | |
ThemeProvider | mount | 1298 | 1332 | 5000 | |
ThemeProvider | virtual-rerender | 625 | 663 | 5000 | |
ThemeProviderNext | mount | 7034 | 7216 | 5000 | |
Toggle | mount | 876 | 892 | 5000 | |
buttonNative | mount | 114 | 117 | 5000 |
Perf Analysis (@fluentui/react-northstar
)
Perf tests with no regressions
Scenario | Current PR Ticks | Baseline Ticks | Ratio |
---|---|---|---|
LayoutMinimalPerf.default | 431 | 391 | 1.1:1 |
AvatarMinimalPerf.default | 239 | 220 | 1.09:1 |
TreeWith60ListItems.default | 196 | 180 | 1.09:1 |
AccordionMinimalPerf.default | 183 | 171 | 1.07:1 |
FormMinimalPerf.default | 471 | 442 | 1.07:1 |
ListWith60ListItems.default | 739 | 692 | 1.07:1 |
AnimationMinimalPerf.default | 478 | 449 | 1.06:1 |
AttachmentSlotsPerf.default | 1319 | 1244 | 1.06:1 |
FlexMinimalPerf.default | 332 | 312 | 1.06:1 |
SegmentMinimalPerf.default | 407 | 384 | 1.06:1 |
DatepickerMinimalPerf.default | 6155 | 5844 | 1.05:1 |
IconMinimalPerf.default | 700 | 669 | 1.05:1 |
TextMinimalPerf.default | 405 | 384 | 1.05:1 |
CarouselMinimalPerf.default | 530 | 510 | 1.04:1 |
HeaderSlotsPerf.default | 878 | 843 | 1.04:1 |
LabelMinimalPerf.default | 438 | 420 | 1.04:1 |
PortalMinimalPerf.default | 195 | 188 | 1.04:1 |
ProviderMinimalPerf.default | 1130 | 1085 | 1.04:1 |
TableMinimalPerf.default | 471 | 451 | 1.04:1 |
ChatDuplicateMessagesPerf.default | 336 | 327 | 1.03:1 |
ChatMinimalPerf.default | 694 | 673 | 1.03:1 |
VideoMinimalPerf.default | 715 | 692 | 1.03:1 |
AttachmentMinimalPerf.default | 183 | 180 | 1.02:1 |
CardMinimalPerf.default | 649 | 636 | 1.02:1 |
DialogMinimalPerf.default | 840 | 827 | 1.02:1 |
DropdownManyItemsPerf.default | 805 | 793 | 1.02:1 |
GridMinimalPerf.default | 388 | 381 | 1.02:1 |
ImageMinimalPerf.default | 424 | 414 | 1.02:1 |
ListMinimalPerf.default | 585 | 575 | 1.02:1 |
ListNestedPerf.default | 632 | 622 | 1.02:1 |
RosterPerf.default | 1344 | 1315 | 1.02:1 |
PopupMinimalPerf.default | 630 | 618 | 1.02:1 |
RefMinimalPerf.default | 253 | 249 | 1.02:1 |
TooltipMinimalPerf.default | 1106 | 1082 | 1.02:1 |
ButtonSlotsPerf.default | 618 | 611 | 1.01:1 |
CheckboxMinimalPerf.default | 3055 | 3018 | 1.01:1 |
DropdownMinimalPerf.default | 3356 | 3318 | 1.01:1 |
EmbedMinimalPerf.default | 4567 | 4500 | 1.01:1 |
RadioGroupMinimalPerf.default | 518 | 512 | 1.01:1 |
SplitButtonMinimalPerf.default | 4230 | 4206 | 1.01:1 |
TableManyItemsPerf.default | 2204 | 2188 | 1.01:1 |
TextAreaMinimalPerf.default | 584 | 581 | 1.01:1 |
ToolbarMinimalPerf.default | 1059 | 1050 | 1.01:1 |
DividerMinimalPerf.default | 418 | 420 | 1:1 |
HeaderMinimalPerf.default | 429 | 428 | 1:1 |
ListCommonPerf.default | 710 | 711 | 1:1 |
LoaderMinimalPerf.default | 759 | 756 | 1:1 |
MenuMinimalPerf.default | 977 | 973 | 1:1 |
MenuButtonMinimalPerf.default | 1768 | 1770 | 1:1 |
ProviderMergeThemesPerf.default | 1779 | 1776 | 1:1 |
SkeletonMinimalPerf.default | 409 | 407 | 1:1 |
SliderMinimalPerf.default | 1702 | 1697 | 1:1 |
CustomToolbarPrototype.default | 4117 | 4108 | 1:1 |
AlertMinimalPerf.default | 317 | 320 | 0.99:1 |
BoxMinimalPerf.default | 392 | 394 | 0.99:1 |
ItemLayoutMinimalPerf.default | 1437 | 1451 | 0.99:1 |
ReactionMinimalPerf.default | 429 | 432 | 0.99:1 |
ButtonOverridesMissPerf.default | 1905 | 1953 | 0.98:1 |
ChatWithPopoverPerf.default | 405 | 417 | 0.97:1 |
TreeMinimalPerf.default | 881 | 909 | 0.97:1 |
InputMinimalPerf.default | 1359 | 1414 | 0.96:1 |
StatusMinimalPerf.default | 767 | 795 | 0.96:1 |
}, | ||
}); | ||
|
||
export const useMenuButtonStyles = (state: MenuButtonState, selectors: MenuButtonStyleSelectors) => { | ||
export const useMenuButtonStyles = (state: MenuButtonState): MenuButtonState => { | ||
useButtonStyles(state); |
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.
Similar to CompoundButton, should useButtonStyles
be called at the end of this function so that the styles defined here will override the button styles? (Probably not an issue now, but in the future someone might try to add more styles and get confused why it's not working as expected).
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.
Yeah, will make the changes.
🎉 Handy links: |
Pull request checklist
$ yarn change
Description of changes
Follow-up of #18468 and #18497.
This PR removes the legacy patterns that were being used in the
MenuButton
component in@fluentui/react-button
, instead opting for using the patterns adopted by other converged components such asAvatar
in@fluentui/react-avatar
.A PR to update
ToggleButton
will follow.