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.

Sign up for GitHub

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

Jump to bottom

chore(Embed|RadioGroupItem|CarouselNavigationItem): remove hard icon dependencies #12242

Merged
merged 7 commits into from
Mar 10, 2020
Merged

chore(Embed|RadioGroupItem|CarouselNavigationItem): remove hard icon dependencies #12242

merged 7 commits into from
Mar 10, 2020

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Mar 9, 2020 •
edited by msft-github-bot

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

@msft-github-bot msft-github-bot added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label Mar 9, 2020
@size-auditor
Copy link

size-auditor bot commented Mar 9, 2020 •
edited

Asset size changes

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

Baseline commit: 2409073af2cf7179238a6d7c9caf563ddffb048d (build)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Mar 9, 2020 •
edited

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 661 673
BaseButton (experiments) 851 840
DefaultButton 901 935
DefaultButton (experiments) 1772 1735
DetailsRow 3079 3116
DetailsRow (fast icons) 3109 3187
DetailsRow without styles 2885 2960
DocumentCardTitle with truncation 1553 1538
MenuButton 1252 1238
MenuButton (experiments) 3332 3237
PrimaryButton 1085 1090
PrimaryButton (experiments) 1844 1800
SplitButton 2589 2598
SplitButton (experiments) 6525 6464
Stack 420 412
Stack with Intrinsic children 1001 1023
Stack with Text children 3665 3703
Text 355 322
Toggle 753 740
Toggle (experiments) 2083 2042
button 66 61

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
EmbedMinimalPerf.default 5576 6319 0.88:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.53 0.47 1.13:1 2000 1069
🎯 Button.Fluent 0.16 0.22 0.73:1 1000 156
🔧 Checkbox.Fluent 0.73 0.37 1.97:1 1000 734
🔧 Dialog.Fluent 0.36 0.21 1.71:1 5000 1797
🔧 Dropdown.Fluent 3.57 0.49 7.29:1 1000 3566
🔧 Icon.Fluent 0.16 0.04 4:1 5000 818
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 333
🔧 Slider.Fluent 1.59 0.39 4.08:1 1000 1591
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 351
🦄 Tooltip.Fluent 0.12 15.97 0.01:1 5000 600

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
Button.Fluent 156 137 1.14:1
PortalMinimalPerf.default 287 264 1.09:1
RefMinimalPerf.default 211 193 1.09:1
StatusMinimalPerf.default 335 307 1.09:1
Image.Fluent 333 315 1.06:1
FormMinimalPerf.default 922 876 1.05:1
HeaderMinimalPerf.default 548 522 1.05:1
ListWith60ListItems.default 183 174 1.05:1
SliderMinimalPerf.default 1590 1515 1.05:1
AttachmentMinimalPerf.default 972 935 1.04:1
ButtonSlotsPerf.default 769 739 1.04:1
DropdownManyItemsPerf.default 351 339 1.04:1
TreeMinimalPerf.default 1252 1203 1.04:1
DividerMinimalPerf.default 963 936 1.03:1
DropdownMinimalPerf.default 3565 3458 1.03:1
GridMinimalPerf.default 850 828 1.03:1
HeaderSlotsPerf.default 1656 1600 1.03:1
ItemLayoutMinimalPerf.default 2113 2044 1.03:1
TableMinimalPerf.default 656 637 1.03:1
CarouselMinimalPerf.default 2100 2062 1.02:1
ChatWithPopoverPerf.default 611 601 1.02:1
FlexMinimalPerf.default 253 249 1.02:1
IconMinimalPerf.default 396 390 1.02:1
LayoutMinimalPerf.default 642 629 1.02:1
ListCommonPerf.default 994 978 1.02:1
LoaderMinimalPerf.default 1093 1072 1.02:1
BoxMinimalPerf.default 340 336 1.01:1
HierarchicalTreeMinimalPerf.default 1002 991 1.01:1
InputMinimalPerf.default 1078 1066 1.01:1
ListMinimalPerf.default 423 419 1.01:1
ListNestedPerf.default 934 923 1.01:1
ProviderMinimalPerf.default 642 635 1.01:1
SegmentMinimalPerf.default 1084 1077 1.01:1
CustomToolbarPrototype.default 3931 3900 1.01:1
AccordionMinimalPerf.default 232 233 1:1
AnimationMinimalPerf.default 617 614 1:1
ChatDuplicateMessagesPerf.default 445 443 1:1
MenuMinimalPerf.default 2026 2036 1:1
MenuButtonMinimalPerf.default 1826 1834 1:1
ProviderMergeThemesPerf.default 1321 1324 1:1
RadioGroupMinimalPerf.default 516 516 1:1
ReactionMinimalPerf.default 2533 2534 1:1
TextAreaMinimalPerf.default 3164 3176 1:1
Avatar.Fluent 1069 1066 1:1
Checkbox.Fluent 734 734 1:1
Dialog.Fluent 1797 1804 1:1
Dropdown.Fluent 3566 3582 1:1
Text.Fluent 351 351 1:1
Tooltip.Fluent 600 603 1:1
AttachmentSlotsPerf.default 3620 3646 0.99:1
LabelMinimalPerf.default 348 350 0.99:1
PopupMinimalPerf.default 397 401 0.99:1
SplitButtonMinimalPerf.default 12721 12786 0.99:1
TextMinimalPerf.default 353 355 0.99:1
TooltipMinimalPerf.default 883 894 0.99:1
TreeWith60ListItems.default 236 238 0.99:1
VideoMinimalPerf.default 879 892 0.99:1
AvatarMinimalPerf.default 547 560 0.98:1
ImageMinimalPerf.default 312 318 0.98:1
Icon.Fluent 818 831 0.98:1
Slider.Fluent 1591 1622 0.98:1
AlertMinimalPerf.default 584 604 0.97:1
ChatMinimalPerf.default 558 575 0.97:1
CheckboxMinimalPerf.default 3259 3356 0.97:1
ButtonMinimalPerf.default 140 146 0.96:1
DialogMinimalPerf.default 1726 1798 0.96:1
ToolbarMinimalPerf.default 1062 1101 0.96:1

@mnajdova mnajdova changed the title [WIP] chore(Embed|RadioGroupItem): remove hard icon dependencies [WIP] chore(Embed|RadioGroupItem|CarouselNavigationItem): remove hard icon dependencies Mar 9, 2020
@mnajdova mnajdova changed the title [WIP] chore(Embed|RadioGroupItem|CarouselNavigationItem): remove hard icon dependencies chore(Embed|RadioGroupItem|CarouselNavigationItem): remove hard icon dependencies Mar 10, 2020
@mnajdova @layershifter
Update packages/fluentui/react/src/components/RadioGroup/RadioGroupIt…
044f7b0 
…em.tsx

Co-Authored-By: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
layershifter
layershifter approved these changes Mar 10, 2020
View reviewed changes
Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

👍

@layershifter
Copy link
Member

Please include CHANGELOG before merging

@mnajdova mnajdova merged commit d510e06 into microsoft:master Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@silviuaavram silviuaavram silviuaavram approved these changes

@layershifter layershifter layershifter approved these changes

@dzearing dzearing Awaiting requested review from dzearing

@ecraig12345 ecraig12345 Awaiting requested review from ecraig12345

@JasonGore JasonGore Awaiting requested review from JasonGore

@jdhuntington jdhuntington Awaiting requested review from jdhuntington

@levithomason levithomason Awaiting requested review from levithomason

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

@pompomon pompomon Awaiting requested review from pompomon

Assignees

@aneeshack4 aneeshack4

Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mnajdova @msft-github-bot @layershifter @silviuaavram @aneeshack4