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

fix(Popup): update offsets computations #13002

Merged
merged 7 commits into from
May 11, 2020
Merged

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented May 5, 2020

Pull request checklist

Description of changes

In #12577 I fixed sizes of pointers, but during Popper update (#12530) I missed this update. I checked the issue and we should include padding when we are computing offsets. It's annoying that Screener can't catch such issues 👎

Before

image

After

image
image

(with pointerMargin: pxToRem(20))

image
image

(with pointerMargin: pxToRem(30))

image
image

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@@ -13,7 +13,7 @@ const popupContentStyles: ComponentSlotStylesPrepared<PopupContentStylesProps, P
pointerEvents: 'none',
...getContainerStyles({
placement: p.basePlacement,
margin: v.pointerMargin,
padding: v.pointerMargin,
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's actually padding. Decided to keep variable's name to avoid breaking changes

@msft-github-bot
Copy link
Contributor

msft-github-bot commented May 5, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 754 688 5000
Checkbox 1402 1425 5000
CheckboxBase 1182 1198 5000
CheckboxNext 1408 1413 5000
ChoiceGroup 4584 4544 5000
ComboBox 796 862 1000
CommandBar 6669 6627 1000
ContextualMenu 12679 12357 1000
DefaultButton 880 991 5000
DetailsRow 3098 3009 5000
DetailsRow (fast icons) 2946 3018 5000
DetailsRow without styles 2844 2869 5000
Dialog 1290 1311 1000
DocumentCardTitle with truncation 1462 1521 1000
Dropdown 2130 2044 5000
FocusZone 1426 1445 5000
IconButton 1465 1445 5000
Label 269 273 5000
Link 413 399 5000
MenuButton 1249 1247 5000
Nav 2740 2794 1000
Panel 1255 1307 1000
Persona 763 718 1000
Pivot 1160 1154 1000
PrimaryButton 1069 1089 5000
SearchBox 1097 1067 5000
Slider 1335 1235 5000
Spinner 352 356 5000
SplitButton 2644 2709 5000
Stack 392 401 5000
Stack with Intrinsic children 967 927 5000
Stack with Text children 3626 3476 5000
TagPicker 2349 2436 5000
Text 342 334 5000
TextField 1237 1272 5000
Toggle 766 781 5000
button 70 61 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.44 0.43 1.02:1 2000 870
🦄 Button.Fluent 0.09 0.16 0.56:1 5000 465
🔧 Checkbox.Fluent 0.6 0.3 2:1 1000 596
🔧 Dialog.Fluent 0.32 0.19 1.68:1 5000 1575
🔧 Dropdown.Fluent 3.1 0.39 7.95:1 1000 3099
🔧 Icon.Fluent 0.13 0.04 3.25:1 5000 641
🎯 Image.Fluent 0.07 0.09 0.78:1 5000 339
🔧 Slider.Fluent 1.27 0.33 3.85:1 1000 1269
🔧 Text.Fluent 0.06 0.02 3:1 5000 322
🦄 Tooltip.Fluent 0.08 16.39 0:1 5000 411

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
RadioGroupMinimalPerf.default 540 488 1.11:1
Text.Fluent 322 292 1.1:1
AttachmentMinimalPerf.default 149 137 1.09:1
ChatDuplicateMessagesPerf.default 412 384 1.07:1
PopupMinimalPerf.default 248 233 1.06:1
Image.Fluent 339 321 1.06:1
CardMinimalPerf.default 506 484 1.05:1
FlexMinimalPerf.default 265 252 1.05:1
HeaderMinimalPerf.default 453 430 1.05:1
ProviderMinimalPerf.default 627 598 1.05:1
RefMinimalPerf.default 196 187 1.05:1
AttachmentSlotsPerf.default 1082 1038 1.04:1
DropdownMinimalPerf.default 3217 3106 1.04:1
ListMinimalPerf.default 437 421 1.04:1
ButtonSlotsPerf.default 575 559 1.03:1
StatusMinimalPerf.default 601 584 1.03:1
BoxMinimalPerf.default 292 287 1.02:1
GridMinimalPerf.default 619 604 1.02:1
HierarchicalTreeMinimalPerf.default 905 887 1.02:1
LayoutMinimalPerf.default 513 502 1.02:1
ListCommonPerf.default 902 884 1.02:1
MenuMinimalPerf.default 1723 1684 1.02:1
IconMinimalPerf.default 598 587 1.02:1
Checkbox.Fluent 596 583 1.02:1
Icon.Fluent 641 630 1.02:1
AccordionMinimalPerf.default 125 124 1.01:1
AlertMinimalPerf.default 277 275 1.01:1
AnimationMinimalPerf.default 590 586 1.01:1
DividerMinimalPerf.default 306 303 1.01:1
EmbedMinimalPerf.default 1881 1859 1.01:1
ItemLayoutMinimalPerf.default 1569 1559 1.01:1
LabelMinimalPerf.default 354 349 1.01:1
ReactionMinimalPerf.default 1754 1739 1.01:1
TableMinimalPerf.default 341 337 1.01:1
TextMinimalPerf.default 318 315 1.01:1
ToolbarMinimalPerf.default 763 754 1.01:1
TooltipMinimalPerf.default 683 676 1.01:1
TreeMinimalPerf.default 1090 1080 1.01:1
Dialog.Fluent 1575 1558 1.01:1
CarouselMinimalPerf.default 432 432 1:1
ChatMinimalPerf.default 574 572 1:1
FormMinimalPerf.default 682 682 1:1
ListWith60ListItems.default 1044 1045 1:1
ProviderMergeThemesPerf.default 1567 1564 1:1
SegmentMinimalPerf.default 831 833 1:1
SplitButtonMinimalPerf.default 3086 3077 1:1
CustomToolbarPrototype.default 3430 3425 1:1
Dropdown.Fluent 3099 3089 1:1
AvatarMinimalPerf.default 490 497 0.99:1
ChatWithPopoverPerf.default 557 565 0.99:1
DropdownManyItemsPerf.default 1266 1283 0.99:1
LoaderMinimalPerf.default 737 746 0.99:1
TextAreaMinimalPerf.default 407 412 0.99:1
Button.Fluent 465 472 0.99:1
CheckboxMinimalPerf.default 2706 2754 0.98:1
DialogMinimalPerf.default 1587 1626 0.98:1
HeaderSlotsPerf.default 1326 1347 0.98:1
InputMinimalPerf.default 925 945 0.98:1
ListNestedPerf.default 802 815 0.98:1
MenuButtonMinimalPerf.default 1427 1449 0.98:1
SliderMinimalPerf.default 1302 1333 0.98:1
Avatar.Fluent 870 891 0.98:1
Slider.Fluent 1269 1306 0.97:1
ButtonMinimalPerf.default 146 152 0.96:1
PortalMinimalPerf.default 269 279 0.96:1
TreeWith60ListItems.default 206 215 0.96:1
VideoMinimalPerf.default 512 536 0.96:1
Tooltip.Fluent 411 429 0.96:1
ImageMinimalPerf.default 321 337 0.95:1

@size-auditor
Copy link

size-auditor bot commented May 5, 2020

Asset size changes

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

Baseline commit: aa2d3f52cfc21c8c15e87e46947f43c2249339b2 (build)

@layershifter layershifter merged commit 4ea3c3f into master May 11, 2020
@layershifter layershifter deleted the fix/popup-padding branch May 11, 2020 10:24
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.

Popup: pointers broken in Fluent v0
4 participants