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

feat(Popup): add show/hide animations #13439

Merged
merged 8 commits into from
Jun 13, 2020

Conversation

mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Jun 2, 2020

This PR is simply adding animations for showing/hiding popups. No customization are allowed at this moment, if we need to we can add them as a follow up

@@ -540,7 +545,7 @@ const Popup: React.FC<PopupProps> &
const element = (
<>
{triggerElement}
{open && (inline ? contentElement : <PortalInner mountNode={mountNode}>{contentElement}</PortalInner>)}
{inline ? contentElement : <PortalInner mountNode={mountNode}>{contentElement}</PortalInner>}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Animation component is responsible for mounting/removing the element now, so no need for this check

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Jun 2, 2020

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 824 853 5000
ButtonNext mount 409 401 5000
Checkbox mount 1520 1490 5000
CheckboxBase mount 1231 1180 5000
CheckboxNext mount 1560 1566 5000
ChoiceGroup mount 4682 4636 5000
ComboBox mount 889 903 1000
CommandBar mount 7025 7201 1000
ContextualMenu mount 11532 11589 1000
DefaultButton mount 1022 1020 5000
DetailsRow mount 3395 3401 5000
DetailsRowFast mount 3327 3371 5000
DetailsRowNoStyles mount 3051 3156 5000
Dialog mount 1331 1325 1000
DocumentCardTitle mount 1619 1599 1000
Dropdown mount 2287 2187 5000
FocusZone mount 1710 1726 5000
IconButton mount 1648 1680 5000
Label mount 357 335 5000
Link mount 469 476 5000
LinkNext mount 487 498 5000
MenuButton mount 1366 1371 5000
Nav mount 3000 3064 1000
Panel mount 1324 1384 1000
Persona mount 767 789 1000
Pivot mount 1312 1303 1000
PivotNext mount 1314 1265 1000
PrimaryButton mount 1222 1189 5000
SearchBox mount 1247 1231 5000
Slider mount 1453 1468 5000
SliderNext mount 1842 1892 5000
Spinner mount 415 414 5000
SplitButton mount 2971 3013 5000
Stack mount 477 472 5000
StackWithIntrinsicChildren mount 1757 1815 5000
StackWithTextChildren mount 4628 4635 5000
TagPicker mount 2669 2653 5000
Text mount 400 410 5000
TextField mount 1348 1347 5000
ThemeProvider mount 2779 2692 5000
ThemeProvider virtual-rerender 504 486 5000
Toggle mount 852 892 5000
ToggleNext mount 865 863 5000
button mount 103 107 5000

Perf Analysis (Fluent)

⚠️ 3 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
PopupMinimalPerf.default 892 253 3.53:1 analysis
MenuButtonMinimalPerf.default 1683 1116 1.51:1 analysis
SplitButtonMinimalPerf.default 3538 3174 1.11:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.43 0.46 0.93:1 2000 854
🦄 Button.Fluent 0.1 0.18 0.56:1 5000 491
🔧 Checkbox.Fluent 0.59 0.32 1.84:1 1000 593
🎯 Dialog.Fluent 0.14 0.2 0.7:1 5000 675
🔧 Dropdown.Fluent 3.05 0.4 7.62:1 1000 3054
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 628
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 329
🔧 Slider.Fluent 1.52 0.32 4.75:1 1000 1515
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 325
🦄 Tooltip.Fluent 0.09 15.62 0.01:1 5000 454

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
HeaderMinimalPerf.default 407 316 1.29:1
FlexMinimalPerf.default 276 239 1.15:1
GridMinimalPerf.default 639 582 1.1:1
RadioGroupMinimalPerf.default 395 359 1.1:1
TextMinimalPerf.default 311 282 1.1:1
BoxMinimalPerf.default 282 258 1.09:1
IconMinimalPerf.default 633 584 1.08:1
ListNestedPerf.default 852 795 1.07:1
RefMinimalPerf.default 193 181 1.07:1
ListMinimalPerf.default 452 425 1.06:1
ReactionMinimalPerf.default 363 344 1.06:1
DropdownMinimalPerf.default 3181 3044 1.05:1
ListCommonPerf.default 875 831 1.05:1
Slider.Fluent 1515 1462 1.04:1
HeaderSlotsPerf.default 737 717 1.03:1
SegmentMinimalPerf.default 293 285 1.03:1
TooltipMinimalPerf.default 723 703 1.03:1
Avatar.Fluent 854 830 1.03:1
Image.Fluent 329 319 1.03:1
AttachmentMinimalPerf.default 123 120 1.02:1
CardMinimalPerf.default 449 442 1.02:1
ChatDuplicateMessagesPerf.default 353 346 1.02:1
DividerMinimalPerf.default 297 292 1.02:1
FormMinimalPerf.default 372 365 1.02:1
ItemLayoutMinimalPerf.default 1212 1192 1.02:1
ProviderMinimalPerf.default 795 776 1.02:1
SliderMinimalPerf.default 1506 1476 1.02:1
ButtonSlotsPerf.default 482 479 1.01:1
ChatMinimalPerf.default 480 477 1.01:1
DialogMinimalPerf.default 596 591 1.01:1
LabelMinimalPerf.default 358 353 1.01:1
ListWith60ListItems.default 1030 1017 1.01:1
LoaderMinimalPerf.default 683 678 1.01:1
StatusMinimalPerf.default 615 611 1.01:1
Checkbox.Fluent 593 589 1.01:1
AvatarMinimalPerf.default 387 386 1:1
CheckboxMinimalPerf.default 2251 2250 1:1
InputMinimalPerf.default 1001 998 1:1
ProviderMergeThemesPerf.default 2056 2056 1:1
TableManyItemsPerf.default 2008 2000 1:1
TableMinimalPerf.default 343 342 1:1
TextAreaMinimalPerf.default 398 398 1:1
Dialog.Fluent 675 678 1:1
Icon.Fluent 628 625 1:1
Text.Fluent 325 326 1:1
Tooltip.Fluent 454 453 1:1
AccordionMinimalPerf.default 107 108 0.99:1
AnimationMinimalPerf.default 283 286 0.99:1
HierarchicalTreeMinimalPerf.default 383 385 0.99:1
MenuMinimalPerf.default 775 780 0.99:1
Dropdown.Fluent 3054 3085 0.99:1
ButtonMinimalPerf.default 139 142 0.98:1
CarouselMinimalPerf.default 354 363 0.98:1
ChatWithPopoverPerf.default 368 375 0.98:1
EmbedMinimalPerf.default 1808 1850 0.98:1
LayoutMinimalPerf.default 354 361 0.98:1
CustomToolbarPrototype.default 3618 3707 0.98:1
ToolbarMinimalPerf.default 855 876 0.98:1
TreeMinimalPerf.default 784 803 0.98:1
TreeWith60ListItems.default 205 212 0.97:1
VideoMinimalPerf.default 568 587 0.97:1
AttachmentSlotsPerf.default 932 967 0.96:1
PortalMinimalPerf.default 109 113 0.96:1
AlertMinimalPerf.default 218 229 0.95:1
ImageMinimalPerf.default 323 339 0.95:1
Button.Fluent 491 517 0.95:1
DropdownManyItemsPerf.default 1107 1261 0.88:1

@size-auditor
Copy link

size-auditor bot commented Jun 2, 2020

Asset size changes

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

Baseline commit: d4ec6202be3cee08373ab8636cdc6fa76a448aac (build)

@mnajdova
Copy link
Contributor Author

mnajdova commented Jun 8, 2020

The Animation component currently is not using the useStyles hook, but it is using directly the getStyles function with all performance flags disabled, which is causing a perf issues (at least for second renderings). This PR will not be merged until that issue is solved.

Edited: the PR #13509 is in, so we may continue with this one.

import * as React from 'react';

export * from 'react-transition-group';
export const Transition = props => {
Copy link
Member

Choose a reason for hiding this comment

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

Are there TransitionProps ? Can we use them?

Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

Please address comments and add a changelog entry before merging

@mnajdova mnajdova merged commit a317777 into microsoft:master Jun 13, 2020
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