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

feat(factories): changed createShorthand signature, to use the optional shorthandConfig static on the component #12783

Merged
merged 7 commits into from
Apr 21, 2020

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Apr 20, 2020

This PR is adding updated createShorthand utility, that based on the static shorthandConfig on the component, allows user to create react elements from the shorthand. As a showcase it is used on this PR to replace the Button.create, AccordionTitle.create and AccordionContent.create usages (this covers both functional and class components). This will allow us to have more flexible shorthand story, for allowing users to define their own components for the shorthands (this will be required later on on the compose factory).

BREAKING CHANGES

The mappedProp, allowsJSX and mappedArrayProp are expected to be defined as a static shorthandConfig on the component. If it is not defined, this are the defaults:

mappedProp = 'children'
allowsJSX = true
mappedArrayProp = undefined

Example:

// before
createShorthand({
  Component: Button,
  mappedProp: 'content',
  value: buttonShorthandValue,
  options = {},
});

// after
Button.shorthandConfig = {
   mappedProp: 'content',
};

createShorthand(Button, buttonShorthandValue, {});

-added shorthandConfig on the Button component
-replaced Button.create usages
@msft-github-bot
Copy link
Contributor

msft-github-bot commented Apr 20, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 837 820 5000
Checkbox 1508 1518 5000
CheckboxBase 1216 1293 5000
ChoiceGroup 4846 4796 5000
ComboBox 870 860 1000
CommandBar 7003 7067 1000
ContextualMenu 13959 13307 1000
DefaultButton 1093 1035 5000
DetailsRow 3224 3333 5000
DetailsRow (fast icons) 3223 3191 5000
DetailsRow without styles 3011 3040 5000
Dialog 1355 1370 1000
DocumentCardTitle with truncation 1442 1409 1000
Dropdown 2503 2279 5000
FocusZone 1499 1456 5000
IconButton 1638 1625 5000
Label 279 281 5000
Link 437 420 5000
MenuButton 1320 1332 5000
Nav 2935 2943 1000
Panel 1367 1354 1000
Persona 752 785 1000
Pivot 1220 1237 1000
PrimaryButton 1131 1145 5000
SearchBox 1152 1214 5000
Slider 1389 1357 5000
Spinner 386 382 5000
SplitButton 3097 2913 5000
Stack 453 454 5000
Stack with Intrinsic children 1041 1015 5000
Stack with Text children 3989 3886 5000
TagPicker 2530 2476 5000
Text 354 358 5000
TextField 1321 1289 5000
Toggle 841 835 5000
button 59 50 5000

Perf Analysis (Fluent)

⚠️ 3 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
MenuButtonMinimalPerf.default 1592 1545 1.03:1 analysis
LoaderMinimalPerf.default 725 710 1.02:1 analysis
AlertMinimalPerf.default 282 291 0.97:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.48 0.44 1.09:1 2000 959
🦄 Button.Fluent 0.11 0.18 0.61:1 5000 529
🔧 Checkbox.Fluent 0.61 0.33 1.85:1 1000 610
🔧 Dialog.Fluent 0.34 0.2 1.7:1 5000 1703
🔧 Dropdown.Fluent 2.96 0.44 6.73:1 1000 2957
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 660
🎯 Image.Fluent 0.08 0.1 0.8:1 5000 398
🔧 Slider.Fluent 1.26 0.33 3.82:1 1000 1258
🔧 Text.Fluent 0.08 0.02 4:1 5000 393
🦄 Tooltip.Fluent 0.09 15.65 0.01:1 5000 426

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PortalMinimalPerf.default 325 288 1.13:1
PopupMinimalPerf.default 260 237 1.1:1
FlexMinimalPerf.default 334 306 1.09:1
StatusMinimalPerf.default 780 713 1.09:1
TextMinimalPerf.default 392 358 1.09:1
AttachmentMinimalPerf.default 139 129 1.08:1
TooltipMinimalPerf.default 671 624 1.08:1
Text.Fluent 393 363 1.08:1
InputMinimalPerf.default 943 882 1.07:1
LayoutMinimalPerf.default 602 566 1.06:1
RefMinimalPerf.default 186 176 1.06:1
IconMinimalPerf.default 670 630 1.06:1
AnimationMinimalPerf.default 684 654 1.05:1
ChatMinimalPerf.default 657 626 1.05:1
ListCommonPerf.default 1016 972 1.05:1
ListNestedPerf.default 919 875 1.05:1
ToolbarMinimalPerf.default 1041 994 1.05:1
Dialog.Fluent 1703 1615 1.05:1
BoxMinimalPerf.default 344 330 1.04:1
CheckboxMinimalPerf.default 2822 2720 1.04:1
AccordionMinimalPerf.default 204 198 1.03:1
DropdownManyItemsPerf.default 1292 1259 1.03:1
ListWith60ListItems.default 1138 1105 1.03:1
Avatar.Fluent 959 929 1.03:1
AttachmentSlotsPerf.default 1085 1061 1.02:1
AvatarMinimalPerf.default 487 476 1.02:1
ChatWithPopoverPerf.default 544 532 1.02:1
LabelMinimalPerf.default 441 433 1.02:1
ListMinimalPerf.default 476 467 1.02:1
SliderMinimalPerf.default 1311 1286 1.02:1
CarouselMinimalPerf.default 586 582 1.01:1
FormMinimalPerf.default 762 751 1.01:1
HeaderMinimalPerf.default 506 500 1.01:1
ImageMinimalPerf.default 394 389 1.01:1
ProviderMergeThemesPerf.default 1428 1409 1.01:1
ProviderMinimalPerf.default 593 590 1.01:1
CustomToolbarPrototype.default 3348 3321 1.01:1
Button.Fluent 529 525 1.01:1
Checkbox.Fluent 610 605 1.01:1
Image.Fluent 398 396 1.01:1
CardMinimalPerf.default 583 582 1:1
HierarchicalTreeMinimalPerf.default 1011 1007 1:1
ItemLayoutMinimalPerf.default 1665 1664 1:1
MenuMinimalPerf.default 1818 1822 1:1
RadioGroupMinimalPerf.default 576 578 1:1
SplitButtonMinimalPerf.default 3609 3594 1:1
ButtonSlotsPerf.default 567 574 0.99:1
DividerMinimalPerf.default 731 740 0.99:1
DropdownMinimalPerf.default 2999 3029 0.99:1
EmbedMinimalPerf.default 4006 4047 0.99:1
GridMinimalPerf.default 674 679 0.99:1
ChatDuplicateMessagesPerf.default 387 393 0.98:1
TextAreaMinimalPerf.default 2592 2655 0.98:1
TreeMinimalPerf.default 1147 1176 0.98:1
Dropdown.Fluent 2957 3004 0.98:1
Slider.Fluent 1258 1280 0.98:1
HeaderSlotsPerf.default 1455 1497 0.97:1
ReactionMinimalPerf.default 1769 1815 0.97:1
SegmentMinimalPerf.default 893 918 0.97:1
ButtonMinimalPerf.default 164 170 0.96:1
TableMinimalPerf.default 571 600 0.95:1
DialogMinimalPerf.default 1678 1803 0.93:1
TreeWith60ListItems.default 213 229 0.93:1
Icon.Fluent 660 712 0.93:1
Tooltip.Fluent 426 459 0.93:1
VideoMinimalPerf.default 772 851 0.91:1

@size-auditor
Copy link

size-auditor bot commented Apr 20, 2020

Asset size changes

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

Baseline commit: 89b53e7f43d70ffac4e9d308428356c90b8ac06b (build)

@mnajdova mnajdova changed the title [WIP] chore: remove create factories chore: remove create factories Apr 20, 2020
@@ -68,7 +69,7 @@ class ButtonGroup extends UIComponent<WithAsProp<ButtonGroupProps>, any> {
return (
<ElementType {...unhandledProps} className={classes.root}>
{_.map(buttons, (button, idx) =>
Button.create(button, {
createShorthand<WithAsProp<ButtonProps>>(Button, button, {
Copy link
Member

Choose a reason for hiding this comment

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

I expect that this will be inferred from the first param

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sense, I changed it

@mnajdova mnajdova changed the title chore: remove create factories feat(factories): changed createShorthand signature, to use the optional shorthandConfig static on the component Apr 21, 2020
@mnajdova mnajdova merged commit a328ef5 into microsoft:master Apr 21, 2020
DuanShaolong pushed a commit to DuanShaolong/fluentui that referenced this pull request Apr 27, 2020
…al shorthandConfig static on the component (microsoft#12783)

* -added updated createdShorthand method
-added shorthandConfig on the Button component
-replaced Button.create usages

* -fixed typings

* -removed console log

* -removed unnecessary undefined

* -improved typings
-added class component example

* -updated changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants