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(Avatar): add icon prop #12660

Merged
merged 5 commits into from
Apr 14, 2020

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Apr 13, 2020

Pull request checklist

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

Description of changes

(give an overview)

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@size-auditor
Copy link

size-auditor bot commented Apr 13, 2020

Asset size changes

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

Baseline commit: 7927551e96fa5b7572425d947b58ba07c826f401 (build)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Apr 13, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 749 785 5000
Checkbox 1784 1692 5000
CheckboxBase 1379 1362 5000
ChoiceGroup 5323 5075 5000
ComboBox 1040 950 1000
CommandBar 6723 6997 1000
DefaultButton 1023 991 5000
DetailsRow 3165 3267 5000
DetailsRow (fast icons) 3276 3182 5000
DetailsRow without styles 2898 2921 5000
Dialog 1334 1292 1000
DocumentCardTitle with truncation 1412 1415 1000
Dropdown 2780 2763 5000
FocusZone 1459 1589 5000
IconButton 1746 1821 5000
Label 515 543 5000
Link 443 464 5000
MenuButton 1353 1392 5000
Nav 2942 2996 1000
Panel 1306 1288 1000
Persona 765 725 1000
Pivot 1348 1214 1000
PrimaryButton 1187 1227 5000
SearchBox 1422 1402 5000
Slider 1666 1720 5000
Spinner 350 333 5000
SplitButton 2876 2742 5000
Stack 417 388 5000
Stack with Intrinsic children 984 948 5000
Stack with Text children 3790 3814 5000
TagPicker 2438 2396 5000
Text 322 354 5000
TextField 1565 1599 5000
Toggle 762 764 5000
button 46 66 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.55 0.45 1.22:1 2000 1097
🦄 Button.Fluent 0.1 0.17 0.59:1 5000 482
🔧 Checkbox.Fluent 0.67 0.37 1.81:1 1000 671
🔧 Dialog.Fluent 0.42 0.19 2.21:1 5000 2081
🔧 Dropdown.Fluent 3.58 0.46 7.78:1 1000 3577
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 668
🎯 Image.Fluent 0.08 0.09 0.89:1 5000 379
🔧 Slider.Fluent 1.4 0.41 3.41:1 1000 1404
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 349
🦄 Tooltip.Fluent 0.11 11.97 0.01:1 5000 573

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ButtonMinimalPerf.default 173 149 1.16:1
Text.Fluent 349 302 1.16:1
ChatDuplicateMessagesPerf.default 454 398 1.14:1
RefMinimalPerf.default 197 175 1.13:1
AttachmentMinimalPerf.default 174 155 1.12:1
DropdownMinimalPerf.default 3997 3713 1.08:1
SegmentMinimalPerf.default 1135 1054 1.08:1
Avatar.Fluent 1097 1016 1.08:1
LabelMinimalPerf.default 451 423 1.07:1
LayoutMinimalPerf.default 728 681 1.07:1
PopupMinimalPerf.default 243 228 1.07:1
VideoMinimalPerf.default 909 849 1.07:1
AvatarMinimalPerf.default 624 589 1.06:1
DialogMinimalPerf.default 2084 1972 1.06:1
TooltipMinimalPerf.default 946 892 1.06:1
Checkbox.Fluent 671 633 1.06:1
Image.Fluent 379 358 1.06:1
FormMinimalPerf.default 1032 989 1.04:1
GridMinimalPerf.default 894 859 1.04:1
HierarchicalTreeMinimalPerf.default 1112 1066 1.04:1
ListCommonPerf.default 1126 1080 1.04:1
EmbedMinimalPerf.default 5575 5439 1.03:1
ItemLayoutMinimalPerf.default 2219 2146 1.03:1
MenuButtonMinimalPerf.default 1601 1556 1.03:1
PortalMinimalPerf.default 311 302 1.03:1
Icon.Fluent 668 647 1.03:1
AlertMinimalPerf.default 663 649 1.02:1
AttachmentSlotsPerf.default 2765 2712 1.02:1
CheckboxMinimalPerf.default 3396 3336 1.02:1
ListWith60ListItems.default 1308 1282 1.02:1
CustomToolbarPrototype.default 3481 3407 1.02:1
Dialog.Fluent 2081 2045 1.02:1
AnimationMinimalPerf.default 748 742 1.01:1
CarouselMinimalPerf.default 661 654 1.01:1
ChatWithPopoverPerf.default 623 615 1.01:1
HeaderSlotsPerf.default 1774 1753 1.01:1
SplitButtonMinimalPerf.default 4232 4196 1.01:1
TextAreaMinimalPerf.default 2950 2927 1.01:1
TreeMinimalPerf.default 1279 1272 1.01:1
ProviderMergeThemesPerf.default 1452 1449 1:1
ProviderMinimalPerf.default 591 590 1:1
StatusMinimalPerf.default 783 786 1:1
TreeWith60ListItems.default 233 233 1:1
HeaderMinimalPerf.default 572 577 0.99:1
ListNestedPerf.default 1027 1033 0.99:1
MenuMinimalPerf.default 2080 2105 0.99:1
Button.Fluent 482 486 0.99:1
Dropdown.Fluent 3577 3598 0.99:1
ButtonSlotsPerf.default 645 656 0.98:1
ChatMinimalPerf.default 650 662 0.98:1
InputMinimalPerf.default 1060 1085 0.98:1
ListMinimalPerf.default 523 535 0.98:1
RadioGroupMinimalPerf.default 589 598 0.98:1
ToolbarMinimalPerf.default 1165 1186 0.98:1
Slider.Fluent 1404 1432 0.98:1
CardMinimalPerf.default 426 437 0.97:1
ImageMinimalPerf.default 388 402 0.97:1
TableMinimalPerf.default 663 683 0.97:1
TextMinimalPerf.default 349 358 0.97:1
DropdownManyItemsPerf.default 1655 1722 0.96:1
LoaderMinimalPerf.default 1043 1084 0.96:1
AccordionMinimalPerf.default 243 256 0.95:1
SliderMinimalPerf.default 1496 1578 0.95:1
Tooltip.Fluent 573 601 0.95:1
ReactionMinimalPerf.default 2544 2715 0.94:1
BoxMinimalPerf.default 336 361 0.93:1
DividerMinimalPerf.default 1041 1136 0.92:1
FlexMinimalPerf.default 293 320 0.92:1
IconMinimalPerf.default 583 674 0.86:1

@@ -0,0 +1,13 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

Should be this example in Slots section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably, but then we should restructure all examples, as we don't even have slots examples here. I will just leave it now after the image example, and we can restructure the examples in a separate PR.

}),
})}
{!image &&
!icon &&
Label.create(label || {}, {
Copy link
Member

Choose a reason for hiding this comment

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

Not related to your changes, but this is not cool as if I will pass there null the slot will be still rendered

@mnajdova mnajdova merged commit 1ed7214 into microsoft:master Apr 14, 2020
DuanShaolong pushed a commit to DuanShaolong/fluentui that referenced this pull request Apr 27, 2020
* -added icon prop inside Avatar

* -updated changelog

* Update packages/fluentui/react-northstar/src/themes/teams/components/Avatar/avatarStyles.ts

Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* -addressed comments

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
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