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

chore(Menu): Convert to RFC #13040

Merged
merged 8 commits into from
May 12, 2020

Conversation

assuncaocharles
Copy link
Contributor

@assuncaocharles assuncaocharles commented May 7, 2020

Pull request checklist

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

BREAKING CHANGES

This PR converts Menu component to be functional. Restricting props set that will be passed to styles functions.

Related to #12237

Prop sets

Menu
iconOnly
fluid
pointing
pills
primary
underlined
vertical
submenu

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@msft-github-bot
Copy link
Contributor

msft-github-bot commented May 7, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 858 809 5000
Checkbox 1547 1557 5000
CheckboxBase 1313 1404 5000
CheckboxNext 1560 1509 5000
ChoiceGroup 4826 4833 5000
ComboBox 957 877 1000
CommandBar 7441 7698 1000
ContextualMenu 14961 15218 1000
DefaultButton 1062 1056 5000
DetailsRow 3356 3355 5000
DetailsRow (fast icons) 3342 3332 5000
DetailsRow without styles 3219 3198 5000
Dialog 1499 1489 1000
DocumentCardTitle with truncation 1630 1659 1000
Dropdown 2347 2283 5000
FocusZone 1689 1575 5000
IconButton 1603 1604 5000
Label 311 309 5000
Link 469 460 5000
MenuButton 1400 1424 5000
Nav 3138 3097 1000
Panel 1379 1443 1000
Persona 824 815 1000
Pivot 1268 1287 1000
PrimaryButton 1174 1187 5000
SearchBox 1169 1253 5000
Slider 1497 1438 5000
Spinner 394 383 5000
SplitButton 3059 2999 5000
Stack 469 446 5000
Stack with Intrinsic children 1166 1125 5000
Stack with Text children 3981 3912 5000
TagPicker 2636 2608 5000
Text 362 364 5000
TextField 1374 1321 5000
Toggle 909 865 5000
button 70 67 5000

Perf Analysis (Fluent)

⚠️ 2 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ChatWithPopoverPerf.default 543 601 0.9:1 analysis
MenuMinimalPerf.default 652 1842 0.35:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.46 0.47 0.98:1 2000 918
🦄 Button.Fluent 0.11 0.18 0.61:1 5000 526
🔧 Checkbox.Fluent 0.64 0.32 2:1 1000 637
🔧 Dialog.Fluent 0.34 0.21 1.62:1 5000 1689
🔧 Dropdown.Fluent 3.3 0.43 7.67:1 1000 3298
🔧 Icon.Fluent 0.15 0.05 3:1 5000 733
🦄 Image.Fluent 0.06 0.1 0.6:1 5000 316
🔧 Slider.Fluent 1.45 0.35 4.14:1 1000 1454
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 341
🦄 Tooltip.Fluent 0.09 18.61 0:1 5000 450

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ButtonMinimalPerf.default 181 158 1.15:1
BoxMinimalPerf.default 339 314 1.08:1
FlexMinimalPerf.default 277 258 1.07:1
GridMinimalPerf.default 667 622 1.07:1
TextAreaMinimalPerf.default 456 429 1.06:1
Icon.Fluent 733 692 1.06:1
Button.Fluent 526 502 1.05:1
ButtonSlotsPerf.default 633 606 1.04:1
HeaderMinimalPerf.default 503 482 1.04:1
ListMinimalPerf.default 462 443 1.04:1
ListWith60ListItems.default 1170 1128 1.04:1
PortalMinimalPerf.default 304 292 1.04:1
ProviderMinimalPerf.default 709 684 1.04:1
CheckboxMinimalPerf.default 2992 2899 1.03:1
InputMinimalPerf.default 1013 981 1.03:1
AnimationMinimalPerf.default 619 608 1.02:1
AvatarMinimalPerf.default 503 494 1.02:1
CardMinimalPerf.default 544 531 1.02:1
EmbedMinimalPerf.default 2001 1957 1.02:1
MenuButtonMinimalPerf.default 1526 1496 1.02:1
SplitButtonMinimalPerf.default 3435 3366 1.02:1
StatusMinimalPerf.default 673 662 1.02:1
VideoMinimalPerf.default 598 586 1.02:1
LabelMinimalPerf.default 399 396 1.01:1
ListCommonPerf.default 985 975 1.01:1
CustomToolbarPrototype.default 3670 3619 1.01:1
Dialog.Fluent 1689 1673 1.01:1
Text.Fluent 341 337 1.01:1
DividerMinimalPerf.default 337 336 1:1
ListNestedPerf.default 894 892 1:1
LoaderMinimalPerf.default 763 761 1:1
TextMinimalPerf.default 339 340 1:1
AttachmentMinimalPerf.default 140 142 0.99:1
ChatMinimalPerf.default 623 628 0.99:1
DialogMinimalPerf.default 1696 1718 0.99:1
DropdownMinimalPerf.default 3263 3301 0.99:1
ItemLayoutMinimalPerf.default 1661 1670 0.99:1
ReactionMinimalPerf.default 1886 1904 0.99:1
TableMinimalPerf.default 355 358 0.99:1
TreeMinimalPerf.default 1215 1230 0.99:1
Checkbox.Fluent 637 646 0.99:1
Dropdown.Fluent 3298 3340 0.99:1
Slider.Fluent 1454 1471 0.99:1
ChatDuplicateMessagesPerf.default 409 418 0.98:1
HierarchicalTreeMinimalPerf.default 985 1009 0.98:1
ImageMinimalPerf.default 346 352 0.98:1
PopupMinimalPerf.default 249 255 0.98:1
RefMinimalPerf.default 198 203 0.98:1
ToolbarMinimalPerf.default 793 810 0.98:1
AccordionMinimalPerf.default 129 133 0.97:1
ProviderMergeThemesPerf.default 1678 1734 0.97:1
SegmentMinimalPerf.default 941 970 0.97:1
SliderMinimalPerf.default 1387 1423 0.97:1
IconMinimalPerf.default 648 665 0.97:1
TreeWith60ListItems.default 228 236 0.97:1
CarouselMinimalPerf.default 481 500 0.96:1
DropdownManyItemsPerf.default 1298 1355 0.96:1
HeaderSlotsPerf.default 1415 1480 0.96:1
RadioGroupMinimalPerf.default 553 576 0.96:1
TooltipMinimalPerf.default 730 762 0.96:1
Avatar.Fluent 918 952 0.96:1
Tooltip.Fluent 450 470 0.96:1
AlertMinimalPerf.default 281 296 0.95:1
LayoutMinimalPerf.default 525 554 0.95:1
AttachmentSlotsPerf.default 1105 1195 0.92:1
Image.Fluent 316 355 0.89:1
FormMinimalPerf.default 715 855 0.84:1

@size-auditor
Copy link

size-auditor bot commented May 7, 2020

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 767c1b15e2dbc67d98d1f36d93d027e7cf881459 (build)

@assuncaocharles
Copy link
Contributor Author

@layershifter The screenshots are showing some changes, doesn't make much sense for me. Any idea?

@layershifter
Copy link
Member

Please revert changes related to variables and React Context. In Toolbar it allows to use component with Children API, here it just passes variables.

@msft-github-bot
Copy link
Contributor

Hello @layershifter!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msft-github-bot) and give me an instruction to get started! Learn more here.

@msft-github-bot msft-github-bot merged commit 0e7d10c into microsoft:master May 12, 2020
@assuncaocharles assuncaocharles deleted the chore/menu-rfc branch May 12, 2020 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants