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

fix(Button): children styles differ than shorthand #12292

Merged
merged 3 commits into from
Mar 13, 2020

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Mar 13, 2020

This PR fixes microsoft/fluent-ui-react#1589 - children styles differ than shorthand. For fixing the issue, I've created ButtonContent component, that contains all styles that were previosly in the buttonStyle's content slot, and used this component in the children examples that we had.

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Mar 13, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 693 692
BaseButton (experiments) 885 883
DefaultButton 993 953
DefaultButton (experiments) 1754 1734
DetailsRow 3168 3139
DetailsRow (fast icons) 3104 3133
DetailsRow without styles 2899 2900
DocumentCardTitle with truncation 1548 1570
MenuButton 1268 1262
MenuButton (experiments) 3312 3253
PrimaryButton 1134 1109
PrimaryButton (experiments) 1791 1816
SplitButton 2641 2670
SplitButton (experiments) 6566 6521
Stack 434 424
Stack with Intrinsic children 1022 1036
Stack with Text children 3732 3720
Text 333 343
Toggle 780 765
Toggle (experiments) 2075 2046
button 74 59

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ButtonSlotsPerf.default 637 739 0.86:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.53 0.49 1.08:1 2000 1053
🦄 Button.Fluent 0.14 0.22 0.64:1 1000 140
🔧 Checkbox.Fluent 0.76 0.42 1.81:1 1000 756
🔧 Dialog.Fluent 0.39 0.22 1.77:1 5000 1973
🔧 Dropdown.Fluent 3.7 0.52 7.12:1 1000 3697
🔧 Icon.Fluent 0.16 0.05 3.2:1 5000 810
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 327
🔧 Slider.Fluent 1.58 0.41 3.85:1 1000 1579
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 356
🦄 Tooltip.Fluent 0.12 18.71 0.01:1 5000 589

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
DropdownManyItemsPerf.default 362 336 1.08:1
TooltipMinimalPerf.default 865 810 1.07:1
LabelMinimalPerf.default 362 340 1.06:1
PortalMinimalPerf.default 295 279 1.06:1
StatusMinimalPerf.default 341 326 1.05:1
TextMinimalPerf.default 363 346 1.05:1
AlertMinimalPerf.default 645 618 1.04:1
AvatarMinimalPerf.default 579 557 1.04:1
FlexMinimalPerf.default 252 242 1.04:1
ItemLayoutMinimalPerf.default 2096 2016 1.04:1
ListNestedPerf.default 952 919 1.04:1
ReactionMinimalPerf.default 2562 2466 1.04:1
TreeWith60ListItems.default 227 219 1.04:1
ChatDuplicateMessagesPerf.default 429 418 1.03:1
DividerMinimalPerf.default 972 945 1.03:1
HierarchicalTreeMinimalPerf.default 1008 981 1.03:1
PopupMinimalPerf.default 253 246 1.03:1
Checkbox.Fluent 756 737 1.03:1
Icon.Fluent 810 786 1.03:1
EmbedMinimalPerf.default 5618 5519 1.02:1
ProviderMinimalPerf.default 653 640 1.02:1
RefMinimalPerf.default 194 191 1.02:1
SegmentMinimalPerf.default 1088 1070 1.02:1
TextAreaMinimalPerf.default 3214 3163 1.02:1
Dialog.Fluent 1973 1938 1.02:1
Dropdown.Fluent 3697 3614 1.02:1
AnimationMinimalPerf.default 623 615 1.01:1
AttachmentSlotsPerf.default 3696 3642 1.01:1
CarouselMinimalPerf.default 2132 2117 1.01:1
ChatMinimalPerf.default 549 545 1.01:1
FormMinimalPerf.default 915 904 1.01:1
HeaderSlotsPerf.default 1617 1598 1.01:1
ListWith60ListItems.default 190 189 1.01:1
TableMinimalPerf.default 638 629 1.01:1
Avatar.Fluent 1053 1045 1.01:1
Slider.Fluent 1579 1560 1.01:1
Text.Fluent 356 353 1.01:1
AttachmentMinimalPerf.default 950 954 1:1
BoxMinimalPerf.default 327 327 1:1
CheckboxMinimalPerf.default 3273 3267 1:1
DropdownMinimalPerf.default 3692 3675 1:1
ImageMinimalPerf.default 321 322 1:1
MenuMinimalPerf.default 2029 2031 1:1
MenuButtonMinimalPerf.default 1551 1552 1:1
RadioGroupMinimalPerf.default 529 531 1:1
CustomToolbarPrototype.default 3741 3747 1:1
ToolbarMinimalPerf.default 1099 1096 1:1
TreeMinimalPerf.default 1142 1142 1:1
VideoMinimalPerf.default 857 857 1:1
ButtonMinimalPerf.default 143 144 0.99:1
ChatWithPopoverPerf.default 587 592 0.99:1
DialogMinimalPerf.default 1913 1925 0.99:1
LayoutMinimalPerf.default 627 634 0.99:1
ListCommonPerf.default 976 981 0.99:1
LoaderMinimalPerf.default 1076 1084 0.99:1
SplitButtonMinimalPerf.default 12536 12618 0.99:1
Image.Fluent 327 329 0.99:1
HeaderMinimalPerf.default 534 547 0.98:1
IconMinimalPerf.default 402 411 0.98:1
ProviderMergeThemesPerf.default 1354 1382 0.98:1
Tooltip.Fluent 589 603 0.98:1
ListMinimalPerf.default 425 440 0.97:1
SliderMinimalPerf.default 1529 1584 0.97:1
Button.Fluent 140 144 0.97:1
GridMinimalPerf.default 789 825 0.96:1
InputMinimalPerf.default 1020 1062 0.96:1
AccordionMinimalPerf.default 237 252 0.94:1

@size-auditor
Copy link

size-auditor bot commented Mar 13, 2020

Asset size changes

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

Baseline commit: 755b666e2ecaf5aedb5e39ff84233cb7a9a11d0e (build)

@mnajdova mnajdova merged commit 72b3c86 into microsoft:master Mar 13, 2020
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.

Button: different styling for shorthand and children APIs
3 participants