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(Toolbar): convert to be functional component #13024

Merged
merged 4 commits into from
May 6, 2020
Merged

chore(Toolbar): convert to be functional component #13024

merged 4 commits into from
May 6, 2020

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented May 6, 2020 •
edited

Pull request checklist

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

BREAKING CHANGES

This PR converts Toolbar component to be functional. Also restricting props set that will be passed to styles functions.

Related to #12237.

Prop sets

Toolbar
nothing

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 May 6, 2020
layershifter
layershifter commented May 6, 2020
View reviewed changes
packages/fluentui/react-northstar/src/themes/teams/types.ts
ToolbarCustomItem: ToolbarCustomItemStylesProps;
ToolbarItem: ToolbarItemStylesProps;
ToolbarDivider: ToolbarDividerStylesProps;
ToolbarRadioGroup: ToolbarRadioGroupProps;
ToolbarRadioGroup: ToolbarRadioGroupStylesProps;
Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, was missed originally

@msft-github-bot
Copy link
Contributor

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

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 714 705 5000
Checkbox 1407 1424 5000
CheckboxBase 1146 1159 5000
ChoiceGroup 4304 4523 5000
ComboBox 814 831 1000
CommandBar 6771 6847 1000
ContextualMenu 12856 12779 1000
DefaultButton 923 918 5000
DetailsRow 3122 3063 5000
DetailsRow (fast icons) 3136 3120 5000
DetailsRow without styles 2930 2875 5000
Dialog 1326 1274 1000
DocumentCardTitle with truncation 1499 1509 1000
Dropdown 2179 2139 5000
FocusZone 1493 1478 5000
IconButton 1504 1461 5000
Label 265 267 5000
Link 391 408 5000
MenuButton 1268 1293 5000
Nav 2803 2775 1000
Panel 1304 1279 1000
Persona 759 732 1000
Pivot 1170 1201 1000
PrimaryButton 1099 1098 5000
SearchBox 1113 1127 5000
Slider 1353 1322 5000
Spinner 355 353 5000
SplitButton 2711 2741 5000
Stack 417 429 5000
Stack with Intrinsic children 989 998 5000
Stack with Text children 3608 3623 5000
TagPicker 2510 2456 5000
Text 326 333 5000
TextField 1250 1257 5000
Toggle 798 770 5000
button 64 54 5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ToolbarMinimalPerf.default 745 904 0.82:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.43 0.43 1:1 2000 851
🦄 Button.Fluent 0.09 0.16 0.56:1 5000 455
🔧 Checkbox.Fluent 0.58 0.3 1.93:1 1000 583
🔧 Dialog.Fluent 0.31 0.17 1.82:1 5000 1564
🔧 Dropdown.Fluent 3 0.39 7.69:1 1000 3003
🔧 Icon.Fluent 0.12 0.04 3:1 5000 587
🦄 Image.Fluent 0.06 0.09 0.67:1 5000 313
🔧 Slider.Fluent 1.3 0.31 4.19:1 1000 1296
🔧 Text.Fluent 0.06 0.02 3:1 5000 299
🦄 Tooltip.Fluent 0.08 15.54 0.01:1 5000 415

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 138 123 1.12:1
BoxMinimalPerf.default 299 282 1.06:1
CarouselMinimalPerf.default 433 407 1.06:1
TextMinimalPerf.default 317 300 1.06:1
AccordionMinimalPerf.default 133 127 1.05:1
ChatWithPopoverPerf.default 547 522 1.05:1
ItemLayoutMinimalPerf.default 1512 1445 1.05:1
ListCommonPerf.default 891 850 1.05:1
ListMinimalPerf.default 423 403 1.05:1
ReactionMinimalPerf.default 1771 1694 1.05:1
PortalMinimalPerf.default 274 264 1.04:1
StatusMinimalPerf.default 592 571 1.04:1
AvatarMinimalPerf.default 464 452 1.03:1
HierarchicalTreeMinimalPerf.default 867 840 1.03:1
ImageMinimalPerf.default 327 316 1.03:1
LoaderMinimalPerf.default 709 688 1.03:1
ButtonMinimalPerf.default 149 146 1.02:1
ChatMinimalPerf.default 559 548 1.02:1
CheckboxMinimalPerf.default 2734 2675 1.02:1
DropdownManyItemsPerf.default 1221 1196 1.02:1
ProviderMergeThemesPerf.default 1548 1517 1.02:1
VideoMinimalPerf.default 558 547 1.02:1
Image.Fluent 313 308 1.02:1
DialogMinimalPerf.default 1581 1570 1.01:1
DividerMinimalPerf.default 311 309 1.01:1
HeaderSlotsPerf.default 1287 1279 1.01:1
ListWith60ListItems.default 1041 1030 1.01:1
RadioGroupMinimalPerf.default 509 502 1.01:1
TableMinimalPerf.default 347 342 1.01:1
TextAreaMinimalPerf.default 385 383 1.01:1
CustomToolbarPrototype.default 3333 3295 1.01:1
Button.Fluent 455 449 1.01:1
Dialog.Fluent 1564 1541 1.01:1
Dropdown.Fluent 3003 2971 1.01:1
ChatDuplicateMessagesPerf.default 374 375 1:1
DropdownMinimalPerf.default 2987 2986 1:1
HeaderMinimalPerf.default 430 431 1:1
InputMinimalPerf.default 910 912 1:1
LayoutMinimalPerf.default 491 490 1:1
ListNestedPerf.default 817 819 1:1
ProviderMinimalPerf.default 617 620 1:1
SplitButtonMinimalPerf.default 3098 3098 1:1
Avatar.Fluent 851 855 1:1
Tooltip.Fluent 415 416 1:1
AnimationMinimalPerf.default 563 566 0.99:1
ButtonSlotsPerf.default 540 545 0.99:1
FlexMinimalPerf.default 249 251 0.99:1
GridMinimalPerf.default 586 590 0.99:1
PopupMinimalPerf.default 235 237 0.99:1
RefMinimalPerf.default 180 182 0.99:1
TreeMinimalPerf.default 1067 1074 0.99:1
AlertMinimalPerf.default 254 258 0.98:1
AttachmentSlotsPerf.default 1027 1048 0.98:1
EmbedMinimalPerf.default 1761 1805 0.98:1
FormMinimalPerf.default 632 648 0.98:1
MenuMinimalPerf.default 1615 1642 0.98:1
MenuButtonMinimalPerf.default 1368 1393 0.98:1
TreeWith60ListItems.default 204 209 0.98:1
Checkbox.Fluent 583 595 0.98:1
SegmentMinimalPerf.default 804 827 0.97:1
TooltipMinimalPerf.default 628 652 0.96:1
Icon.Fluent 587 613 0.96:1
Slider.Fluent 1296 1354 0.96:1
CardMinimalPerf.default 472 496 0.95:1
SliderMinimalPerf.default 1269 1333 0.95:1
Text.Fluent 299 317 0.94:1
LabelMinimalPerf.default 336 370 0.91:1
IconMinimalPerf.default 538 602 0.89:1

@size-auditor
Copy link

size-auditor bot commented May 6, 2020

Asset size changes

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

Baseline commit: 8d7faca2da69b5c0203041e3397bf0edf9210f48 (build)

@layershifter layershifter merged commit 8689ebb into master May 6, 2020
@layershifter layershifter deleted the chore/toolbar-fc branch May 6, 2020 14:53
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

@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

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