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

feat(ToolbarMenuRadioGroup|ToolbarRadioGroup): use compose() #13322

Merged
merged 12 commits into from
May 27, 2020
Merged

feat(ToolbarMenuRadioGroup|ToolbarRadioGroup): use compose() #13322

merged 12 commits into from
May 27, 2020

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented May 26, 2020 •
edited

Pull request checklist

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

BREAKING CHANGES

wrapper slot in ToolbarMenuRadioGroup now is ToolbarMenuRadioGroupWrapper instead of Box.

  1. toolbarMenuRadioGroupSlotClassNames and interface ToolbarMenuRadioGroupSlotClassNames were removed, please use toolbarMenuRadioGroupWrapperClassName.
// Before
console.log(toolbarMenuRadioGroupSlotClassNames.wrapper)

// After
console.log(toolbarMenuRadioGroupClassName)
  1. Styles for wrapper slot in ToolbarMenuRadioGroup are moved to ToolbarMenuRadioGroupWrapper, please move matching styles to toolbarMenuRadioGroupWrapperStyles.
// Before
const toolbarMenuRadioGroupStyles = {
  wrapper: (): ICSSInJSStyle => ({
    // styles
  }),
};

// After
const toolbarMenuRadioGroupWrapperStyles = {
  root: (): ICSSInJSStyle => ({
    // styles
  }),
};

Description of changes

This PR:

  • add compose() usage to ToolbarMenuRadioGroup & ToolbarRadioGroup
  • introduces ToolbarMenuRadioGroupWrapper components and moves styles there

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 793 799 5000
Checkbox 1486 1532 5000
CheckboxBase 1286 1235 5000
CheckboxNext 1555 1514 5000
ChoiceGroup 4859 5053 5000
ComboBox 922 935 1000
CommandBar 7478 7614 1000
ContextualMenu 13453 13056 1000
DefaultButton 1060 1011 5000
DetailsRow 3367 3380 5000
DetailsRow (fast icons) 3539 3553 5000
DetailsRow without styles 3394 3259 5000
Dialog 1472 1471 1000
DocumentCardTitle with truncation 1881 1889 1000
Dropdown 2399 2344 5000
FocusZone 1745 1720 5000
IconButton 1696 1679 5000
Label 315 311 5000
Link 455 449 5000
LinkNext 453 447 5000
MenuButton 1389 1419 5000
Nav 3170 3197 1000
Panel 1462 1363 1000
Persona 824 810 1000
Pivot 1346 1351 1000
PrimaryButton 1153 1180 5000
SearchBox 1182 1212 5000
Slider 1500 1463 5000
SliderNext 1447 1449 5000
Spinner 402 384 5000
SplitButton 2991 2973 5000
Stack 493 465 5000
Stack with Intrinsic children 1881 1817 5000
Stack with Text children 4854 4906 5000
TagPicker 2699 2679 5000
Text 380 375 5000
TextField 1354 1348 5000
Toggle 853 843 5000
ToggleNext 850 849 5000
button 88 73 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.69 0.48 1.44:1 2000 1377
🦄 Button.Fluent 0.1 0.19 0.53:1 5000 515
🔧 Checkbox.Fluent 1.04 0.37 2.81:1 1000 1043
🔧 Dialog.Fluent 0.54 0.21 2.57:1 5000 2696
🔧 Dropdown.Fluent 6.33 0.44 14.39:1 1000 6329
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 671
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 352
🔧 Slider.Fluent 2.78 0.36 7.72:1 1000 2784
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 331
🦄 Tooltip.Fluent 0.1 17.45 0.01:1 5000 479

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AlertMinimalPerf.default 331 287 1.15:1
FlexMinimalPerf.default 295 263 1.12:1
ButtonMinimalPerf.default 175 162 1.08:1
DividerMinimalPerf.default 335 310 1.08:1
IconMinimalPerf.default 656 605 1.08:1
AttachmentMinimalPerf.default 147 137 1.07:1
HeaderMinimalPerf.default 348 324 1.07:1
AttachmentSlotsPerf.default 1244 1178 1.06:1
FormMinimalPerf.default 363 342 1.06:1
TextMinimalPerf.default 334 314 1.06:1
ToolbarMinimalPerf.default 758 718 1.06:1
SplitButtonMinimalPerf.default 3920 3745 1.05:1
TooltipMinimalPerf.default 746 713 1.05:1
TreeMinimalPerf.default 1274 1215 1.05:1
GridMinimalPerf.default 1261 1213 1.04:1
LayoutMinimalPerf.default 814 785 1.04:1
ListWith60ListItems.default 1552 1487 1.04:1
PopupMinimalPerf.default 264 254 1.04:1
Dropdown.Fluent 6329 6093 1.04:1
ChatDuplicateMessagesPerf.default 527 512 1.03:1
CheckboxMinimalPerf.default 5153 5010 1.03:1
ProviderMergeThemesPerf.default 1797 1742 1.03:1
RadioGroupMinimalPerf.default 562 547 1.03:1
VideoMinimalPerf.default 583 565 1.03:1
Tooltip.Fluent 479 466 1.03:1
CarouselMinimalPerf.default 505 493 1.02:1
MenuMinimalPerf.default 816 803 1.02:1
RefMinimalPerf.default 197 194 1.02:1
CustomToolbarPrototype.default 4678 4594 1.02:1
AvatarMinimalPerf.default 719 712 1.01:1
ButtonSlotsPerf.default 719 710 1.01:1
HeaderSlotsPerf.default 736 731 1.01:1
InputMinimalPerf.default 1559 1540 1.01:1
ItemLayoutMinimalPerf.default 2588 2565 1.01:1
ProviderMinimalPerf.default 811 803 1.01:1
SliderMinimalPerf.default 2886 2849 1.01:1
Avatar.Fluent 1377 1368 1.01:1
CardMinimalPerf.default 527 527 1:1
DropdownMinimalPerf.default 6324 6306 1:1
HierarchicalTreeMinimalPerf.default 388 389 1:1
ImageMinimalPerf.default 344 344 1:1
ListMinimalPerf.default 442 444 1:1
MenuButtonMinimalPerf.default 1634 1636 1:1
PortalMinimalPerf.default 112 112 1:1
StatusMinimalPerf.default 628 625 1:1
AccordionMinimalPerf.default 128 129 0.99:1
EmbedMinimalPerf.default 3288 3328 0.99:1
ListNestedPerf.default 1040 1054 0.99:1
ReactionMinimalPerf.default 357 362 0.99:1
TableMinimalPerf.default 352 354 0.99:1
TreeWith60ListItems.default 265 268 0.99:1
Checkbox.Fluent 1043 1055 0.99:1
Icon.Fluent 671 680 0.99:1
Slider.Fluent 2784 2814 0.99:1
Text.Fluent 331 335 0.99:1
AnimationMinimalPerf.default 693 707 0.98:1
DialogMinimalPerf.default 2756 2807 0.98:1
DropdownManyItemsPerf.default 2033 2085 0.98:1
ListCommonPerf.default 1108 1130 0.98:1
TextAreaMinimalPerf.default 423 430 0.98:1
Button.Fluent 515 523 0.98:1
ChatMinimalPerf.default 547 562 0.97:1
SegmentMinimalPerf.default 311 322 0.97:1
BoxMinimalPerf.default 319 331 0.96:1
LabelMinimalPerf.default 380 394 0.96:1
LoaderMinimalPerf.default 1066 1108 0.96:1
Dialog.Fluent 2696 2804 0.96:1
ChatWithPopoverPerf.default 637 668 0.95:1
Image.Fluent 352 375 0.94:1

@size-auditor
Copy link

size-auditor bot commented May 26, 2020

Asset size changes

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

Baseline commit: ef1e33d47db0b55d83ee68fa58bd0ec8ed39f0a4 (build)

@layershifter layershifter changed the title chore(ToolbarMenuRadioGroup|ToolbarRadioGroup): use compose() feat(ToolbarMenuRadioGroup|ToolbarRadioGroup): use compose() May 26, 2020
@layershifter layershifter merged commit 9498d77 into master May 27, 2020
@layershifter layershifter deleted the chore/toolbar-radio-groups branch May 27, 2020 08:04
miroslavstastny pushed a commit to levithomason/fluentui that referenced this pull request Jun 8, 2020
@layershifter @miroslavstastny
feat(ToolbarMenuRadioGroup|ToolbarRadioGroup): use compose() (microso…
86bb7e1 
…ft#13322)

* chore(ToolbarMenuRadioGroup|ToolbarRadioGroup): use compose()

* add changelog entries

* add and fix UTs

* fix exports

* fix export name

* fix changelog

* add export

* fix overrideStyles

* use slots

* fix broken UTs

* other fixes
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