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(ItemLayout): Convert to RFC #13327

Merged
merged 7 commits into from
May 27, 2020
Merged

chore(ItemLayout): Convert to RFC #13327

merged 7 commits into from
May 27, 2020

Conversation

assuncaocharles
Copy link
Contributor

@assuncaocharles assuncaocharles commented May 26, 2020 •
edited

Pull request checklist

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

BREAKING CHANGES

Converting ItemLayout from class component to functional. Any props are not longer passed to styles function. Removed state parameter from renderContentArea, renderHeaderArea and renderMainArea

Related to #12237

Focus areas to test

(optional)

@msft-github-bot msft-github-bot added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label May 26, 2020
@msft-github-bot
Copy link
Contributor

msft-github-bot commented May 26, 2020 •
edited

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 830 795 5000
ButtonNext 469 455 5000
Checkbox 1587 1653 5000
CheckboxBase 1303 1311 5000
CheckboxNext 1518 1516 5000
ChoiceGroup 4985 4953 5000
ComboBox 913 937 1000
CommandBar 8080 8106 1000
ContextualMenu 16405 15787 1000
DefaultButton 1075 1071 5000
DetailsRow 3488 3603 5000
DetailsRow (fast icons) 3487 3530 5000
DetailsRow without styles 3365 3270 5000
Dialog 1495 1516 1000
DocumentCardTitle with truncation 1986 1974 1000
Dropdown 2442 2499 5000
FocusZone 1758 1810 5000
IconButton 1747 1718 5000
Label 328 324 5000
Link 471 469 5000
LinkNext 461 460 5000
MenuButton 1443 1385 5000
Nav 3202 3299 1000
Panel 1468 1504 1000
Persona 858 853 1000
Pivot 1423 1408 1000
PrimaryButton 1210 1224 5000
SearchBox 1244 1279 5000
Slider 1523 1527 5000
SliderNext 1882 1888 5000
Spinner 408 408 5000
SplitButton 3205 3100 5000
Stack 496 482 5000
Stack with Intrinsic children 1904 1835 5000
Stack with Text children 4990 4962 5000
TagPicker 2847 2788 5000
Text 407 387 5000
TextField 1424 1393 5000
Toggle 871 865 5000
ToggleNext 881 898 5000
button 62 77 5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ItemLayoutMinimalPerf.default 2330 2722 0.86:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.71 0.49 1.45:1 2000 1416
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 536
🔧 Checkbox.Fluent 1.08 0.33 3.27:1 1000 1082
🔧 Dialog.Fluent 0.56 0.22 2.55:1 5000 2805
🔧 Dropdown.Fluent 6.48 0.44 14.73:1 1000 6476
🔧 Icon.Fluent 0.14 0.05 2.8:1 5000 683
🦄 Image.Fluent 0.07 0.11 0.64:1 5000 362
🔧 Slider.Fluent 2.81 0.36 7.81:1 1000 2808
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 347
🦄 Tooltip.Fluent 0.1 21.77 0:1 5000 479

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PortalMinimalPerf.default 126 109 1.16:1
AccordionMinimalPerf.default 145 129 1.12:1
ButtonMinimalPerf.default 176 166 1.06:1
ListNestedPerf.default 1140 1077 1.06:1
AttachmentMinimalPerf.default 152 145 1.05:1
HierarchicalTreeMinimalPerf.default 415 397 1.05:1
ListWith60ListItems.default 1611 1538 1.05:1
DividerMinimalPerf.default 345 332 1.04:1
FlexMinimalPerf.default 299 287 1.04:1
ReactionMinimalPerf.default 382 367 1.04:1
IconMinimalPerf.default 660 634 1.04:1
TextAreaMinimalPerf.default 454 438 1.04:1
DropdownMinimalPerf.default 6456 6269 1.03:1
LayoutMinimalPerf.default 403 390 1.03:1
StatusMinimalPerf.default 684 661 1.03:1
ToolbarMinimalPerf.default 831 807 1.03:1
VideoMinimalPerf.default 606 590 1.03:1
CardMinimalPerf.default 556 547 1.02:1
ChatWithPopoverPerf.default 697 682 1.02:1
HeaderSlotsPerf.default 787 771 1.02:1
ListCommonPerf.default 1158 1134 1.02:1
SegmentMinimalPerf.default 330 325 1.02:1
SplitButtonMinimalPerf.default 4059 3962 1.02:1
CustomToolbarPrototype.default 4996 4888 1.02:1
Dropdown.Fluent 6476 6334 1.02:1
Image.Fluent 362 356 1.02:1
ChatMinimalPerf.default 586 579 1.01:1
ImageMinimalPerf.default 354 350 1.01:1
ListMinimalPerf.default 478 475 1.01:1
MenuMinimalPerf.default 847 842 1.01:1
MenuButtonMinimalPerf.default 1699 1684 1.01:1
RadioGroupMinimalPerf.default 405 400 1.01:1
TableMinimalPerf.default 390 388 1.01:1
AnimationMinimalPerf.default 719 716 1:1
BoxMinimalPerf.default 336 336 1:1
CheckboxMinimalPerf.default 5177 5176 1:1
DialogMinimalPerf.default 2856 2858 1:1
FormMinimalPerf.default 402 403 1:1
LabelMinimalPerf.default 396 395 1:1
SliderMinimalPerf.default 2908 2917 1:1
TooltipMinimalPerf.default 747 745 1:1
TreeMinimalPerf.default 1295 1295 1:1
AlertMinimalPerf.default 322 325 0.99:1
AttachmentSlotsPerf.default 1222 1233 0.99:1
GridMinimalPerf.default 1314 1331 0.99:1
PopupMinimalPerf.default 274 276 0.99:1
ProviderMergeThemesPerf.default 1857 1878 0.99:1
RefMinimalPerf.default 204 206 0.99:1
Avatar.Fluent 1416 1424 0.99:1
Dialog.Fluent 2805 2829 0.99:1
Text.Fluent 347 351 0.99:1
ButtonSlotsPerf.default 730 747 0.98:1
CarouselMinimalPerf.default 533 544 0.98:1
ChatDuplicateMessagesPerf.default 545 557 0.98:1
EmbedMinimalPerf.default 3373 3425 0.98:1
InputMinimalPerf.default 1603 1630 0.98:1
TextMinimalPerf.default 337 343 0.98:1
Checkbox.Fluent 1082 1104 0.98:1
AvatarMinimalPerf.default 738 762 0.97:1
DropdownManyItemsPerf.default 2136 2198 0.97:1
Button.Fluent 536 550 0.97:1
Icon.Fluent 683 702 0.97:1
Tooltip.Fluent 479 496 0.97:1
Slider.Fluent 2808 2934 0.96:1
LoaderMinimalPerf.default 1123 1185 0.95:1
ProviderMinimalPerf.default 837 889 0.94:1
TreeWith60ListItems.default 267 283 0.94:1
HeaderMinimalPerf.default 327 366 0.89:1

@size-auditor
Copy link

size-auditor bot commented May 26, 2020 •
edited

Asset size changes

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

Baseline commit: a562eba88ce9e1029d0aaeda9cc0e5d59a7a91df (build)

@layershifter
Copy link
Member

Any props are not longer passed to styles function.

It's not true, at least debugLayout.

@assuncaocharles
Copy link
Contributor Author

Any props are not longer passed to styles function.

It's not true, at least debugLayout.

But it doesn't come from props. For what I understand isn't for development purposes?

@layershifter
Copy link
Member

Any props are not longer passed to styles function.

It's not true, at least debugLayout.

But it doesn't come from props. For what I understand isn't for development purposes?

OMG, I see 🦀

@assuncaocharles
Copy link
Contributor Author

Any props are not longer passed to styles function.

It's not true, at least debugLayout.

But it doesn't come from props. For what I understand isn't for development purposes?

OMG, I see 🦀

I can even remove it at all if you think it would be better.

@layershifter
Copy link
Member

I can even remove it at all if you think it would be better.

If it's impossible to use, let's just remove these styles and props 👍

assuncaocharles and others added 2 commits May 27, 2020 16:40
@assuncaocharles @layershifter
Update packages/fluentui/react-northstar/src/components/ItemLayout/It…
3987a18 
…emLayout.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
@assuncaocharles
chore(ItemLayout): remove debug styles
dca3d43
@assuncaocharles assuncaocharles merged commit ff77a1a into microsoft:master May 27, 2020
@assuncaocharles assuncaocharles deleted the chore/item-layout-rfc branch May 27, 2020 15:55
miroslavstastny pushed a commit to levithomason/fluentui that referenced this pull request Jun 8, 2020
@assuncaocharles @layershifter @miroslavstastny
chore(ItemLayout): Convert to RFC (microsoft#13327)
c732cf9 
* chore(ItemLayout): Convert to RFC

* chore(ItemLayout): Add changelog

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update packages/fluentui/react-northstar/src/components/ItemLayout/ItemLayout.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* chore(ItemLayout): remove debug styles

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
Reviewers

@layershifter layershifter layershifter approved these changes

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@dzearing dzearing Awaiting requested review from dzearing

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@levithomason levithomason Awaiting requested review from levithomason

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

@mnajdova mnajdova Awaiting requested review from mnajdova

@pompomon pompomon Awaiting requested review from pompomon

Assignees

@ecraig12345 ecraig12345

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
4 participants
@assuncaocharles @msft-github-bot @layershifter @ecraig12345