Callout/composed path #19798
Merged
Callout/composed path #19798
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
@GeoffCoxMSFT do you have by any chance the opportunity to review that piece of code? |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ca12473:
|
📊 Bundle size report🤖 This report was generated against 9fafa80e50b2a53b2372a6844f04ef9ae9150e0b |
Asset size changes
Baseline commit: 9fafa80e50b2a53b2372a6844f04ef9ae9150e0b (build) |
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 797 | 784 | 5000 | |
| BaseButton | mount | 826 | 827 | 5000 | |
| Breadcrumb | mount | 2489 | 2505 | 1000 | |
| ButtonNext | mount | 465 | 460 | 5000 | |
| Checkbox | mount | 1393 | 1380 | 5000 | |
| CheckboxBase | mount | 1143 | 1155 | 5000 | |
| ChoiceGroup | mount | 4219 | 4211 | 5000 | |
| ComboBox | mount | 883 | 896 | 1000 | |
| CommandBar | mount | 9538 | 9627 | 1000 | |
| ContextualMenu | mount | 8016 | 8005 | 1000 | |
| DefaultButton | mount | 1052 | 1029 | 5000 | |
| DetailsRow | mount | 3340 | 3396 | 5000 | |
| DetailsRowFast | mount | 3402 | 3386 | 5000 | |
| DetailsRowNoStyles | mount | 3282 | 3287 | 5000 | |
| Dialog | mount | 2345 | 2376 | 1000 | |
| DocumentCardTitle | mount | 178 | 183 | 1000 | |
| Dropdown | mount | 2889 | 2892 | 5000 | |
| FluentProviderNext | mount | 2008 | 1911 | 5000 | |
| FluentProviderWithTheme | mount | 146 | 134 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 99 | 92 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 186 | 187 | 10 | |
| FocusTrapZone | mount | 1635 | 1634 | 5000 | |
| FocusZone | mount | 1573 | 1595 | 5000 | |
| IconButton | mount | 1583 | 1525 | 5000 | |
| Label | mount | 334 | 336 | 5000 | |
| Layer | mount | 2626 | 2626 | 5000 | |
| Link | mount | 439 | 449 | 5000 | |
| MakeStyles | mount | 1506 | 1495 | 50000 | |
| MenuButton | mount | 1257 | 1304 | 5000 | |
| MessageBar | mount | 1787 | 1762 | 5000 | |
| Nav | mount | 2890 | 2905 | 1000 | |
| OverflowSet | mount | 981 | 973 | 5000 | |
| Panel | mount | 2199 | 2183 | 1000 | |
| Persona | mount | 761 | 762 | 1000 | |
| Pivot | mount | 1304 | 1276 | 1000 | |
| PrimaryButton | mount | 1146 | 1140 | 5000 | |
| Rating | mount | 6770 | 6743 | 5000 | |
| SearchBox | mount | 1215 | 1176 | 5000 | |
| Shimmer | mount | 2257 | 2235 | 5000 | |
| Slider | mount | 1756 | 1734 | 5000 | |
| SpinButton | mount | 4377 | 4325 | 5000 | |
| Spinner | mount | 392 | 402 | 5000 | |
| SplitButton | mount | 2744 | 2738 | 5000 | |
| Stack | mount | 469 | 475 | 5000 | |
| StackWithIntrinsicChildren | mount | 1991 | 1995 | 5000 | |
| StackWithTextChildren | mount | 4565 | 4546 | 5000 | |
| SwatchColorPicker | mount | 9949 | 10007 | 5000 | |
| TagPicker | mount | 2309 | 2334 | 5000 | |
| TeachingBubble | mount | 11527 | 11634 | 5000 | |
| Text | mount | 400 | 399 | 5000 | |
| TextField | mount | 1166 | 1226 | 5000 | |
| ThemeProvider | mount | 1044 | 1060 | 5000 | |
| ThemeProvider | virtual-rerender | 549 | 556 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1601 | 1637 | 5000 | |
| Toggle | mount | 727 | 736 | 5000 | |
| buttonNative | mount | 133 | 133 | 5000 |
Perf Analysis (@fluentui/react-northstar)
Perf tests with no regressions
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| PortalMinimalPerf.default | 171 | 153 | 1.12:1 |
| AccordionMinimalPerf.default | 139 | 127 | 1.09:1 |
| TreeWith60ListItems.default | 167 | 157 | 1.06:1 |
| AttachmentSlotsPerf.default | 956 | 910 | 1.05:1 |
| ButtonSlotsPerf.default | 483 | 461 | 1.05:1 |
| ChatWithPopoverPerf.default | 339 | 322 | 1.05:1 |
| IconMinimalPerf.default | 534 | 511 | 1.05:1 |
| TextAreaMinimalPerf.default | 446 | 424 | 1.05:1 |
| AvatarMinimalPerf.default | 170 | 163 | 1.04:1 |
| ButtonMinimalPerf.default | 150 | 144 | 1.04:1 |
| ListMinimalPerf.default | 455 | 437 | 1.04:1 |
| AnimationMinimalPerf.default | 477 | 463 | 1.03:1 |
| FormMinimalPerf.default | 360 | 348 | 1.03:1 |
| HeaderMinimalPerf.default | 326 | 316 | 1.03:1 |
| ListNestedPerf.default | 481 | 465 | 1.03:1 |
| ToolbarMinimalPerf.default | 820 | 795 | 1.03:1 |
| BoxMinimalPerf.default | 304 | 297 | 1.02:1 |
| CardMinimalPerf.default | 485 | 477 | 1.02:1 |
| ChatDuplicateMessagesPerf.default | 263 | 258 | 1.02:1 |
| HeaderSlotsPerf.default | 657 | 643 | 1.02:1 |
| ProviderMinimalPerf.default | 1020 | 999 | 1.02:1 |
| ReactionMinimalPerf.default | 326 | 321 | 1.02:1 |
| SegmentMinimalPerf.default | 305 | 299 | 1.02:1 |
| TooltipMinimalPerf.default | 904 | 888 | 1.02:1 |
| VideoMinimalPerf.default | 555 | 542 | 1.02:1 |
| CheckboxMinimalPerf.default | 2317 | 2295 | 1.01:1 |
| DialogMinimalPerf.default | 658 | 653 | 1.01:1 |
| DividerMinimalPerf.default | 313 | 309 | 1.01:1 |
| DropdownManyItemsPerf.default | 594 | 588 | 1.01:1 |
| DropdownMinimalPerf.default | 2594 | 2580 | 1.01:1 |
| FlexMinimalPerf.default | 252 | 250 | 1.01:1 |
| GridMinimalPerf.default | 296 | 292 | 1.01:1 |
| ListWith60ListItems.default | 553 | 547 | 1.01:1 |
| MenuMinimalPerf.default | 725 | 717 | 1.01:1 |
| PopupMinimalPerf.default | 538 | 533 | 1.01:1 |
| RadioGroupMinimalPerf.default | 387 | 383 | 1.01:1 |
| StatusMinimalPerf.default | 589 | 583 | 1.01:1 |
| TableMinimalPerf.default | 352 | 347 | 1.01:1 |
| TextMinimalPerf.default | 300 | 298 | 1.01:1 |
| CustomToolbarPrototype.default | 3576 | 3558 | 1.01:1 |
| TreeMinimalPerf.default | 683 | 677 | 1.01:1 |
| AlertMinimalPerf.default | 248 | 248 | 1:1 |
| ButtonOverridesMissPerf.default | 1450 | 1452 | 1:1 |
| ChatMinimalPerf.default | 639 | 642 | 1:1 |
| DatepickerMinimalPerf.default | 4831 | 4821 | 1:1 |
| EmbedMinimalPerf.default | 3537 | 3540 | 1:1 |
| InputMinimalPerf.default | 1118 | 1119 | 1:1 |
| LabelMinimalPerf.default | 334 | 333 | 1:1 |
| LayoutMinimalPerf.default | 309 | 310 | 1:1 |
| LoaderMinimalPerf.default | 603 | 604 | 1:1 |
| MenuButtonMinimalPerf.default | 1428 | 1435 | 1:1 |
| ProviderMergeThemesPerf.default | 1484 | 1487 | 1:1 |
| SliderMinimalPerf.default | 1439 | 1445 | 1:1 |
| SplitButtonMinimalPerf.default | 3727 | 3712 | 1:1 |
| TableManyItemsPerf.default | 1610 | 1614 | 1:1 |
| CarouselMinimalPerf.default | 406 | 411 | 0.99:1 |
| ItemLayoutMinimalPerf.default | 1012 | 1025 | 0.99:1 |
| ListCommonPerf.default | 544 | 548 | 0.99:1 |
| RefMinimalPerf.default | 205 | 208 | 0.99:1 |
| SkeletonMinimalPerf.default | 303 | 306 | 0.99:1 |
| ImageMinimalPerf.default | 311 | 321 | 0.97:1 |
| AttachmentMinimalPerf.default | 133 | 138 | 0.96:1 |
| RosterPerf.default | 1011 | 1053 | 0.96:1 |
|
@microsoft/cxe-red can you please check these changes? |
ecraig12345
reviewed
Dec 7, 2021
| @@ -0,0 +1,10 @@ | |||
| { | |||
There was a problem hiding this comment.
This got generated in the wrong format somehow. Can you delete this file, merge with master, and then run yarn and yarn change again? Be sure to add an informative message, since this goes in the package changelog.
There was a problem hiding this comment.
@ecraig12345 Regenerated now. :)
|
@GeoffCoxMSFT / @ecraig12345 - May I ask the status on this PR? As there are various issues associated to this suggested fix. |
|
@spmonahan this change looks reasonable to me from Please defer to someone on the Fluent team for final approval as I am still ramping up, but I have no concerns with the proposed change. I do wonder if other components may also have the same bug and need a similar fix in the long-term, but for now let's keep the change scoped. |
|
@ermercer Appreciate you taking a look! |
spmonahan
requested changes
Jan 15, 2022
There was a problem hiding this comment.
@mlp73 Thanks for the PR! Change looks good to me.
The build is failing right now however which may be related to the age of this PR. Can you pull the latest changes from upstream master into your fork? Hopefully that will get the build passing.
JustSlone
removed request for
czearing,
dzearing,
EisenbergEffect,
evlevy,
GeoffCoxMSFT,
Hirse,
hyoshis,
Jacqueline-ms,
Jahnp,
jurokapsiar,
khmakoto,
kolaps33,
layershifter,
leddie24,
ling1726,
micahgodbolt,
miroslavstastny,
nicholasrice,
phkuo,
pompomon,
Raghurk,
ThomasMichon,
Vitalius1 and
yiminwu
|
@mlp73 I cleaned up the review list, if I removed someone that should be on, feel free to add them back |
|
@spmonahan It seems like the Fluent UI React pipeline is failing on the "build Storybooks" step. I cannot see any immediate relation with the code in the PR, but I can't fint any re-run action button to retry. Could you help? |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
@mlp73 Done! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request checklist
$ yarn changeDescription of changes
Making dismissOnClickOrScroll function in Callout component more robust by supporting both target event (Edge) and event composedPath (other browsers). As described in the related issue, this opens up to precisely targeting the actual node when an event is triggered accross light and shadow doms.