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

Dialog: Fixing broken deprecated props and adding snapshot tests for them #13425

Merged
merged 3 commits into from
Jun 1, 2020

Conversation

khmakoto
Copy link
Member

@khmakoto khmakoto commented Jun 1, 2020

Pull request checklist

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

Description of changes

#12045 broke the use of some deprecated props in Dialog by overriding them with default props. This PR fixes this issue and adds snapshot tests for the deprecated props so that this doesn't happen again.

Focus areas to test

(optional)

@khmakoto
Copy link
Member Author

khmakoto commented Jun 1, 2020

@msft-github-bot merge in 1 minute.

@msft-github-bot
Copy link
Contributor

Hello @khmakoto!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Mon, 01 Jun 2020 21:06:13 GMT, which is in 1 minute

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@msft-github-bot
Copy link
Contributor

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 799 831 5000
ButtonNext 474 435 5000
Checkbox 1601 1502 5000
CheckboxBase 1438 1316 5000
CheckboxNext 1539 1502 5000
ChoiceGroup 4930 4992 5000
ComboBox 975 925 1000
CommandBar 7282 7300 1000
ContextualMenu 11286 11366 1000
DefaultButton 1017 1069 5000
DetailsRow 3495 3357 5000
DetailsRow (fast icons) 3487 3520 5000
DetailsRow without styles 3163 3202 5000
Dialog 1474 1421 1000
DocumentCardTitle with truncation 1737 1746 1000
Dropdown 2414 2420 5000
FocusZone 1575 1644 5000
IconButton 1713 1679 5000
Label 301 298 5000
Link 450 463 5000
LinkNext 440 471 5000
MenuButton 1414 1405 5000
Nav 3040 3160 1000
Panel 1413 1371 1000
Persona 819 806 1000
Pivot 1342 1353 1000
PivotNext 779 784 5000
PrimaryButton 1179 1202 5000
SearchBox 1235 1235 5000
Slider 1448 1450 5000
SliderNext 1870 1848 5000
Spinner 401 403 5000
SplitButton 2965 2972 5000
Stack 469 484 5000
Stack with Intrinsic children 1904 1776 5000
Stack with Text children 4997 4864 5000
TagPicker 2789 2614 5000
Text 371 384 5000
TextField 1386 1358 5000
ThemeProvider 2585 2683 5000
Toggle 840 853 5000
ToggleNext 898 838 5000
button 81 80 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.69 0.47 1.47:1 2000 1375
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 545
🔧 Checkbox.Fluent 1.03 0.37 2.78:1 1000 1032
🦄 Dialog.Fluent 0.14 0.21 0.67:1 5000 707
🔧 Dropdown.Fluent 6.05 0.43 14.07:1 1000 6052
🔧 Icon.Fluent 0.14 0.05 2.8:1 5000 699
🎯 Image.Fluent 0.09 0.1 0.9:1 5000 430
🔧 Slider.Fluent 2.6 0.35 7.43:1 1000 2602
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 343
🦄 Tooltip.Fluent 0.09 14.8 0.01:1 5000 474

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
Image.Fluent 430 370 1.16:1
AttachmentMinimalPerf.default 160 143 1.12:1
ButtonMinimalPerf.default 180 163 1.1:1
BoxMinimalPerf.default 355 332 1.07:1
ReactionMinimalPerf.default 384 358 1.07:1
TableMinimalPerf.default 374 348 1.07:1
ListNestedPerf.default 1124 1061 1.06:1
MenuButtonMinimalPerf.default 1781 1686 1.06:1
VideoMinimalPerf.default 622 586 1.06:1
CheckboxMinimalPerf.default 5244 5016 1.05:1
DropdownManyItemsPerf.default 2092 2000 1.05:1
ImageMinimalPerf.default 367 351 1.05:1
LabelMinimalPerf.default 418 399 1.05:1
ProviderMinimalPerf.default 800 764 1.05:1
ToolbarMinimalPerf.default 947 905 1.05:1
HeaderMinimalPerf.default 357 342 1.04:1
MenuMinimalPerf.default 850 818 1.04:1
ProviderMergeThemesPerf.default 1818 1751 1.04:1
IconMinimalPerf.default 695 668 1.04:1
Dialog.Fluent 707 678 1.04:1
Text.Fluent 343 329 1.04:1
AlertMinimalPerf.default 358 346 1.03:1
ChatDuplicateMessagesPerf.default 541 523 1.03:1
InputMinimalPerf.default 1556 1504 1.03:1
TextMinimalPerf.default 350 339 1.03:1
TreeWith60ListItems.default 271 262 1.03:1
AvatarMinimalPerf.default 758 743 1.02:1
CarouselMinimalPerf.default 509 498 1.02:1
DialogMinimalPerf.default 715 701 1.02:1
RadioGroupMinimalPerf.default 394 385 1.02:1
TooltipMinimalPerf.default 707 695 1.02:1
Avatar.Fluent 1375 1353 1.02:1
AttachmentSlotsPerf.default 1216 1205 1.01:1
CardMinimalPerf.default 541 536 1.01:1
DropdownMinimalPerf.default 6023 5954 1.01:1
ListWith60ListItems.default 1540 1532 1.01:1
SegmentMinimalPerf.default 335 331 1.01:1
TableManyItemsPerf.default 2440 2423 1.01:1
CustomToolbarPrototype.default 4712 4682 1.01:1
HeaderSlotsPerf.default 764 766 1:1
ItemLayoutMinimalPerf.default 2344 2341 1:1
LayoutMinimalPerf.default 378 377 1:1
ListCommonPerf.default 1107 1104 1:1
TextAreaMinimalPerf.default 485 485 1:1
Dropdown.Fluent 6052 6061 1:1
AnimationMinimalPerf.default 761 772 0.99:1
ChatMinimalPerf.default 590 597 0.99:1
EmbedMinimalPerf.default 3404 3428 0.99:1
RefMinimalPerf.default 183 185 0.99:1
SplitButtonMinimalPerf.default 3954 4003 0.99:1
TreeMinimalPerf.default 1362 1377 0.99:1
Checkbox.Fluent 1032 1039 0.99:1
Icon.Fluent 699 706 0.99:1
DividerMinimalPerf.default 339 345 0.98:1
FormMinimalPerf.default 386 393 0.98:1
GridMinimalPerf.default 1350 1371 0.98:1
ListMinimalPerf.default 457 465 0.98:1
PopupMinimalPerf.default 245 249 0.98:1
SliderMinimalPerf.default 2764 2808 0.98:1
Button.Fluent 545 557 0.98:1
Tooltip.Fluent 474 483 0.98:1
ButtonSlotsPerf.default 699 722 0.97:1
ChatWithPopoverPerf.default 514 530 0.97:1
StatusMinimalPerf.default 697 720 0.97:1
FlexMinimalPerf.default 290 304 0.95:1
HierarchicalTreeMinimalPerf.default 404 425 0.95:1
PortalMinimalPerf.default 105 113 0.93:1
Slider.Fluent 2602 2819 0.92:1
AccordionMinimalPerf.default 139 152 0.91:1
LoaderMinimalPerf.default 1060 1159 0.91:1

@size-auditor
Copy link

size-auditor bot commented Jun 1, 2020

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react office-ui-fabric-react-Dialog 198.649 kB 198.655 kB ExceedsBaseline     6 bytes
office-ui-fabric-react fluentui-react-next-Dialog 198.608 kB 198.614 kB ExceedsBaseline     6 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 49592828383aeb1a4c9b199c99dad89ab2fafbae (build)

@msft-github-bot msft-github-bot merged commit 7a81e94 into microsoft:master Jun 1, 2020
@khmakoto khmakoto deleted the dialogDeprecatedProps branch June 1, 2020 22:57
@msft-github-bot
Copy link
Contributor

🎉office-ui-fabric-react@v7.117.1 has been released which incorporates this pull request.:tada:

Handy links:

miroslavstastny pushed a commit to levithomason/fluentui that referenced this pull request Jun 8, 2020
…them (microsoft#13425)

#### Pull request checklist

- [ ] Addresses an existing issue: Fixes #0000
- [x] Include a change request file using `$ yarn change`

#### Description of changes

microsoft#12045 broke the use of some deprecated props in `Dialog` by overriding them with default props. This PR fixes this issue and adds snapshot tests for the deprecated props so that this doesn't happen again.

#### Focus areas to test

(optional)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants