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

Copy ContextualMenu to react-next #14391

Merged
merged 13 commits into from
Aug 12, 2020
Merged

Copy ContextualMenu to react-next #14391

merged 13 commits into from
Aug 12, 2020

Conversation

MLoughry
Copy link
Contributor

@MLoughry MLoughry commented Aug 6, 2020

Pull request checklist

  • Include a change request file using $ yarn change

Description of changes

Copy ContextualMenu to react-next to stage for fixing of StrictMode issues.

Focus areas to test

(optional)

@MLoughry MLoughry requested a review from joschect as a code owner August 6, 2020 20:31
@MLoughry MLoughry requested a review from ecraig12345 August 6, 2020 20:31
@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 6, 2020 •
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 5137a70:

Sandbox Source
Fluent UI Button Configuration
microsoft/fluentui: codesandbox-react-template Configuration
microsoft/fluentui: codesandbox-react-next-template Configuration
microsoft/fluentui: codesandbox-react-northstar-template Configuration

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Aug 6, 2020 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 923 945 5000
ButtonNext mount 634 607 5000
Checkbox mount 1621 1660 5000
CheckboxBase mount 1316 1374 5000
CheckboxNext mount 1781 1701 5000
ChoiceGroup mount 5118 5222 5000
ComboBox mount 953 975 1000
CommandBar mount 7995 7844 1000
ContextualMenu mount 13658 13938 1000
DefaultButton mount 1144 1144 5000
DetailsRow mount 3678 3636 5000
DetailsRowFast mount 3568 3690 5000
DetailsRowNoStyles mount 3490 3514 5000
Dialog mount 1558 1523 1000
DocumentCardTitle mount 1840 1876 1000
Dropdown mount 2668 2652 5000
FocusZone mount 1886 1989 5000
IconButton mount 1720 1810 5000
Label mount 352 342 5000
Link mount 461 470 5000
LinkNext mount 482 486 5000
MenuButton mount 1552 1450 5000
Nav mount 3305 3384 1000
Panel mount 1462 1534 1000
Persona mount 867 896 1000
Pivot mount 1443 1435 1000
PivotNext mount 1602 1436 1000
PrimaryButton mount 1300 1323 5000
SearchBox mount 1354 1425 5000
SearchBoxNext mount 1448 1387 5000
Slider mount 1530 1560 5000
SliderNext mount 1963 2035 5000
SpinButton mount 5147 5218 5000
SpinButtonNext mount 5270 5138 5000
Spinner mount 442 450 5000
SplitButton mount 3226 3158 5000
Stack mount 539 539 5000
StackWithIntrinsicChildren mount 2152 2171 5000
StackWithTextChildren mount 5259 5197 5000
TagPicker mount 2740 2896 5000
Text mount 423 445 5000
TextField mount 1475 1390 5000
ThemeProvider mount 2936 2795 5000
ThemeProvider virtual-rerender 476 450 5000
Toggle mount 825 893 5000
ToggleNext mount 856 840 5000
button mount 110 116 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.44 0.47 0.94:1 2000 888
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 526
🔧 Checkbox.Fluent 0.66 0.34 1.94:1 1000 656
🎯 Dialog.Fluent 0.16 0.23 0.7:1 5000 800
🔧 Dropdown.Fluent 3.1 0.5 6.2:1 1000 3096
🔧 Icon.Fluent 0.15 0.05 3:1 5000 739
🦄 Image.Fluent 0.07 0.11 0.64:1 5000 350
🔧 Slider.Fluent 1.65 0.35 4.71:1 1000 1654
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 331
🦄 Tooltip.Fluent 0.11 17.7 0.01:1 5000 541

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PortalMinimalPerf.default 140 121 1.16:1
IconMinimalPerf.default 699 648 1.08:1
AttachmentMinimalPerf.default 176 164 1.07:1
RadioGroupMinimalPerf.default 455 425 1.07:1
TextMinimalPerf.default 354 330 1.07:1
AnimationMinimalPerf.default 412 390 1.06:1
SegmentMinimalPerf.default 350 330 1.06:1
Icon.Fluent 739 698 1.06:1
AttachmentSlotsPerf.default 1226 1165 1.05:1
ChatDuplicateMessagesPerf.default 447 425 1.05:1
DialogMinimalPerf.default 801 762 1.05:1
DividerMinimalPerf.default 366 350 1.05:1
MenuMinimalPerf.default 897 857 1.05:1
SplitButtonMinimalPerf.default 4024 3846 1.05:1
Tooltip.Fluent 541 516 1.05:1
AvatarMinimalPerf.default 490 472 1.04:1
BoxMinimalPerf.default 347 333 1.04:1
ReactionMinimalPerf.default 394 378 1.04:1
TableMinimalPerf.default 403 387 1.04:1
VideoMinimalPerf.default 626 604 1.04:1
ButtonMinimalPerf.default 180 175 1.03:1
InputMinimalPerf.default 1388 1347 1.03:1
ToolbarMinimalPerf.default 983 957 1.03:1
CarouselMinimalPerf.default 462 453 1.02:1
GridMinimalPerf.default 332 326 1.02:1
HeaderMinimalPerf.default 357 350 1.02:1
ListWith60ListItems.default 1154 1127 1.02:1
ProviderMinimalPerf.default 994 977 1.02:1
TooltipMinimalPerf.default 809 794 1.02:1
DropdownManyItemsPerf.default 786 781 1.01:1
HeaderSlotsPerf.default 775 764 1.01:1
ItemLayoutMinimalPerf.default 1276 1268 1.01:1
ListCommonPerf.default 992 982 1.01:1
LoaderMinimalPerf.default 765 757 1.01:1
PopupMinimalPerf.default 697 689 1.01:1
ProviderMergeThemesPerf.default 2004 1976 1.01:1
Button.Fluent 526 522 1.01:1
Checkbox.Fluent 656 647 1.01:1
Dialog.Fluent 800 793 1.01:1
ButtonSlotsPerf.default 603 606 1:1
CheckboxMinimalPerf.default 2950 2944 1:1
FormMinimalPerf.default 399 401 1:1
HierarchicalTreeMinimalPerf.default 420 422 1:1
LayoutMinimalPerf.default 390 391 1:1
ListNestedPerf.default 907 910 1:1
SliderMinimalPerf.default 1699 1695 1:1
TableManyItemsPerf.default 2237 2228 1:1
Avatar.Fluent 888 891 1:1
Dropdown.Fluent 3096 3095 1:1
Image.Fluent 350 349 1:1
CardMinimalPerf.default 545 552 0.99:1
ChatMinimalPerf.default 605 610 0.99:1
DropdownMinimalPerf.default 3092 3131 0.99:1
ImageMinimalPerf.default 363 367 0.99:1
LabelMinimalPerf.default 406 410 0.99:1
MenuButtonMinimalPerf.default 1565 1577 0.99:1
StatusMinimalPerf.default 692 699 0.99:1
CustomToolbarPrototype.default 3940 3971 0.99:1
TreeMinimalPerf.default 869 881 0.99:1
TreeWith60ListItems.default 228 230 0.99:1
Slider.Fluent 1654 1692 0.98:1
EmbedMinimalPerf.default 1954 2012 0.97:1
ListMinimalPerf.default 491 507 0.97:1
TextAreaMinimalPerf.default 455 471 0.97:1
AlertMinimalPerf.default 317 330 0.96:1
ChatWithPopoverPerf.default 483 505 0.96:1
RefMinimalPerf.default 216 224 0.96:1
Text.Fluent 331 347 0.95:1
FlexMinimalPerf.default 282 305 0.92:1
SkeletonMinimalPerf.default 385 420 0.92:1
AccordionMinimalPerf.default 139 156 0.89:1

ecraig12345
ecraig12345 requested changes Aug 7, 2020
View reviewed changes
Copy link
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

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

Requesting changes due to common scss addition (see below)

packages/react-next/etc/react-next.api.md Show resolved Hide resolved
packages/react-next/src/common/ThemingSass.scss Outdated Show resolved Hide resolved
@MLoughry
Copy link
Contributor Author

@ecraig12345 @dzearing I'm unable to reproduce the build failure locally, nor am I sure what the issue is.

[10:02:36 PM] x Error detected while running 'bundle'
[10:02:36 PM] x ------------------------------------
[10:02:36 PM] x [WEBPACK] Errors building fluentui-react-next-ContextualMenu.min.js
Module not found: Error: Can't resolve 'office-ui-fabric-react/src/components/Divider/VerticalDivider.classNames' in 'D:\a\1\s\packages\react-next\lib\components\ContextualMenu'
[10:02:36 PM] x ------------------------------------

@ecraig12345 ecraig12345 merged commit e34570d into microsoft:master Aug 12, 2020
@ecraig12345
Copy link
Member

Went ahead and merged this since the size auditor failure was clearly unrelated this time.

levithomason pushed a commit to levithomason/fluentui that referenced this pull request Aug 24, 2020
@MLoughry @levithomason
Copy ContextualMenu to react-next (microsoft#14391)
80e91b1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behowell behowell behowell left review comments

@ecraig12345 ecraig12345 ecraig12345 approved these changes

@joschect joschect Awaiting requested review from joschect

Assignees

@khmakoto khmakoto

Labels
Component: ContextualMenu PR: API Modified
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MLoughry @msft-github-bot @ecraig12345 @behowell @khmakoto