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(AccordionTitle): remove hard icon dependencies #12160

Merged

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Mar 3, 2020

Pull request checklist

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

Description of changes

This PR removes the internal usage of the Icon component inside the AccordionTitle for the active indicator. The icon that was previously rendered as submenu indicator, is now added as part of the styles in the theme. See more details regarding this effort on #12145

BREAKING CHANGE

The indicator prop in the AccordionTitle component is now ShorthandValue<BoxProps> instead of ShorthandValue<IconProps>

Microsoft Reviewers: Open in CodeFlow

@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: cdb0f04597186741918262aeef64bbfb6b913911 (build)

@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 775 747
BaseButton (experiments) 1009 1022
DefaultButton 1013 1036
DefaultButton (experiments) 2031 1972
DetailsRow 3587 3484
DetailsRow (fast icons) 3512 3543
DetailsRow without styles 3212 3327
DocumentCardTitle with truncation 1609 1633
MenuButton 1377 1457
MenuButton (experiments) 3667 3513
PrimaryButton 1272 1233
PrimaryButton (experiments) 2035 2057
SplitButton 2829 2940
SplitButton (experiments) 6556 6804
Stack 447 453
Stack with Intrinsic children 1004 1016
Stack with Text children 4079 4094
Text 403 400
Toggle 858 858
Toggle (experiments) 2284 2342
button 68 75

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.57 0.52 1.1:1 2000 1131
🎯 Button.Fluent 0.16 0.23 0.7:1 1000 161
🔧 Checkbox.Fluent 0.96 0.41 2.34:1 1000 956
🔧 Dialog.Fluent 0.4 0.24 1.67:1 5000 1983
🔧 Dropdown.Fluent 4.15 0.55 7.55:1 1000 4149
🔧 Icon.Fluent 0.17 0.05 3.4:1 5000 872
🦄 Image.Fluent 0.07 0.11 0.64:1 5000 348
🔧 Slider.Fluent 1.57 0.41 3.83:1 1000 1567
🔧 Text.Fluent 0.08 0.02 4:1 5000 405
🦄 Tooltip.Fluent 0.13 19.79 0.01:1 5000 654

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ListWith60ListItems.default 210 185 1.14:1
PortalMinimalPerf.default 346 313 1.11:1
StatusMinimalPerf.default 360 332 1.08:1
ListNestedPerf.default 998 930 1.07:1
ButtonMinimalPerf.default 149 141 1.06:1
HierarchicalTreeMinimalPerf.default 1233 1167 1.06:1
TableMinimalPerf.default 796 753 1.06:1
ChatWithPopoverPerf.default 586 556 1.05:1
RefMinimalPerf.default 223 212 1.05:1
TextMinimalPerf.default 427 407 1.05:1
BoxMinimalPerf.default 325 314 1.04:1
ChatMinimalPerf.default 544 524 1.04:1
FlexMinimalPerf.default 265 256 1.04:1
InputMinimalPerf.default 1114 1070 1.04:1
PopupMinimalPerf.default 466 447 1.04:1
AttachmentMinimalPerf.default 877 852 1.03:1
ChatDuplicateMessagesPerf.default 409 396 1.03:1
DropdownManyItemsPerf.default 341 332 1.03:1
GridMinimalPerf.default 891 863 1.03:1
IconMinimalPerf.default 411 400 1.03:1
LoaderMinimalPerf.default 1136 1106 1.03:1
Avatar.Fluent 1131 1094 1.03:1
Dropdown.Fluent 4149 4037 1.03:1
Image.Fluent 348 338 1.03:1
AvatarMinimalPerf.default 533 520 1.02:1
ProviderMinimalPerf.default 724 713 1.02:1
TextAreaMinimalPerf.default 3411 3350 1.02:1
ToolbarMinimalPerf.default 1250 1223 1.02:1
Checkbox.Fluent 956 936 1.02:1
AnimationMinimalPerf.default 651 647 1.01:1
ButtonSlotsPerf.default 730 722 1.01:1
DialogMinimalPerf.default 1817 1806 1.01:1
FormMinimalPerf.default 981 968 1.01:1
HeaderMinimalPerf.default 585 582 1.01:1
ItemLayoutMinimalPerf.default 2170 2146 1.01:1
LabelMinimalPerf.default 377 372 1.01:1
MenuMinimalPerf.default 2251 2229 1.01:1
SegmentMinimalPerf.default 1272 1262 1.01:1
TreeMinimalPerf.default 1230 1220 1.01:1
Text.Fluent 405 401 1.01:1
LayoutMinimalPerf.default 752 755 1:1
ListMinimalPerf.default 467 468 1:1
MenuButtonMinimalPerf.default 2110 2114 1:1
SliderMinimalPerf.default 1588 1593 1:1
SplitButtonMinimalPerf.default 13765 13800 1:1
CustomToolbarPrototype.default 4098 4085 1:1
AttachmentSlotsPerf.default 3372 3392 0.99:1
DividerMinimalPerf.default 1039 1045 0.99:1
DropdownMinimalPerf.default 3590 3633 0.99:1
HeaderSlotsPerf.default 1748 1760 0.99:1
RadioGroupMinimalPerf.default 627 633 0.99:1
ReactionMinimalPerf.default 2640 2671 0.99:1
TooltipMinimalPerf.default 903 911 0.99:1
CarouselMinimalPerf.default 1971 2008 0.98:1
ProviderMergeThemesPerf.default 1374 1407 0.98:1
Icon.Fluent 872 887 0.98:1
Slider.Fluent 1567 1595 0.98:1
Tooltip.Fluent 654 670 0.98:1
CheckboxMinimalPerf.default 3912 4025 0.97:1
EmbedMinimalPerf.default 6169 6357 0.97:1
ListCommonPerf.default 1051 1086 0.97:1
VideoMinimalPerf.default 997 1038 0.96:1
Button.Fluent 161 167 0.96:1
Dialog.Fluent 1983 2059 0.96:1
AccordionMinimalPerf.default 234 250 0.94:1
AlertMinimalPerf.default 546 590 0.93:1
ImageMinimalPerf.default 350 380 0.92:1
TreeWith60ListItems.default 231 261 0.89:1

@mnajdova mnajdova merged commit 81f4003 into microsoft:master Mar 3, 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.

None yet

5 participants