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(MenuItem): prevent to dismiss menu on scroll inside menu #17294

Merged
merged 5 commits into from
Mar 6, 2021

Conversation

assuncaocharles
Copy link
Contributor

Pull request checklist

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

Description of changes

Prevent submenu to be dismissed when scroll happens inside Menu

Focus areas to test

(optional)

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.

We discussed this in offline and a fix makes sense for me, thanks for doing this 👍

….tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 5, 2021

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 d75d24f:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@size-auditor
Copy link

size-auditor bot commented Mar 5, 2021

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-northstar-Menu 143.3 kB 143.333 kB ExceedsBaseline     33 bytes
office-ui-fabric-react fluentui-react-northstar-Chat 158.518 kB 158.55 kB ExceedsBaseline     32 bytes
office-ui-fabric-react fluentui-react-northstar-SplitButton 190.886 kB 190.918 kB ExceedsBaseline     32 bytes
office-ui-fabric-react fluentui-react-northstar-MenuButton 178.064 kB 178.096 kB ExceedsBaseline     32 bytes

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

Baseline commit: d9fb5e3ce8dfdf0f6fdb5dee945a29243ec85dc9 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 5, 2021

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1255 1265 5000
BaseButton mount 1042 1054 5000
Breadcrumb mount 44478 45683 5000
ButtonNext mount 1376 1386 5000
Checkbox mount 1608 1639 5000
CheckboxBase mount 1382 1376 5000
ChoiceGroup mount 5231 5101 5000
ComboBox mount 1106 1063 1000
CommandBar mount 10682 10575 1000
ContextualMenu mount 6469 6571 1000
DefaultButton mount 1198 1227 5000
DetailsRow mount 3952 3777 5000
DetailsRowFast mount 3849 3887 5000
DetailsRowNoStyles mount 3604 3651 5000
Dialog mount 1586 1550 1000
DocumentCardTitle mount 1876 1897 1000
Dropdown mount 3587 3547 5000
FocusTrapZone mount 1860 1831 5000
FocusZone mount 1907 1881 5000
IconButton mount 1871 1944 5000
Label mount 359 350 5000
Layer mount 1954 1940 5000
Link mount 509 519 5000
MakeStyles mount 2105 2095 50000
MenuButton mount 1618 1595 5000
MessageBar mount 2091 2148 5000
Nav mount 3597 3572 1000
OverflowSet mount 1076 1142 5000
Panel mount 1501 1549 1000
Persona mount 892 935 1000
Pivot mount 1545 1515 1000
PrimaryButton mount 1410 1416 5000
Rating mount 8253 8359 5000
SearchBox mount 1467 1449 5000
Shimmer mount 2843 2889 5000
Slider mount 2091 2126 5000
SpinButton mount 5345 5365 5000
Spinner mount 437 435 5000
SplitButton mount 3494 3414 5000
Stack mount 553 543 5000
StackWithIntrinsicChildren mount 1719 1747 5000
StackWithTextChildren mount 4923 5091 5000
SwatchColorPicker mount 12108 10896 5000
Tabs mount 1517 1498 1000
TagPicker mount 2989 3023 5000
TeachingBubble mount 12419 12205 5000
Text mount 456 474 5000
TextField mount 1466 1474 5000
ThemeProvider mount 1220 1249 5000
ThemeProvider virtual-rerender 627 619 5000
ThemeProviderNext mount 16104 15974 5000
Toggle mount 876 889 5000
buttonNative mount 134 114 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.19 0.53 0.36:1 2000 381
🦄 Button.Fluent 0.14 0.22 0.64:1 5000 694
🔧 Checkbox.Fluent 0.69 0.37 1.86:1 1000 687
🎯 Dialog.Fluent 0.18 0.23 0.78:1 5000 880
🔧 Dropdown.Fluent 3.21 0.43 7.47:1 1000 3211
🔧 Icon.Fluent 0.15 0.06 2.5:1 5000 761
🦄 Image.Fluent 0.09 0.14 0.64:1 5000 456
🔧 Slider.Fluent 1.74 0.5 3.48:1 1000 1738
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 422
🦄 Tooltip.Fluent 0.12 0.97 0.12:1 5000 624

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
TreeWith60ListItems.default 213 187 1.14:1
AlertMinimalPerf.default 381 348 1.09:1
BoxMinimalPerf.default 447 410 1.09:1
FlexMinimalPerf.default 358 331 1.08:1
ButtonSlotsPerf.default 693 647 1.07:1
CarouselMinimalPerf.default 595 555 1.07:1
Button.Fluent 694 648 1.07:1
GridMinimalPerf.default 419 394 1.06:1
ReactionMinimalPerf.default 505 476 1.06:1
TextMinimalPerf.default 422 399 1.06:1
ImageMinimalPerf.default 483 461 1.05:1
PortalMinimalPerf.default 183 174 1.05:1
TextAreaMinimalPerf.default 597 571 1.05:1
ChatDuplicateMessagesPerf.default 426 411 1.04:1
ChatWithPopoverPerf.default 515 495 1.04:1
HeaderSlotsPerf.default 934 897 1.04:1
PopupMinimalPerf.default 791 759 1.04:1
ProviderMinimalPerf.default 1092 1053 1.04:1
AvatarMinimalPerf.default 238 232 1.03:1
CheckboxMinimalPerf.default 3135 3032 1.03:1
DropdownManyItemsPerf.default 870 846 1.03:1
DropdownMinimalPerf.default 3359 3274 1.03:1
ListCommonPerf.default 807 782 1.03:1
SegmentMinimalPerf.default 423 411 1.03:1
TableManyItemsPerf.default 2407 2337 1.03:1
Tooltip.Fluent 624 605 1.03:1
AttachmentSlotsPerf.default 1355 1324 1.02:1
DividerMinimalPerf.default 459 452 1.02:1
LabelMinimalPerf.default 494 486 1.02:1
LoaderMinimalPerf.default 835 822 1.02:1
MenuMinimalPerf.default 1038 1017 1.02:1
RadioGroupMinimalPerf.default 542 533 1.02:1
ToolbarMinimalPerf.default 1109 1084 1.02:1
AnimationMinimalPerf.default 481 476 1.01:1
AttachmentMinimalPerf.default 193 191 1.01:1
ButtonMinimalPerf.default 212 209 1.01:1
CardMinimalPerf.default 670 663 1.01:1
DatepickerMinimalPerf.default 52034 51714 1.01:1
EmbedMinimalPerf.default 4717 4660 1.01:1
LayoutMinimalPerf.default 491 484 1.01:1
ListMinimalPerf.default 617 611 1.01:1
StatusMinimalPerf.default 857 850 1.01:1
IconMinimalPerf.default 801 795 1.01:1
TableMinimalPerf.default 493 487 1.01:1
Dialog.Fluent 880 874 1.01:1
ButtonUseCssPerf.default 925 928 1:1
HeaderMinimalPerf.default 432 433 1:1
InputMinimalPerf.default 1435 1432 1:1
ItemLayoutMinimalPerf.default 1425 1432 1:1
ListNestedPerf.default 682 682 1:1
SplitButtonMinimalPerf.default 4213 4194 1:1
TooltipMinimalPerf.default 931 927 1:1
Checkbox.Fluent 687 690 1:1
Text.Fluent 422 423 1:1
ButtonOverridesMissPerf.default 1860 1881 0.99:1
ButtonUseCssNestingPerf.default 1200 1209 0.99:1
MenuButtonMinimalPerf.default 1788 1813 0.99:1
RosterPerf.default 1301 1313 0.99:1
TreeMinimalPerf.default 878 890 0.99:1
Avatar.Fluent 381 384 0.99:1
Dropdown.Fluent 3211 3258 0.99:1
Icon.Fluent 761 771 0.99:1
Slider.Fluent 1738 1763 0.99:1
ChatMinimalPerf.default 730 742 0.98:1
DialogMinimalPerf.default 895 913 0.98:1
FormMinimalPerf.default 490 498 0.98:1
ProviderMergeThemesPerf.default 1672 1698 0.98:1
RefMinimalPerf.default 260 264 0.98:1
SliderMinimalPerf.default 1713 1743 0.98:1
CustomToolbarPrototype.default 4027 4097 0.98:1
VideoMinimalPerf.default 729 742 0.98:1
Image.Fluent 456 464 0.98:1
ListWith60ListItems.default 722 752 0.96:1
SkeletonMinimalPerf.default 424 451 0.94:1
AccordionMinimalPerf.default 181 196 0.92:1

@assuncaocharles assuncaocharles merged commit 7101008 into microsoft:master Mar 6, 2021
@assuncaocharles assuncaocharles deleted the fix/menu-item branch March 6, 2021 00:42
YuanboXue-Amber pushed a commit that referenced this pull request Mar 9, 2021
* fix(MenuItem): prevent to dimiss menu on scroll inside menu

* fix(MenuItem): add changelog

* Update packages/fluentui/react-northstar/src/components/Menu/MenuItem.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
joshualamusga1 pushed a commit to joshualamusga1/fluentui that referenced this pull request Mar 25, 2021
…ft#17294)

* fix(MenuItem): prevent to dimiss menu on scroll inside menu

* fix(MenuItem): add changelog

* Update packages/fluentui/react-northstar/src/components/Menu/MenuItem.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
…ft#17294)

* fix(MenuItem): prevent to dimiss menu on scroll inside menu

* fix(MenuItem): add changelog

* Update packages/fluentui/react-northstar/src/components/Menu/MenuItem.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.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.

6 participants