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.

Sign up for GitHub

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

Jump to bottom

MenuButton: Re-introducing MenuButton using the latest version of makeStyles #18168

Merged
merged 33 commits into from
Jun 3, 2021
Merged

MenuButton: Re-introducing MenuButton using the latest version of makeStyles #18168

merged 33 commits into from
Jun 3, 2021

Conversation

khmakoto
Copy link
Member

Pull request checklist

Description of changes

This PR follows #17534, #17566, #17676 and #17736.

This PR re-introduces the MenuButton component with the latest makeStyles changes under @fluentui/react-button after having been removed for the initial release of @fluentui/react-components.

A PR re-introducing SplitButton will follow.

PRs reintroducing the block, circular and outline props will follow.

@khmakoto khmakoto requested a review from a team May 13, 2021 05:33
@khmakoto khmakoto requested a review from dzearing as a code owner May 13, 2021 05:33
@codesandbox-ci
Copy link

codesandbox-ci bot commented May 13, 2021 •
edited
Loading

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

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

@miroslavstastny
Copy link
Member

There is already a Menu component which implements this:
image

What is the difference between the two and why do we need both?
cc @ling1726

@khmakoto
Copy link
Member Author

@miroslavstastny the idea is to have a MenuButton that can potentially work with any menu you pass to it. I've modified the example to work with both v8's ContextualMenu and converged Menu in react-menu.

MenuButton

@size-auditor
Copy link

size-auditor bot commented May 13, 2021 •
edited
Loading

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-components-MenuButton New   35.111 kB ExceedsTolerance     35.111 kB
office-ui-fabric-react fluentui-react-components-Tooltip 45.241 kB 45.243 kB ExceedsBaseline     2 bytes
office-ui-fabric-react fluentui-react-components-CompoundButton 39.404 kB 39.398 kB BelowBaseline     -6 bytes
office-ui-fabric-react fluentui-react-components-ToggleButton 51.263 kB 51.236 kB BelowBaseline     -27 bytes
office-ui-fabric-react fluentui-react-components-Button 33.545 kB 33.516 kB BelowBaseline     -29 bytes

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

Baseline commit: 2c9b177f2c54ef93d8d66c1242573b257fdaf083 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented May 13, 2021 •
edited
Loading

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 821 814 5000
BaseButton mount 906 915 5000
Breadcrumb mount 2636 2668 1000
ButtonNext mount 494 499 5000
Checkbox mount 1542 1552 5000
CheckboxBase mount 1282 1310 5000
ChoiceGroup mount 4754 4730 5000
ComboBox mount 983 977 1000
CommandBar mount 10170 10195 1000
ContextualMenu mount 6150 6124 1000
DefaultButton mount 1125 1150 5000
DetailsRow mount 3708 3682 5000
DetailsRowFast mount 3701 3683 5000
DetailsRowNoStyles mount 3498 3416 5000
Dialog mount 2128 2092 1000
DocumentCardTitle mount 138 145 1000
Dropdown mount 3222 3231 5000
FocusTrapZone mount 1794 1792 5000
FocusZone mount 1899 1848 5000
IconButton mount 1768 1733 5000
Label mount 343 341 5000
Layer mount 1771 1780 5000
Link mount 461 475 5000
MakeStyles mount 1864 1848 50000
MenuButton mount 1461 1428 5000
MessageBar mount 2075 2041 5000
Nav mount 3227 3232 1000
OverflowSet mount 1033 1089 5000
Panel mount 2106 2114 1000
Persona mount 798 834 1000
Pivot mount 1413 1443 1000
PrimaryButton mount 1290 1306 5000
Rating mount 7697 7536 5000
SearchBox mount 1288 1300 5000
Shimmer mount 2544 2502 5000
Slider mount 1966 1976 5000
SpinButton mount 4969 4911 5000
Spinner mount 416 424 5000
SplitButton mount 3148 3154 5000
Stack mount 509 484 5000
StackWithIntrinsicChildren mount 1506 1488 5000
StackWithTextChildren mount 4508 4516 5000
SwatchColorPicker mount 10200 10261 5000
Tabs mount 1414 1398 1000
TagPicker mount 2378 2413 5000
TeachingBubble mount 11929 11879 5000
Text mount 414 418 5000
TextField mount 1375 1372 5000
ThemeProvider mount 1189 1210 5000
ThemeProvider virtual-rerender 590 625 5000
ThemeProviderNext mount 7174 7270 5000
Toggle mount 817 812 5000
buttonNative mount 117 127 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AccordionMinimalPerf.default 158 143 1.1:1
RefMinimalPerf.default 247 227 1.09:1
TreeWith60ListItems.default 174 160 1.09:1
ChatWithPopoverPerf.default 377 349 1.08:1
AlertMinimalPerf.default 287 271 1.06:1
AttachmentMinimalPerf.default 159 152 1.05:1
AttachmentSlotsPerf.default 1157 1101 1.05:1
PopupMinimalPerf.default 589 564 1.04:1
IconMinimalPerf.default 613 590 1.04:1
CardMinimalPerf.default 544 529 1.03:1
CarouselMinimalPerf.default 477 461 1.03:1
ChatMinimalPerf.default 621 604 1.03:1
DividerMinimalPerf.default 356 346 1.03:1
DropdownManyItemsPerf.default 689 667 1.03:1
ReactionMinimalPerf.default 380 370 1.03:1
SegmentMinimalPerf.default 355 343 1.03:1
AnimationMinimalPerf.default 420 411 1.02:1
HeaderMinimalPerf.default 365 358 1.02:1
LayoutMinimalPerf.default 368 360 1.02:1
PortalMinimalPerf.default 173 170 1.02:1
ProviderMergeThemesPerf.default 1664 1638 1.02:1
SliderMinimalPerf.default 1538 1512 1.02:1
TooltipMinimalPerf.default 986 964 1.02:1
VideoMinimalPerf.default 616 602 1.02:1
ButtonOverridesMissPerf.default 1682 1658 1.01:1
DatepickerMinimalPerf.default 5375 5311 1.01:1
EmbedMinimalPerf.default 4154 4107 1.01:1
HeaderSlotsPerf.default 753 749 1.01:1
InputMinimalPerf.default 1276 1263 1.01:1
LabelMinimalPerf.default 388 383 1.01:1
ListMinimalPerf.default 514 507 1.01:1
CustomToolbarPrototype.default 3790 3758 1.01:1
ToolbarMinimalPerf.default 936 923 1.01:1
TreeMinimalPerf.default 799 792 1.01:1
BoxMinimalPerf.default 350 350 1:1
CheckboxMinimalPerf.default 2773 2763 1:1
DropdownMinimalPerf.default 3098 3084 1:1
FormMinimalPerf.default 390 391 1:1
ItemLayoutMinimalPerf.default 1267 1268 1:1
ListWith60ListItems.default 651 651 1:1
LoaderMinimalPerf.default 701 699 1:1
MenuButtonMinimalPerf.default 1582 1579 1:1
ProviderMinimalPerf.default 957 961 1:1
ButtonMinimalPerf.default 165 167 0.99:1
DialogMinimalPerf.default 758 762 0.99:1
GridMinimalPerf.default 331 333 0.99:1
ImageMinimalPerf.default 372 374 0.99:1
ListCommonPerf.default 592 598 0.99:1
MenuMinimalPerf.default 827 837 0.99:1
StatusMinimalPerf.default 656 665 0.99:1
TableManyItemsPerf.default 1868 1889 0.99:1
TextAreaMinimalPerf.default 479 483 0.99:1
ChatDuplicateMessagesPerf.default 288 294 0.98:1
FlexMinimalPerf.default 282 287 0.98:1
ListNestedPerf.default 556 570 0.98:1
RosterPerf.default 1125 1146 0.98:1
RadioGroupMinimalPerf.default 441 450 0.98:1
SplitButtonMinimalPerf.default 3715 3773 0.98:1
TextMinimalPerf.default 344 350 0.98:1
ButtonSlotsPerf.default 535 553 0.97:1
SkeletonMinimalPerf.default 343 355 0.97:1
TableMinimalPerf.default 401 413 0.97:1
AvatarMinimalPerf.default 181 192 0.94:1

@khmakoto khmakoto mentioned this pull request May 17, 2021
Closed
32 tasks
@khmakoto khmakoto mentioned this pull request May 21, 2021
Merged
2 tasks
behowell
behowell approved these changes May 28, 2021
View reviewed changes
Copy link
Contributor

@behowell behowell left a comment

Choose a reason for hiding this comment

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

I made a few comments about the API patterns for Button being different from other components, but that can be figured out later and doesn't need to block this PR. The rest of the comments are relatively minor. Thanks!

packages/react-button/src/components/Button/useButtonState.ts Outdated Show resolved Hide resolved
packages/react-button/src/components/MenuButton/MenuButton.types.ts Outdated Show resolved Hide resolved
packages/react-button/src/components/MenuButton/MenuButton.types.ts Outdated Show resolved Hide resolved
packages/react-button/src/components/MenuButton/MenuButton.types.ts Show resolved Hide resolved
packages/react-button/src/components/MenuButton/renderMenuButton.tsx Outdated Show resolved Hide resolved
packages/react-button/src/components/MenuButton/useExpanded.ts Outdated Show resolved Hide resolved
packages/react-button/src/components/MenuButton/useExpanded.ts Outdated Show resolved Hide resolved
packages/react-button/src/components/MenuButton/useMenuButton.ts Outdated Show resolved Hide resolved
packages/react-button/src/components/MenuButton/useMenuButton.ts Outdated Show resolved Hide resolved
packages/react-button/src/components/MenuButton/MenuButton.types.ts Show resolved Hide resolved
@khmakoto khmakoto enabled auto-merge (squash) June 3, 2021 17:55
@khmakoto khmakoto disabled auto-merge June 3, 2021 18:35
@ecraig12345 ecraig12345 merged commit 1ca8c8f into microsoft:master Jun 3, 2021
@khmakoto khmakoto deleted the menuButton branch June 3, 2021 18:50
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-theme-provider@v9.0.0-alpha.41 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-tabster@v9.0.0-alpha.32 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-utilities@v9.0.0-alpha.27 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-provider@v9.0.0-alpha.44 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-positioning@v9.0.0-alpha.26 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-tooltip@v9.0.0-alpha.42 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-shared-contexts@v9.0.0-alpha.14 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-menu@v9.0.0-alpha.32 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-portal@v9.0.0-alpha.19 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-popover@v9.0.0-alpha.4 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-link@v9.0.0-alpha.43 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-make-styles@v9.0.0-alpha.40 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-image@v9.0.0-alpha.43 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-divider@v9.0.0-alpha.32 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-context-selector@v9.0.0-alpha.11 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-components@v9.0.0-alpha.58 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-button@v9.0.0-alpha.43 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-cards@v1.0.0-beta.102 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-accordion@v9.0.0-alpha.36 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-badge@v9.0.0-alpha.45 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-avatar@v9.0.0-alpha.44 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/babel-make-styles@v9.0.0-alpha.14 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/jest-serializer-make-styles@v9.0.0-alpha.16 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ling1726 ling1726 ling1726 approved these changes

@behowell behowell behowell approved these changes

@dzearing dzearing Awaiting requested review from dzearing

@layershifter layershifter Awaiting requested review from layershifter

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

Assignees
No one assigned
Labels
Component: MenuButton
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@khmakoto @miroslavstastny @fabricteam @ling1726 @msft-fluent-ui-bot @behowell @ecraig12345