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

Update icons to v3.70 #12088

Closed
wants to merge 6 commits into from
Closed

Conversation

Jahnp
Copy link
Member

@Jahnp Jahnp commented Feb 26, 2020

Pull request checklist

Description of changes

Updating @uifabric/icon's icon set to the latest version from the Fabric icons tool, v3.70. This PR adds new icons from that version to the "chunked" subsets that get automatically loaded on usage. There may also be visual updates to some icons.

Apologies - I know this has a lot of changed files, but it can't really be batched. The core code/asset changes are under packages/icons.

Microsoft Reviewers: Open in CodeFlow

@Jahnp Jahnp changed the title Jahnp/update icons v3.70 Update icons to v3.70 Feb 26, 2020
@size-auditor
Copy link

size-auditor bot commented Feb 26, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react Icons 70.702 kB 73.62 kB ExceedsTolerance     2.918 kB

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: c7c4b6098eea407a37897279d953a42a11ccea86 (build)

@Jahnp Jahnp mentioned this pull request Feb 26, 2020
2 tasks
@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 685 672
BaseButton (experiments) 892 893
DefaultButton 920 951
DefaultButton (experiments) 1821 1804
DetailsRow 3328 3203
DetailsRow (fast icons) 3310 3255
DetailsRow without styles 3093 3055
DocumentCardTitle with truncation 1436 1421
MenuButton 1356 1365
MenuButton (experiments) 3646 3424
PrimaryButton 1140 1210
PrimaryButton (experiments) 1844 1853
SplitButton 2707 2767
SplitButton (experiments) 6657 6668
Stack 437 444
Stack with Intrinsic children 1052 1056
Stack with Text children 4054 4111
Text 381 367
Toggle 799 804
Toggle (experiments) 2164 2131
button 51 66

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.58 0.49 1.18:1 2000 1153
🦄 Button.Fluent 0.14 0.21 0.67:1 1000 140
🔧 Checkbox.Fluent 0.85 0.41 2.07:1 1000 846
🔧 Dialog.Fluent 0.39 0.22 1.77:1 5000 1956
🔧 Dropdown.Fluent 3.59 0.5 7.18:1 1000 3588
🔧 Icon.Fluent 0.18 0.05 3.6:1 5000 882
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 367
🔧 Slider.Fluent 1.44 0.41 3.51:1 1000 1442
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 354
🦄 Tooltip.Fluent 0.12 12.57 0.01:1 5000 621

🔧 Needs work     🎯 On target     🦄 Amazing

All perf tests
Scenario Current PR Ticks Baseline Ticks Ratio
Image.Fluent 367 315 1.17:1
RefMinimalPerf.default 208 179 1.16:1
ListWith60ListItems.default 189 168 1.13:1
TextMinimalPerf.default 391 347 1.13:1
LabelMinimalPerf.default 1209 1091 1.11:1
DropdownManyItemsPerf.default 379 343 1.1:1
AnimationMinimalPerf.default 700 643 1.09:1
Dialog.Fluent 1956 1805 1.08:1
HierarchicalTreeMinimalPerf.default 1128 1057 1.07:1
IconMinimalPerf.default 401 375 1.07:1
TreeWith60ListItems.default 276 257 1.07:1
ButtonMinimalPerf.default 157 148 1.06:1
DropdownMinimalPerf.default 3774 3545 1.06:1
FlexMinimalPerf.default 523 494 1.06:1
PortalMinimalPerf.default 297 281 1.06:1
ProviderMinimalPerf.default 598 562 1.06:1
AvatarMinimalPerf.default 640 609 1.05:1
GridMinimalPerf.default 925 882 1.05:1
ListMinimalPerf.default 437 416 1.05:1
ChatMinimalPerf.default 589 564 1.04:1
LoaderMinimalPerf.default 1017 978 1.04:1
Icon.Fluent 882 852 1.04:1
BoxMinimalPerf.default 326 317 1.03:1
ChatDuplicateMessagesPerf.default 407 396 1.03:1
DividerMinimalPerf.default 1068 1036 1.03:1
FormMinimalPerf.default 979 951 1.03:1
PopupMinimalPerf.default 415 401 1.03:1
ToolbarMinimalPerf.default 1222 1191 1.03:1
AttachmentSlotsPerf.default 3486 3432 1.02:1
LayoutMinimalPerf.default 697 681 1.02:1
ProviderMergeThemesPerf.default 1238 1209 1.02:1
RadioGroupMinimalPerf.default 573 562 1.02:1
TooltipMinimalPerf.default 903 884 1.02:1
Avatar.Fluent 1153 1132 1.02:1
Tooltip.Fluent 621 607 1.02:1
AttachmentMinimalPerf.default 910 898 1.01:1
ImageMinimalPerf.default 327 323 1.01:1
ListCommonPerf.default 998 993 1.01:1
Checkbox.Fluent 846 838 1.01:1
Dropdown.Fluent 3588 3542 1.01:1
ChatWithPopoverPerf.default 570 571 1:1
EmbedMinimalPerf.default 6101 6074 1:1
ListNestedPerf.default 905 901 1:1
MenuMinimalPerf.default 2059 2069 1:1
MenuButtonMinimalPerf.default 1953 1956 1:1
SegmentMinimalPerf.default 1156 1151 1:1
SplitButtonMinimalPerf.default 12408 12399 1:1
TableMinimalPerf.default 716 714 1:1
TextAreaMinimalPerf.default 3178 3183 1:1
TreeMinimalPerf.default 1236 1234 1:1
ButtonSlotsPerf.default 723 730 0.99:1
CarouselMinimalPerf.default 1973 2000 0.99:1
HeaderMinimalPerf.default 596 602 0.99:1
InputMinimalPerf.default 1000 1013 0.99:1
ItemLayoutMinimalPerf.default 2212 2231 0.99:1
SliderMinimalPerf.default 1451 1470 0.99:1
StatusMinimalPerf.default 323 325 0.99:1
VideoMinimalPerf.default 931 940 0.99:1
CheckboxMinimalPerf.default 3940 4006 0.98:1
DialogMinimalPerf.default 1824 1865 0.98:1
Button.Fluent 140 143 0.98:1
Slider.Fluent 1442 1478 0.98:1
Text.Fluent 354 361 0.98:1
HeaderSlotsPerf.default 1736 1798 0.97:1
CustomToolbarPrototype.default 3599 3700 0.97:1
ReactionMinimalPerf.default 2393 2528 0.95:1
AlertMinimalPerf.default 568 604 0.94:1
AccordionMinimalPerf.default 238 259 0.92:1

@ecraig12345
Copy link
Member

Size auditor is failing -- @KevinTCoughlin do the icon additions actually have any impact on bundle size for you, or do you use custom subsets?

@KevinTCoughlin
Copy link
Member

KevinTCoughlin commented Mar 3, 2020

Size auditor is failing -- @KevinTCoughlin do the icon additions actually have any impact on bundle size for you, or do you use custom subsets?

Apologies for the delay @ecraig12345. First-party SharePoint code utilizes https://www.npmjs.com/package/@microsoft/office-ui-fabric-react-bundle which does the following:

export * from 'office-ui-fabric-react/lib/Icon';

The majority of third-party developers currently bundle office-ui-fabric-react themselves.

I anticipate the 2.918 kB size increase here to impact first-party SharePoint. That said, I don't have a great solution to mitigate the icon bloat soonest. It is unfortunate that this icon mapping continues to grow linearly over time.

Regarding run-time cost, at least it is a "simple" structure to parse. Linear increase in memory is a greater concern. See the below memory dump from SPO containing all Fabric Icon strings.

image

To address, SharePoint would have to export and utilize an icon sub-set as you mention. @Jahnp perhaps you can consider this improvement if you anticipate additional revisions to this package.

cc: @patmill @johnguy0 @antonlabunets

@dzearing
Copy link
Member

dzearing commented Apr 6, 2020

@Jahnp Is this still needed? It seems there are conflicts, looks like the url may have changed?

@Jahnp
Copy link
Member Author

Jahnp commented Apr 29, 2020

Sorry for letting this linger. This PR is very stale at this point and many new icons have been made available since this was opened. I'm going to go ahead and abandon this for now.

Additionally, @KevinTCoughlin brings up some excellent points about the downstream impact of the constantly-growing icons package's bundle size on users of SPFx and similar solutions. I agree with the concerns and think it's probably best to avoid continued revisions like this that simply add new icons. Within SPFx, it would likely be best to investigate a custom icon subset.

Further, I'm inclined to stop updating the default icon set in this manner and instead recommend that users looking for new icons should consider custom icon subsets with the Fabric icons tool instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing shape icons Please update the project log to the new version
7 participants