Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

chore(DropdownItem): refactor to functional component with hooks #2382

Merged
merged 6 commits into from
Feb 25, 2020

Conversation

silviuaavram
Copy link
Collaborator

@silviuaavram silviuaavram commented Feb 24, 2020

Inspired heavily by ListItem. Removed the use of ListItem and created the slots similarly to how ListItem does it. Also added some listItemStyles to dropdownItemStyles in order to compensate from not using ListItem anymore.

BREAKING CHANGES

Only limited amount of props are passed to component's styles functions.

DropdownItem
active
isFromKeyboard
selected
hasContent
hasHeader

@DustyTheBot
Copy link
Collaborator

DustyTheBot commented Feb 24, 2020

Warnings
⚠️ 1 perf regressions detected

Perf comparison

Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.46 0.39 1.18:1 2000 918
🦄 Button.Fluent 0.11 0.17 0.65:1 1000 110
🔧 Checkbox.Fluent 0.75 0.3 2.5:1 1000 749
🔧 Dialog.Fluent 0.29 0.16 1.81:1 5000 1429
🔧 Dropdown.Fluent 3.15 0.37 8.51:1 1000 3145
🔧 Icon.Fluent 0.12 0.03 4:1 5000 609
🎯 Image.Fluent 0.05 0.07 0.71:1 5000 231
🔧 Slider.Fluent 1.33 0.32 4.16:1 1000 1330
🔧 Text.Fluent 0.05 0.02 2.5:1 5000 259
🦄 Tooltip.Fluent 0.09 17.22 0.01:1 5000 431

🔧 Needs work     🎯 On target     🦄 Amazing

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio
DropdownManyItemsPerf.default 270 384 0.7:1
Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PopupMinimalPerf.default 443 319 1.39:1
RefMinimalPerf.default 196 151 1.3:1
ToolbarMinimalPerf.default 1029 827 1.24:1
SplitButtonMinimalPerf.default 12207 10649 1.15:1
ProviderMergeThemesPerf.default 1247 1094 1.14:1
ListCommonPerf.default 830 735 1.13:1
ChatWithPopoverPerf.default 562 500 1.12:1
FlexMinimalPerf.default 361 322 1.12:1
ItemLayoutMinimalPerf.default 1661 1486 1.12:1
AvatarMinimalPerf.default 507 458 1.11:1
ImageMinimalPerf.default 230 209 1.1:1
BoxMinimalPerf.default 228 209 1.09:1
ButtonSlotsPerf.default 566 523 1.08:1
Icon.Fluent 609 565 1.08:1
ListWith60ListItems.default 153 143 1.07:1
TreeMinimalPerf.default 914 851 1.07:1
ButtonMinimalPerf.default 115 108 1.06:1
GridMinimalPerf.default 742 703 1.06:1
HierarchicalTreeMinimalPerf.default 762 721 1.06:1
RadioGroupMinimalPerf.default 520 489 1.06:1
Image.Fluent 231 217 1.06:1
ReactionMinimalPerf.default 2471 2344 1.05:1
VideoMinimalPerf.default 689 654 1.05:1
AnimationMinimalPerf.default 462 443 1.04:1
ChatDuplicateMessagesPerf.default 344 330 1.04:1
EmbedMinimalPerf.default 5677 5476 1.04:1
Slider.Fluent 1330 1285 1.04:1
Button.Fluent 110 107 1.03:1
HeaderMinimalPerf.default 395 386 1.02:1
LayoutMinimalPerf.default 491 483 1.02:1
ListNestedPerf.default 654 640 1.02:1
StatusMinimalPerf.default 232 228 1.02:1
Checkbox.Fluent 749 732 1.02:1
Text.Fluent 259 255 1.02:1
DividerMinimalPerf.default 787 778 1.01:1
DropdownMinimalPerf.default 3208 3169 1.01:1
IconMinimalPerf.default 264 262 1.01:1
AlertMinimalPerf.default 528 529 1:1
AttachmentSlotsPerf.default 2933 2935 1:1
CheckboxMinimalPerf.default 3389 3396 1:1
LabelMinimalPerf.default 785 786 1:1
ListMinimalPerf.default 291 290 1:1
ProviderMinimalPerf.default 628 626 1:1
TreeWith60ListItems.default 215 216 1:1
AttachmentMinimalPerf.default 850 858 0.99:1
Avatar.Fluent 918 929 0.99:1
FormMinimalPerf.default 633 644 0.98:1
HeaderSlotsPerf.default 1173 1202 0.98:1
TextAreaMinimalPerf.default 2789 2835 0.98:1
Dropdown.Fluent 3145 3202 0.98:1
MenuMinimalPerf.default 1802 1866 0.97:1
CustomToolbarPrototype.default 3432 3541 0.97:1
TooltipMinimalPerf.default 633 653 0.97:1
MenuButtonMinimalPerf.default 1622 1688 0.96:1
SegmentMinimalPerf.default 1213 1270 0.96:1
TableMinimalPerf.default 522 543 0.96:1
ChatMinimalPerf.default 407 427 0.95:1
Dialog.Fluent 1429 1512 0.95:1
Tooltip.Fluent 431 456 0.95:1
AccordionMinimalPerf.default 183 194 0.94:1
LoaderMinimalPerf.default 771 820 0.94:1
CarouselMinimalPerf.default 1751 1887 0.93:1
DialogMinimalPerf.default 1557 1673 0.93:1
TextMinimalPerf.default 254 274 0.93:1
InputMinimalPerf.default 847 918 0.92:1
SliderMinimalPerf.default 1583 1732 0.91:1
PortalMinimalPerf.default 212 240 0.88:1

Generated by 🚫 dangerJS

@layershifter
Copy link
Member

Looks good 👍 Can you please merge master to make Screener passing?

@silviuaavram silviuaavram force-pushed the chore/dropdown-item-functional-component branch from 99f2e5a to fb57c55 Compare February 25, 2020 07:25
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.

Looks good, please add changelog entry with breaking change, and add in the PR description which props are now passed to the styles function

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants