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: move TreeItem & TreeTitle to functional components #12103

Merged
merged 6 commits into from
Mar 2, 2020
Merged

chore: move TreeItem & TreeTitle to functional components #12103

merged 6 commits into from
Mar 2, 2020

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Feb 27, 2020 •
edited

Pull request checklist

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

Description of changes

BREAKING CHANGES

This PR converts TreeTitle & TreeItem components to be functional. As in all other cases it contains a breaking change with restricted props set that will be passed to styles functions.

Prop sets

TreeTitle
no props
TreeItem
level
Microsoft Reviewers: Open in CodeFlow

@layershifter layershifter added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label Feb 27, 2020
@size-auditor
Copy link

size-auditor bot commented Feb 27, 2020 •
edited

Asset size changes

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

Baseline commit: b83ef7276960d459359f2f8630d38d924744a2e0 (build)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Feb 27, 2020 •
edited

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 708 657
BaseButton (experiments) 890 855
DefaultButton 925 939
DefaultButton (experiments) 1690 1779
DetailsRow 3096 3044
DetailsRow (fast icons) 3142 3102
DetailsRow without styles 2847 2806
DocumentCardTitle with truncation 1489 1517
MenuButton 1247 1229
MenuButton (experiments) 3251 3228
PrimaryButton 1095 1085
PrimaryButton (experiments) 1829 1790
SplitButton 2663 2588
SplitButton (experiments) 6267 6258
Stack 408 413
Stack with Intrinsic children 1011 1054
Stack with Text children 3717 3647
Text 346 352
Toggle 726 778
Toggle (experiments) 2039 2095
button 64 61

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
TreeWith60ListItems.default 223 260 0.86:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.52 0.47 1.11:1 2000 1037
🎯 Button.Fluent 0.14 0.2 0.7:1 1000 142
🔧 Checkbox.Fluent 0.91 0.37 2.46:1 1000 907
🔧 Dialog.Fluent 0.35 0.21 1.67:1 5000 1765
🔧 Dropdown.Fluent 3.7 0.47 7.87:1 1000 3701
🔧 Icon.Fluent 0.15 0.04 3.75:1 5000 733
🦄 Image.Fluent 0.06 0.1 0.6:1 5000 286
🔧 Slider.Fluent 1.46 0.37 3.95:1 1000 1457
🔧 Text.Fluent 0.06 0.02 3:1 5000 310
🦄 Tooltip.Fluent 0.11 16.41 0.01:1 5000 559

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
LabelMinimalPerf.default 342 314 1.09:1
RefMinimalPerf.default 197 180 1.09:1
TextMinimalPerf.default 334 307 1.09:1
ButtonSlotsPerf.default 737 687 1.07:1
IconMinimalPerf.default 378 355 1.06:1
Avatar.Fluent 1037 979 1.06:1
Image.Fluent 286 271 1.06:1
PortalMinimalPerf.default 280 267 1.05:1
StatusMinimalPerf.default 293 280 1.05:1
ProviderMinimalPerf.default 633 607 1.04:1
TooltipMinimalPerf.default 827 796 1.04:1
Icon.Fluent 733 707 1.04:1
DialogMinimalPerf.default 1780 1722 1.03:1
HeaderMinimalPerf.default 534 520 1.03:1
HeaderSlotsPerf.default 1627 1583 1.03:1
RadioGroupMinimalPerf.default 495 479 1.03:1
Checkbox.Fluent 907 883 1.03:1
AvatarMinimalPerf.default 546 534 1.02:1
FormMinimalPerf.default 927 909 1.02:1
InputMinimalPerf.default 1074 1058 1.02:1
ListMinimalPerf.default 394 386 1.02:1
SplitButtonMinimalPerf.default 12721 12492 1.02:1
TextAreaMinimalPerf.default 3255 3203 1.02:1
ToolbarMinimalPerf.default 1025 1006 1.02:1
Slider.Fluent 1457 1426 1.02:1
AnimationMinimalPerf.default 601 596 1.01:1
CarouselMinimalPerf.default 2068 2048 1.01:1
ChatMinimalPerf.default 533 529 1.01:1
EmbedMinimalPerf.default 6417 6380 1.01:1
HierarchicalTreeMinimalPerf.default 1007 999 1.01:1
ImageMinimalPerf.default 300 296 1.01:1
ListCommonPerf.default 946 934 1.01:1
ListNestedPerf.default 889 880 1.01:1
MenuMinimalPerf.default 1981 1970 1.01:1
MenuButtonMinimalPerf.default 1818 1801 1.01:1
TableMinimalPerf.default 647 643 1.01:1
VideoMinimalPerf.default 860 849 1.01:1
Button.Fluent 142 140 1.01:1
Dialog.Fluent 1765 1741 1.01:1
AccordionMinimalPerf.default 238 239 1:1
AttachmentSlotsPerf.default 3573 3577 1:1
DropdownMinimalPerf.default 3721 3708 1:1
ListWith60ListItems.default 175 175 1:1
ReactionMinimalPerf.default 2400 2400 1:1
Dropdown.Fluent 3701 3713 1:1
AlertMinimalPerf.default 602 611 0.99:1
AttachmentMinimalPerf.default 969 983 0.99:1
BoxMinimalPerf.default 291 294 0.99:1
ChatDuplicateMessagesPerf.default 410 415 0.99:1
ChatWithPopoverPerf.default 590 595 0.99:1
CheckboxMinimalPerf.default 4008 4044 0.99:1
ItemLayoutMinimalPerf.default 2009 2026 0.99:1
LoaderMinimalPerf.default 1049 1057 0.99:1
ProviderMergeThemesPerf.default 1295 1302 0.99:1
SegmentMinimalPerf.default 1057 1063 0.99:1
CustomToolbarPrototype.default 3855 3909 0.99:1
Text.Fluent 310 313 0.99:1
DropdownManyItemsPerf.default 351 360 0.98:1
LayoutMinimalPerf.default 651 664 0.98:1
SliderMinimalPerf.default 1496 1525 0.98:1
GridMinimalPerf.default 828 854 0.97:1
ButtonMinimalPerf.default 136 142 0.96:1
TreeMinimalPerf.default 1088 1134 0.96:1
PopupMinimalPerf.default 380 399 0.95:1
Tooltip.Fluent 559 589 0.95:1
DividerMinimalPerf.default 940 999 0.94:1
FlexMinimalPerf.default 239 268 0.89:1

mnajdova
mnajdova approved these changes Mar 2, 2020
View reviewed changes
Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

LGTM would recommend someone from the accessibility team to review it for regressions

@layershifter layershifter changed the title chore: move TreeItem & TreeTitle to functional components [WIP] chore: move TreeItem & TreeTitle to functional components Mar 2, 2020
@layershifter
Merge branches 'chore/move-tree-to-fc' and 'master' of https://github…
884f48e 
….com/OfficeDev/office-ui-fabric-react into chore/move-tree-to-fc

� Conflicts:
�	packages/fluentui/CHANGELOG.md
@layershifter layershifter merged commit 47c13f2 into master Mar 2, 2020
@layershifter layershifter deleted the chore/move-tree-to-fc branch March 2, 2020 16:03
@DustyTheBot DustyTheBot mentioned this pull request Mar 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnajdova mnajdova mnajdova approved these changes

@silviuaavram silviuaavram silviuaavram 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

@kenotron kenotron Awaiting requested review from kenotron

@levithomason levithomason Awaiting requested review from levithomason

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

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
@layershifter @msft-github-bot @mnajdova @silviuaavram @aneeshack4