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

Support content and secondary content slots in menuitem #17649

Merged
merged 4 commits into from Apr 1, 2021
Merged

Support content and secondary content slots in menuitem #17649

merged 4 commits into from Apr 1, 2021

Conversation

ling1726
Copy link
Member

@ling1726 ling1726 commented Mar 31, 2021 •
edited

Pull request checklist

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

Description of changes

Supports shortcut text for menu items as secondaryContent, in order to make layouting easier, it was necessary to create a content slot for component children.

TODO check with designers if there should be a minimum space between content and shortcut text

image

Focus areas to test

(optional)

@size-auditor
Copy link

size-auditor bot commented Mar 31, 2021 •
edited

Asset size changes

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

Baseline commit: c47f4a89c46f9864d569ee3547ec25800b38d2c2 (build)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 31, 2021 •
edited

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 2b133ae:

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

@fabricteam
Copy link
Collaborator

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 950 939 5000
BaseButton mount 889 903 5000
Breadcrumb mount 43518 43621 5000
ButtonNext mount 565 566 5000
Checkbox mount 1567 1568 5000
CheckboxBase mount 1288 1270 5000
ChoiceGroup mount 4766 4712 5000
ComboBox mount 981 1017 1000
CommandBar mount 10088 10110 1000
ContextualMenu mount 6282 6275 1000
DefaultButton mount 1123 1146 5000
DetailsRow mount 3662 3732 5000
DetailsRowFast mount 3672 3712 5000
DetailsRowNoStyles mount 3427 3394 5000
Dialog mount 1479 1518 1000
DocumentCardTitle mount 1826 1843 1000
Dropdown mount 3272 3239 5000
FocusTrapZone mount 1769 1769 5000
FocusZone mount 1769 1852 5000
IconButton mount 1700 1704 5000
Label mount 332 333 5000
Layer mount 1758 1776 5000
Link mount 462 448 5000
MakeStyles mount 1809 1790 50000
MenuButton mount 1455 1449 5000
MessageBar mount 2010 2063 5000
Nav mount 3506 3259 1000
OverflowSet mount 1041 1031 5000
Panel mount 1422 1412 1000
Persona mount 806 828 1000
Pivot mount 1375 1414 1000
PrimaryButton mount 1285 1280 5000
Rating mount 7515 7513 5000
SearchBox mount 1335 1306 5000
Shimmer mount 2587 2491 5000
Slider mount 1953 1990 5000
SpinButton mount 4925 4911 5000
Spinner mount 405 428 5000
SplitButton mount 3115 3127 5000
Stack mount 492 496 5000
StackWithIntrinsicChildren mount 1515 1499 5000
StackWithTextChildren mount 4533 4476 5000
SwatchColorPicker mount 10198 10077 5000
Tabs mount 1395 1409 1000
TagPicker mount 2820 2870 5000
TeachingBubble mount 11452 11455 5000
Text mount 417 412 5000
TextField mount 1334 1388 5000
ThemeProvider mount 1211 1213 5000
ThemeProvider virtual-rerender 600 599 5000
ThemeProviderNext mount 16029 15892 5000
Toggle mount 803 779 5000
buttonNative mount 119 116 5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
TextMinimalPerf.default 357 353 1.01:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.17 0.47 0.36:1 2000 336
🦄 Button.Fluent 0.11 0.2 0.55:1 5000 543
🔧 Checkbox.Fluent 0.62 0.36 1.72:1 1000 621
🦄 Dialog.Fluent 0.15 0.22 0.68:1 5000 741
🔧 Dropdown.Fluent 3.11 0.42 7.4:1 1000 3107
🔧 Icon.Fluent 0.13 0.06 2.17:1 5000 651
🦄 Image.Fluent 0.07 0.13 0.54:1 5000 372
🔧 Slider.Fluent 1.58 0.45 3.51:1 1000 1584
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 369
🦄 Tooltip.Fluent 0.14 0.89 0.16:1 5000 700

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
Icon.Fluent 651 591 1.1:1
ReactionMinimalPerf.default 408 389 1.05:1
AttachmentMinimalPerf.default 174 168 1.04:1
DropdownManyItemsPerf.default 706 679 1.04:1
AvatarMinimalPerf.default 217 211 1.03:1
ChatDuplicateMessagesPerf.default 303 295 1.03:1
FlexMinimalPerf.default 300 290 1.03:1
MenuMinimalPerf.default 908 881 1.03:1
PopupMinimalPerf.default 718 699 1.03:1
SegmentMinimalPerf.default 367 357 1.03:1
AttachmentSlotsPerf.default 1154 1134 1.02:1
BoxMinimalPerf.default 365 357 1.02:1
ButtonSlotsPerf.default 569 556 1.02:1
ButtonUseCssPerf.default 814 801 1.02:1
CardMinimalPerf.default 565 555 1.02:1
FormMinimalPerf.default 412 405 1.02:1
HeaderMinimalPerf.default 375 368 1.02:1
LayoutMinimalPerf.default 379 371 1.02:1
PortalMinimalPerf.default 174 170 1.02:1
ProviderMergeThemesPerf.default 1700 1673 1.02:1
StatusMinimalPerf.default 684 673 1.02:1
VideoMinimalPerf.default 648 636 1.02:1
Button.Fluent 543 534 1.02:1
Dialog.Fluent 741 724 1.02:1
ButtonMinimalPerf.default 180 178 1.01:1
CheckboxMinimalPerf.default 2756 2736 1.01:1
DatepickerMinimalPerf.default 45295 44815 1.01:1
GridMinimalPerf.default 363 361 1.01:1
ListCommonPerf.default 628 622 1.01:1
ListWith60ListItems.default 652 647 1.01:1
ProviderMinimalPerf.default 990 980 1.01:1
RadioGroupMinimalPerf.default 432 426 1.01:1
TextAreaMinimalPerf.default 493 487 1.01:1
CustomToolbarPrototype.default 3862 3828 1.01:1
Avatar.Fluent 336 333 1.01:1
Dropdown.Fluent 3107 3077 1.01:1
AnimationMinimalPerf.default 438 437 1:1
ChatMinimalPerf.default 614 611 1:1
DropdownMinimalPerf.default 3082 3083 1:1
EmbedMinimalPerf.default 4131 4136 1:1
HeaderSlotsPerf.default 767 766 1:1
InputMinimalPerf.default 1273 1277 1:1
LabelMinimalPerf.default 394 394 1:1
MenuButtonMinimalPerf.default 1534 1528 1:1
SkeletonMinimalPerf.default 374 373 1:1
ToolbarMinimalPerf.default 946 943 1:1
TreeWith60ListItems.default 187 187 1:1
Slider.Fluent 1584 1585 1:1
Text.Fluent 369 370 1:1
ButtonOverridesMissPerf.default 1700 1724 0.99:1
ButtonUseCssNestingPerf.default 1066 1074 0.99:1
DividerMinimalPerf.default 357 359 0.99:1
ImageMinimalPerf.default 376 379 0.99:1
ItemLayoutMinimalPerf.default 1269 1277 0.99:1
RosterPerf.default 1138 1150 0.99:1
RefMinimalPerf.default 247 249 0.99:1
SplitButtonMinimalPerf.default 3677 3732 0.99:1
TableManyItemsPerf.default 1899 1914 0.99:1
TooltipMinimalPerf.default 962 972 0.99:1
TreeMinimalPerf.default 782 787 0.99:1
AlertMinimalPerf.default 280 286 0.98:1
ListMinimalPerf.default 512 520 0.98:1
LoaderMinimalPerf.default 692 704 0.98:1
SliderMinimalPerf.default 1582 1616 0.98:1
TableMinimalPerf.default 413 420 0.98:1
Tooltip.Fluent 700 714 0.98:1
DialogMinimalPerf.default 728 752 0.97:1
Checkbox.Fluent 621 643 0.97:1
CarouselMinimalPerf.default 459 476 0.96:1
ListNestedPerf.default 566 590 0.96:1
IconMinimalPerf.default 597 625 0.96:1
ChatWithPopoverPerf.default 380 398 0.95:1
AccordionMinimalPerf.default 158 170 0.93:1
Image.Fluent 372 405 0.92:1

@ling1726 ling1726 merged commit 971ee5f into microsoft:master Apr 1, 2021
@msft-fluent-ui-bot
Copy link
Collaborator

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

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-examples@v8.13.0 has been released which incorporates this pull request.:tada:

Handy links:

miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
@ling1726 @miroslavstastny
Support content and secondary content slots in menuitem (microsoft#17649
a441613 
)

* Add slots for menu item content

* Change files

* update comment

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

@layershifter layershifter layershifter approved these changes

@bsunderhus bsunderhus Awaiting requested review from bsunderhus

Assignees

@khmakoto khmakoto

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ling1726 @fabricteam @msft-fluent-ui-bot @layershifter @khmakoto