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.

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

chore(Popup): refactor PopupContent to be functional component #12513

Merged
merged 6 commits into from
Apr 2, 2020

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Apr 2, 2020

BREAKING CHANGES

As in other components converted to hooks PopupContent now passes only these props to styles: placement, pointing.

This PR refactors PopupContent to be functional component, related to #12237.

Microsoft Reviewers: Open in CodeFlow

@@ -2,5 +2,5 @@ import { isConformant } from 'test/specs/commonTests';
import PopupContent from 'src/components/Popup/PopupContent';

describe('PopupContent', () => {
isConformant(PopupContent, { rendersPortal: true });
Copy link
Member Author

Choose a reason for hiding this comment

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

It never rendered Portal 😯

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Apr 2, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 837 852
BaseButton (experiments) 1019 1013
DefaultButton 1060 1118
DefaultButton (experiments) 1994 1949
DetailsRow 3477 3424
DetailsRow (fast icons) 3542 3608
DetailsRow without styles 3326 3166
DocumentCardTitle with truncation 1482 1548
MenuButton 1465 1450
MenuButton (experiments) 3613 3588
PrimaryButton 1222 1225
PrimaryButton (experiments) 1997 2088
SplitButton 3065 3131
SplitButton (experiments) 7062 7090
Stack 500 493
Stack with Intrinsic children 1106 1108
Stack with Text children 4321 4249
Text 410 379
Toggle 902 913
Toggle (experiments) 2349 2344
button 66 61

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.59 0.51 1.16:1 2000 1175
🦄 Button.Fluent 0.12 0.2 0.6:1 5000 587
🔧 Checkbox.Fluent 0.78 0.42 1.86:1 1000 780
🔧 Dialog.Fluent 0.44 0.21 2.1:1 5000 2199
🔧 Dropdown.Fluent 3.94 0.51 7.73:1 1000 3941
🔧 Icon.Fluent 0.21 0.05 4.2:1 5000 1043
🎯 Image.Fluent 0.09 0.11 0.82:1 5000 441
🔧 Slider.Fluent 1.66 0.46 3.61:1 1000 1657
🔧 Text.Fluent 0.1 0.02 5:1 5000 479
🦄 Tooltip.Fluent 0.14 17.26 0.01:1 5000 680

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
PopupMinimalPerf.default 272 248 1.1:1
Icon.Fluent 1043 958 1.09:1
ButtonMinimalPerf.default 184 170 1.08:1
CardMinimalPerf.default 471 436 1.08:1
DividerMinimalPerf.default 1166 1078 1.08:1
TooltipMinimalPerf.default 1016 944 1.08:1
Button.Fluent 587 550 1.07:1
HeaderMinimalPerf.default 649 614 1.06:1
ListMinimalPerf.default 549 516 1.06:1
TableMinimalPerf.default 802 758 1.06:1
CarouselMinimalPerf.default 702 671 1.05:1
DropdownMinimalPerf.default 3994 3791 1.05:1
FormMinimalPerf.default 1108 1051 1.05:1
ImageMinimalPerf.default 419 399 1.05:1
ItemLayoutMinimalPerf.default 2387 2271 1.05:1
ChatWithPopoverPerf.default 671 644 1.04:1
DropdownManyItemsPerf.default 1637 1567 1.04:1
TreeWith60ListItems.default 256 247 1.04:1
Dropdown.Fluent 3941 3772 1.04:1
Tooltip.Fluent 680 657 1.04:1
HeaderSlotsPerf.default 1905 1853 1.03:1
IconMinimalPerf.default 509 496 1.03:1
ListNestedPerf.default 1098 1069 1.03:1
LoaderMinimalPerf.default 1183 1154 1.03:1
Checkbox.Fluent 780 755 1.03:1
Slider.Fluent 1657 1608 1.03:1
AttachmentSlotsPerf.default 3981 3886 1.02:1
AvatarMinimalPerf.default 637 622 1.02:1
BoxMinimalPerf.default 451 441 1.02:1
ButtonSlotsPerf.default 704 692 1.02:1
GridMinimalPerf.default 949 933 1.02:1
SplitButtonMinimalPerf.default 3990 3921 1.02:1
TextMinimalPerf.default 460 451 1.02:1
TextAreaMinimalPerf.default 3413 3347 1.02:1
ToolbarMinimalPerf.default 1287 1266 1.02:1
AnimationMinimalPerf.default 767 756 1.01:1
ChatMinimalPerf.default 702 698 1.01:1
ListCommonPerf.default 1160 1144 1.01:1
RefMinimalPerf.default 212 209 1.01:1
CustomToolbarPrototype.default 3987 3939 1.01:1
Image.Fluent 441 436 1.01:1
InputMinimalPerf.default 1164 1167 1:1
MenuButtonMinimalPerf.default 1822 1819 1:1
PortalMinimalPerf.default 333 332 1:1
TreeMinimalPerf.default 1346 1341 1:1
VideoMinimalPerf.default 1037 1036 1:1
Dialog.Fluent 2199 2197 1:1
Text.Fluent 479 477 1:1
AlertMinimalPerf.default 649 655 0.99:1
CheckboxMinimalPerf.default 3490 3535 0.99:1
HierarchicalTreeMinimalPerf.default 1196 1214 0.99:1
LabelMinimalPerf.default 457 463 0.99:1
ProviderMinimalPerf.default 683 693 0.99:1
RadioGroupMinimalPerf.default 607 615 0.99:1
StatusMinimalPerf.default 794 806 0.99:1
Avatar.Fluent 1175 1192 0.99:1
ChatDuplicateMessagesPerf.default 437 445 0.98:1
DialogMinimalPerf.default 2110 2164 0.98:1
LayoutMinimalPerf.default 749 765 0.98:1
MenuMinimalPerf.default 2140 2186 0.98:1
SegmentMinimalPerf.default 1223 1250 0.98:1
SliderMinimalPerf.default 1669 1704 0.98:1
AttachmentMinimalPerf.default 949 983 0.97:1
FlexMinimalPerf.default 337 347 0.97:1
ListWith60ListItems.default 1328 1364 0.97:1
ProviderMergeThemesPerf.default 1490 1537 0.97:1
EmbedMinimalPerf.default 5683 5894 0.96:1
ReactionMinimalPerf.default 2604 2732 0.95:1
AccordionMinimalPerf.default 265 283 0.94:1

@size-auditor
Copy link

size-auditor bot commented Apr 2, 2020

Asset size changes

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

Baseline commit: ce7ca511daa6798440e675290c69c3ff21830b4f (build)

@layershifter layershifter merged commit 62a9641 into master Apr 2, 2020
@layershifter layershifter deleted the chore/popup-content-to-fc branch April 2, 2020 12:06
DuanShaolong pushed a commit to DuanShaolong/fluentui that referenced this pull request Apr 27, 2020
…soft#12513)

* chore(Popup): refactor PopupContent to be functional component

* add changelog entry

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

Co-Authored-By: Roman Sudarikov <pompomon@users.noreply.github.com>

* make required to catch missing props

* add missing prop

Co-authored-by: Roman Sudarikov <pompomon@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants