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(ToolbarMenuItem): missing aria-haspopup when it has submenu #17646

Merged
merged 4 commits into from
Mar 31, 2021

Conversation

YuanboXue-Amber
Copy link
Contributor

Pull request checklist

Description of changes

Before:
Notice item 'Play' has submenu, but its DOM does not have aria-haspopup
image

After:
image

Focus areas to test

(optional)

/** Indicated if menu item has submenu. TODO: fix types for all behaviours */
menu?: any;
/** Indicated if menu item has submenu */
hasMenu?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

would it make more sense just to intersect `Pick<MenuItemBehaviourProps, 'hasMenu'> ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a great suggestion, after checking MenuItemBehavior, I changed toolbarMenuItemBehaviorProp to:

export type ToolbarMenuItemBehaviorProps = Omit<MenuItemBehaviorProps, 'vertical'>;

@size-auditor
Copy link

size-auditor bot commented Mar 31, 2021

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-northstar-Toolbar 192.3 kB 192.305 kB ExceedsBaseline     5 bytes

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

Baseline commit: 9c4f38b05c23051bdb7e8eb453b72faf3401d9cc (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 31, 2021

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 918 906 5000
BaseButton mount 888 904 5000
Breadcrumb mount 43077 43118 5000
ButtonNext mount 542 562 5000
Checkbox mount 1531 1532 5000
CheckboxBase mount 1283 1256 5000
ChoiceGroup mount 4640 4590 5000
ComboBox mount 956 961 1000
CommandBar mount 9980 10055 1000
ContextualMenu mount 6144 6157 1000
DefaultButton mount 1100 1101 5000
DetailsRow mount 3581 3575 5000
DetailsRowFast mount 3593 3588 5000
DetailsRowNoStyles mount 3467 3425 5000
Dialog mount 1443 1466 1000
DocumentCardTitle mount 1828 1800 1000
Dropdown mount 3265 3245 5000
FocusTrapZone mount 1809 1808 5000
FocusZone mount 1797 1812 5000
IconButton mount 1697 1723 5000
Label mount 320 332 5000
Layer mount 1772 1781 5000
Link mount 452 465 5000
MakeStyles mount 1798 1827 50000
MenuButton mount 1465 1472 5000
MessageBar mount 1983 1992 5000
Nav mount 3254 3185 1000
OverflowSet mount 1048 1014 5000
Panel mount 1394 1395 1000
Persona mount 817 796 1000
Pivot mount 1364 1377 1000
PrimaryButton mount 1271 1251 5000
Rating mount 7501 7609 5000
SearchBox mount 1291 1298 5000
Shimmer mount 2491 2457 5000
Slider mount 1951 1942 5000
SpinButton mount 4960 4874 5000
Spinner mount 412 425 5000
SplitButton mount 3061 3093 5000
Stack mount 493 517 5000
StackWithIntrinsicChildren mount 1513 1506 5000
StackWithTextChildren mount 4417 4438 5000
SwatchColorPicker mount 9996 9964 5000
Tabs mount 1354 1387 1000
TagPicker mount 2780 2739 5000
TeachingBubble mount 11334 11398 5000
Text mount 407 399 5000
TextField mount 1334 1339 5000
ThemeProvider mount 1182 1180 5000
ThemeProvider virtual-rerender 596 619 5000
ThemeProviderNext mount 15753 15749 5000
Toggle mount 783 798 5000
buttonNative mount 114 113 5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
AttachmentMinimalPerf.default 165 164 1.01:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.17 0.47 0.36:1 2000 332
🦄 Button.Fluent 0.11 0.19 0.58:1 5000 542
🔧 Checkbox.Fluent 0.63 0.34 1.85:1 1000 630
🎯 Dialog.Fluent 0.15 0.21 0.71:1 5000 730
🔧 Dropdown.Fluent 3.06 0.4 7.65:1 1000 3055
🔧 Icon.Fluent 0.12 0.06 2:1 5000 589
🦄 Image.Fluent 0.08 0.13 0.62:1 5000 388
🔧 Slider.Fluent 1.59 0.46 3.46:1 1000 1587
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 357
🦄 Tooltip.Fluent 0.14 0.89 0.16:1 5000 706

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
CardMinimalPerf.default 576 543 1.06:1
DividerMinimalPerf.default 379 359 1.06:1
LabelMinimalPerf.default 402 382 1.05:1
ListWith60ListItems.default 678 648 1.05:1
TableMinimalPerf.default 413 395 1.05:1
TreeWith60ListItems.default 187 178 1.05:1
VideoMinimalPerf.default 628 599 1.05:1
AlertMinimalPerf.default 286 275 1.04:1
ReactionMinimalPerf.default 385 370 1.04:1
TableManyItemsPerf.default 1922 1850 1.04:1
AvatarMinimalPerf.default 207 200 1.03:1
CarouselMinimalPerf.default 486 470 1.03:1
FlexMinimalPerf.default 301 293 1.03:1
FormMinimalPerf.default 410 398 1.03:1
ImageMinimalPerf.default 387 375 1.03:1
LoaderMinimalPerf.default 712 690 1.03:1
RadioGroupMinimalPerf.default 440 426 1.03:1
Avatar.Fluent 332 322 1.03:1
ButtonMinimalPerf.default 177 173 1.02:1
ButtonUseCssNestingPerf.default 1072 1055 1.02:1
DropdownManyItemsPerf.default 698 685 1.02:1
GridMinimalPerf.default 352 344 1.02:1
ListCommonPerf.default 629 618 1.02:1
RefMinimalPerf.default 253 248 1.02:1
TextMinimalPerf.default 356 350 1.02:1
ToolbarMinimalPerf.default 947 927 1.02:1
TooltipMinimalPerf.default 971 952 1.02:1
Checkbox.Fluent 630 617 1.02:1
Image.Fluent 388 381 1.02:1
AnimationMinimalPerf.default 419 414 1.01:1
AttachmentSlotsPerf.default 1137 1126 1.01:1
DialogMinimalPerf.default 725 715 1.01:1
DropdownMinimalPerf.default 3053 3032 1.01:1
HeaderSlotsPerf.default 751 744 1.01:1
ProviderMergeThemesPerf.default 1667 1655 1.01:1
ProviderMinimalPerf.default 1003 995 1.01:1
StatusMinimalPerf.default 687 683 1.01:1
CustomToolbarPrototype.default 3798 3755 1.01:1
Button.Fluent 542 534 1.01:1
Dialog.Fluent 730 726 1.01:1
Text.Fluent 357 353 1.01:1
Tooltip.Fluent 706 701 1.01:1
ButtonUseCssPerf.default 805 804 1:1
ChatMinimalPerf.default 603 606 1:1
CheckboxMinimalPerf.default 2722 2722 1:1
DatepickerMinimalPerf.default 45201 45163 1:1
EmbedMinimalPerf.default 4135 4123 1:1
ItemLayoutMinimalPerf.default 1265 1261 1:1
MenuMinimalPerf.default 878 880 1:1
SkeletonMinimalPerf.default 354 353 1:1
SplitButtonMinimalPerf.default 3653 3667 1:1
TextAreaMinimalPerf.default 500 499 1:1
Dropdown.Fluent 3055 3058 1:1
Slider.Fluent 1587 1585 1:1
BoxMinimalPerf.default 357 362 0.99:1
ButtonOverridesMissPerf.default 1671 1685 0.99:1
ChatDuplicateMessagesPerf.default 294 296 0.99:1
ChatWithPopoverPerf.default 382 387 0.99:1
InputMinimalPerf.default 1250 1268 0.99:1
ListMinimalPerf.default 502 509 0.99:1
SegmentMinimalPerf.default 350 355 0.99:1
SliderMinimalPerf.default 1554 1575 0.99:1
TreeMinimalPerf.default 765 776 0.99:1
AccordionMinimalPerf.default 161 164 0.98:1
ButtonSlotsPerf.default 562 571 0.98:1
HeaderMinimalPerf.default 367 373 0.98:1
MenuButtonMinimalPerf.default 1529 1556 0.98:1
RosterPerf.default 1141 1161 0.98:1
PopupMinimalPerf.default 697 709 0.98:1
Icon.Fluent 589 604 0.98:1
ListNestedPerf.default 548 565 0.97:1
IconMinimalPerf.default 614 634 0.97:1
PortalMinimalPerf.default 173 180 0.96:1
LayoutMinimalPerf.default 374 392 0.95:1

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 31, 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 0f09074:

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

@YuanboXue-Amber YuanboXue-Amber enabled auto-merge (squash) March 31, 2021 22:05
@YuanboXue-Amber YuanboXue-Amber merged commit d274cf8 into microsoft:master Mar 31, 2021
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
…osoft#17646)

* fix(ToolbarMenuItem): missing aria-haspopup when it has submenu

* changelog

* improve typing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

react-northstar Toolbar menu item is missing aria-haspopup when it has a submenu
6 participants