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(Toolbar): convert ToolbarDivider, ToolbarCustomItem & ToolbarItem to functional components #12161

Merged
merged 9 commits into from
Mar 3, 2020

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Mar 3, 2020

Pull request checklist

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

BREAKING CHANGES

This PR converts ToolbarDivider, ToolbarCustomItem & ToolbarItem components to be functional. As in all other cases it contains a breaking change with restricted props set that will be passed to styles functions.

Prop sets

ToolbarDivider
no props
ToolbarCustomItem
fitted
ToolbarItem
active
disabled
Microsoft Reviewers: Open in CodeFlow

@@ -39,9 +40,9 @@ export default toolbarItemBehavior;

export type ToolbarItemBehaviorProps = {
/** Indicated if toolbar item has a menu. */
menu?: boolean | object;
hasMenu?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Mar 3, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 747 757
BaseButton (experiments) 951 900
DefaultButton 938 970
DefaultButton (experiments) 1863 1874
DetailsRow 3291 3476
DetailsRow (fast icons) 3249 3243
DetailsRow without styles 3003 3013
DocumentCardTitle with truncation 1506 1480
MenuButton 1340 1347
MenuButton (experiments) 3436 3407
PrimaryButton 1121 1127
PrimaryButton (experiments) 1903 1967
SplitButton 2821 2916
SplitButton (experiments) 6720 6712
Stack 469 484
Stack with Intrinsic children 1046 1075
Stack with Text children 4080 4142
Text 378 408
Toggle 814 787
Toggle (experiments) 2271 2179
button 59 60

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
CustomToolbarPrototype.default 3717 3793 0.98:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.51 0.49 1.04:1 2000 1018
🦄 Button.Fluent 0.16 0.26 0.62:1 1000 161
🔧 Checkbox.Fluent 0.88 0.39 2.26:1 1000 876
🔧 Dialog.Fluent 0.39 0.23 1.7:1 5000 1945
🔧 Dropdown.Fluent 3.71 0.5 7.42:1 1000 3706
🔧 Icon.Fluent 0.17 0.06 2.83:1 5000 872
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 365
🔧 Slider.Fluent 1.49 0.42 3.55:1 1000 1494
🔧 Text.Fluent 0.08 0.02 4:1 5000 388
🦄 Tooltip.Fluent 0.13 15.06 0.01:1 5000 629

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ListMinimalPerf.default 452 412 1.1:1
ListWith60ListItems.default 185 170 1.09:1
ChatDuplicateMessagesPerf.default 446 420 1.06:1
PortalMinimalPerf.default 309 292 1.06:1
SegmentMinimalPerf.default 1192 1128 1.06:1
VideoMinimalPerf.default 952 899 1.06:1
Image.Fluent 365 344 1.06:1
ChatMinimalPerf.default 599 569 1.05:1
TreeMinimalPerf.default 1259 1201 1.05:1
BoxMinimalPerf.default 339 327 1.04:1
FormMinimalPerf.default 1065 1023 1.04:1
ListNestedPerf.default 946 907 1.04:1
MenuMinimalPerf.default 2152 2063 1.04:1
ProviderMinimalPerf.default 655 628 1.04:1
Icon.Fluent 872 839 1.04:1
GridMinimalPerf.default 941 915 1.03:1
IconMinimalPerf.default 404 393 1.03:1
ImageMinimalPerf.default 361 352 1.03:1
ItemLayoutMinimalPerf.default 2227 2165 1.03:1
Dialog.Fluent 1945 1894 1.03:1
AvatarMinimalPerf.default 537 529 1.02:1
InputMinimalPerf.default 1036 1014 1.02:1
MenuButtonMinimalPerf.default 2013 1964 1.02:1
SliderMinimalPerf.default 1507 1484 1.02:1
DropdownMinimalPerf.default 3753 3707 1.01:1
EmbedMinimalPerf.default 6200 6148 1.01:1
HierarchicalTreeMinimalPerf.default 1107 1100 1.01:1
LabelMinimalPerf.default 374 369 1.01:1
ListCommonPerf.default 1022 1015 1.01:1
RadioGroupMinimalPerf.default 599 594 1.01:1
TableMinimalPerf.default 721 717 1.01:1
Tooltip.Fluent 629 623 1.01:1
AttachmentSlotsPerf.default 3531 3517 1:1
DialogMinimalPerf.default 1888 1879 1:1
HeaderSlotsPerf.default 1764 1762 1:1
LoaderMinimalPerf.default 1052 1052 1:1
SplitButtonMinimalPerf.default 12677 12641 1:1
TextAreaMinimalPerf.default 3206 3221 1:1
TooltipMinimalPerf.default 893 895 1:1
Slider.Fluent 1494 1489 1:1
AlertMinimalPerf.default 611 619 0.99:1
ButtonMinimalPerf.default 141 142 0.99:1
ChatWithPopoverPerf.default 598 602 0.99:1
CheckboxMinimalPerf.default 4048 4099 0.99:1
DropdownManyItemsPerf.default 341 346 0.99:1
FlexMinimalPerf.default 282 286 0.99:1
HeaderMinimalPerf.default 598 606 0.99:1
PopupMinimalPerf.default 434 440 0.99:1
ProviderMergeThemesPerf.default 1287 1305 0.99:1
StatusMinimalPerf.default 330 332 0.99:1
Button.Fluent 161 162 0.99:1
Dropdown.Fluent 3706 3745 0.99:1
Text.Fluent 388 390 0.99:1
AccordionMinimalPerf.default 251 256 0.98:1
AnimationMinimalPerf.default 673 685 0.98:1
CarouselMinimalPerf.default 2043 2086 0.98:1
ReactionMinimalPerf.default 2485 2548 0.98:1
Avatar.Fluent 1018 1035 0.98:1
DividerMinimalPerf.default 1021 1049 0.97:1
TextMinimalPerf.default 363 374 0.97:1
AttachmentMinimalPerf.default 883 917 0.96:1
ButtonSlotsPerf.default 737 764 0.96:1
RefMinimalPerf.default 184 194 0.95:1
TreeWith60ListItems.default 226 237 0.95:1
Checkbox.Fluent 876 924 0.95:1
ToolbarMinimalPerf.default 1175 1259 0.93:1
LayoutMinimalPerf.default 710 768 0.92:1

@size-auditor
Copy link

size-auditor bot commented Mar 3, 2020

Asset size changes

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

Baseline commit: a724c8b57e290179f95db3f64759d4a11132a234 (build)

@@ -46,49 +46,70 @@ export interface ToolbarCustomItemProps extends UIComponentProps, ChildrenCompon
onBlur?: ComponentEventHandler<ToolbarCustomItemProps>;
}

class ToolbarCustomItem extends UIComponent<WithAsProp<ToolbarCustomItemProps>> {
static displayName = 'ToolbarCustomItem';
export type ToolbarCustomItemStylesProps = Required<Pick<ToolbarCustomItemProps, 'fitted'>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, why do you set 'fitted' as a required style prop?

@layershifter layershifter changed the title chore(Toolbar): convert ToolbarDivider, ToolbarCustomItem & ToolbarItem to functional components [WIP] chore(Toolbar): convert ToolbarDivider, ToolbarCustomItem & ToolbarItem to functional components Mar 3, 2020
@layershifter layershifter merged commit 8fa2d72 into master Mar 3, 2020
@layershifter layershifter deleted the chore/toolbar-cmpts-to-fc branch March 3, 2020 17:26
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

5 participants