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

ContextualMenu: focused div when menu is opened now has role = "menu" #17683

Merged
merged 9 commits into from
Apr 10, 2021

Conversation

TristanWatanabe
Copy link
Member

Pull request checklist

Description of changes

ContextualMenu: element that's focused when menu is opened now has role = "menu"

  • Moved role = menu, aria-label and aria-labelledby attributes to container div which is the element that gains focus when menu is opened.
  • Updated all elements in between container div and first menu item to have role = presentation.
  • Updated Button tests to account for role changes as element with attributes role='menu', aria-label and aria-labelledby were moved from onRenderMenuList to the container div.
  • Removed 1 onRenderMenuList unit test from ContextualMenu since role will be static as "presentation" moving forward.

@size-auditor
Copy link

size-auditor bot commented Apr 1, 2021

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-TeachingBubble 189.744 kB 189.712 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-ContextualMenu 141.698 kB 141.666 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-SwatchColorPicker 175.236 kB 175.204 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Button 179.631 kB 179.599 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Breadcrumb 184.304 kB 184.272 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-ButtonGrid 165.736 kB 165.704 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Dialog 195.094 kB 195.062 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-DocumentCard 199.861 kB 199.829 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-MessageBar 173.411 kB 173.379 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Dropdown 215.895 kB 215.863 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-SpinButton 176.5 kB 176.468 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Facepile 194.423 kB 194.391 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-FloatingPicker 225.093 kB 225.061 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-SelectedItemsList 214.672 kB 214.64 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Grid 165.736 kB 165.704 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-SearchBox 172.351 kB 172.319 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Pivot 173.621 kB 173.589 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Pickers 268.644 kB 268.612 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Panel 185.012 kB 184.98 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-CommandBar 185.836 kB 185.804 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-Nav 173.681 kB 173.649 kB BelowBaseline     -32 bytes
office-ui-fabric-react fluentui-react-ComboBox 229.469 kB 229.437 kB BelowBaseline     -32 bytes

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

Baseline commit: 8d233d903ed4622fa6537bb300f90f740cfd52b7 (build)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 1, 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 7605487:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Apr 2, 2021

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 948 918 5000
BaseButton mount 894 882 5000
Breadcrumb mount 44383 44784 5000
ButtonNext mount 568 567 5000
Checkbox mount 1540 1531 5000
CheckboxBase mount 1283 1264 5000
ChoiceGroup mount 4661 4693 5000
ComboBox mount 938 985 1000
CommandBar mount 10154 10121 1000
ContextualMenu mount 6245 6234 1000
DefaultButton mount 1112 1138 5000
DetailsRow mount 3581 3606 5000
DetailsRowFast mount 3599 3678 5000
DetailsRowNoStyles mount 3487 3465 5000
Dialog mount 1441 1426 1000
DocumentCardTitle mount 1830 1824 1000
Dropdown mount 3203 3192 5000
FocusTrapZone mount 1790 1783 5000
FocusZone mount 1784 1791 5000
IconButton mount 1725 1709 5000
Label mount 347 337 5000
Layer mount 1772 1779 5000
Link mount 452 459 5000
MakeStyles mount 1827 1827 50000
MenuButton mount 1435 1499 5000
MessageBar mount 2026 1997 5000
Nav mount 3251 3260 1000
OverflowSet mount 1023 1026 5000
Panel mount 1404 1423 1000
Persona mount 836 817 1000
Pivot mount 1402 1401 1000
PrimaryButton mount 1284 1276 5000
Rating mount 7522 7601 5000
SearchBox mount 1301 1300 5000
Shimmer mount 2515 2508 5000
Slider mount 1954 1977 5000
SpinButton mount 4961 4886 5000
Spinner mount 416 417 5000
SplitButton mount 3116 3181 5000
Stack mount 494 499 5000
StackWithIntrinsicChildren mount 1540 1542 5000
StackWithTextChildren mount 4527 4453 5000
SwatchColorPicker mount 10209 10141 5000
Tabs mount 1393 1399 1000
TagPicker mount 2872 2809 5000
TeachingBubble mount 11829 11860 5000
Text mount 416 410 5000
TextField mount 1353 1372 5000
ThemeProvider mount 1174 1172 5000
ThemeProvider virtual-rerender 604 598 5000
ThemeProviderNext mount 15969 16011 5000
Toggle mount 804 778 5000
buttonNative mount 118 114 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.17 0.49 0.35:1 2000 339
🦄 Button.Fluent 0.11 0.2 0.55:1 5000 537
🔧 Checkbox.Fluent 0.63 0.35 1.8:1 1000 627
🎯 Dialog.Fluent 0.15 0.21 0.71:1 5000 749
🔧 Dropdown.Fluent 3.14 0.4 7.85:1 1000 3138
🔧 Icon.Fluent 0.13 0.06 2.17:1 5000 637
🦄 Image.Fluent 0.08 0.13 0.62:1 5000 393
🔧 Slider.Fluent 1.56 0.46 3.39:1 1000 1561
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 370
🦄 Tooltip.Fluent 0.14 0.9 0.16:1 5000 704

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 181 165 1.1:1
TextMinimalPerf.default 377 345 1.09:1
LabelMinimalPerf.default 415 388 1.07:1
AvatarMinimalPerf.default 217 205 1.06:1
TextAreaMinimalPerf.default 503 475 1.06:1
VideoMinimalPerf.default 642 607 1.06:1
ReactionMinimalPerf.default 402 382 1.05:1
Image.Fluent 393 376 1.05:1
Text.Fluent 370 352 1.05:1
AccordionMinimalPerf.default 170 163 1.04:1
FormMinimalPerf.default 401 387 1.04:1
LoaderMinimalPerf.default 732 707 1.04:1
SegmentMinimalPerf.default 364 351 1.04:1
StatusMinimalPerf.default 707 679 1.04:1
Icon.Fluent 637 612 1.04:1
ButtonMinimalPerf.default 181 175 1.03:1
ChatDuplicateMessagesPerf.default 308 298 1.03:1
DropdownManyItemsPerf.default 691 673 1.03:1
FlexMinimalPerf.default 301 292 1.03:1
ImageMinimalPerf.default 397 386 1.03:1
TableManyItemsPerf.default 1927 1873 1.03:1
TreeWith60ListItems.default 195 190 1.03:1
Avatar.Fluent 339 330 1.03:1
Dropdown.Fluent 3138 3061 1.03:1
BoxMinimalPerf.default 371 365 1.02:1
DividerMinimalPerf.default 380 372 1.02:1
InputMinimalPerf.default 1300 1278 1.02:1
LayoutMinimalPerf.default 390 381 1.02:1
ListCommonPerf.default 649 637 1.02:1
ProviderMinimalPerf.default 997 976 1.02:1
RefMinimalPerf.default 258 252 1.02:1
TooltipMinimalPerf.default 968 949 1.02:1
Dialog.Fluent 749 735 1.02:1
CardMinimalPerf.default 560 557 1.01:1
CheckboxMinimalPerf.default 2755 2740 1.01:1
DropdownMinimalPerf.default 3071 3049 1.01:1
HeaderMinimalPerf.default 366 364 1.01:1
ListNestedPerf.default 566 560 1.01:1
MenuButtonMinimalPerf.default 1573 1551 1.01:1
ProviderMergeThemesPerf.default 1685 1675 1.01:1
AlertMinimalPerf.default 279 280 1:1
AnimationMinimalPerf.default 423 423 1:1
AttachmentSlotsPerf.default 1155 1150 1:1
ButtonOverridesMissPerf.default 1699 1704 1:1
ButtonSlotsPerf.default 559 560 1:1
ButtonUseCssNestingPerf.default 1078 1074 1:1
DatepickerMinimalPerf.default 45446 45309 1:1
EmbedMinimalPerf.default 4136 4128 1:1
ItemLayoutMinimalPerf.default 1257 1262 1:1
ListWith60ListItems.default 683 680 1:1
PopupMinimalPerf.default 717 714 1:1
SplitButtonMinimalPerf.default 3707 3701 1:1
ToolbarMinimalPerf.default 940 944 1:1
TreeMinimalPerf.default 797 795 1:1
ButtonUseCssPerf.default 799 810 0.99:1
HeaderSlotsPerf.default 764 770 0.99:1
MenuMinimalPerf.default 891 902 0.99:1
RosterPerf.default 1148 1154 0.99:1
RadioGroupMinimalPerf.default 432 437 0.99:1
SkeletonMinimalPerf.default 359 363 0.99:1
SliderMinimalPerf.default 1568 1584 0.99:1
CustomToolbarPrototype.default 3804 3842 0.99:1
Button.Fluent 537 543 0.99:1
Checkbox.Fluent 627 633 0.99:1
Slider.Fluent 1561 1571 0.99:1
Tooltip.Fluent 704 709 0.99:1
ChatMinimalPerf.default 620 632 0.98:1
ChatWithPopoverPerf.default 375 382 0.98:1
DialogMinimalPerf.default 728 745 0.98:1
ListMinimalPerf.default 508 525 0.97:1
TableMinimalPerf.default 405 416 0.97:1
CarouselMinimalPerf.default 465 486 0.96:1
GridMinimalPerf.default 333 348 0.96:1
PortalMinimalPerf.default 166 175 0.95:1
IconMinimalPerf.default 599 629 0.95:1

Copy link
Member

@khmakoto khmakoto left a comment

Choose a reason for hiding this comment

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

Seems like you need to update some snapshots but other than that this should be good to go 😄

@msft-fluent-ui-bot
Copy link
Collaborator

Hello @khmakoto!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msft-fluent-ui-bot) and give me an instruction to get started! Learn more here.

@msft-fluent-ui-bot msft-fluent-ui-bot merged commit 4e5a243 into microsoft:master Apr 10, 2021
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/theme-samples@v8.0.25 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react@v8.9.4 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-toggle@v1.0.0-beta.78 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-tabs@v1.0.0-beta.80 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-slider@v1.0.0-beta.78 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-monaco-editor@v1.0.26 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-experiments@v8.0.27 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-examples@v8.15.1 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-date-time@v8.0.25 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/react-charting@v5.0.28 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/azure-themes@v8.0.26 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/api-docs@v8.0.25 has been released which incorporates this pull request.:tada:

Handy links:

@TristanWatanabe TristanWatanabe deleted the 17628 branch April 12, 2021 16:03
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request May 5, 2021
…microsoft#17683)

#### Pull request checklist

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

#### Description of changes

ContextualMenu: element that's focused when menu is opened now has role = "menu"

- Moved `role = menu`, `aria-label` and `aria-labelledby` attributes to container div which is the element that gains focus when menu is opened.
- Updated all elements in between container div and first menu item to have `role = presentation`.
- Updated Button tests to account for role changes as element with attributes `role='menu'`, `aria-label` and `aria-labelledby` were moved from `onRenderMenuList` to the container div.
- Removed 1 `onRenderMenuList` unit test from `ContextualMenu` since `role` will be static as `"presentation"` moving forward.
rzhouac pushed a commit to rzhouac/fluentui that referenced this pull request Jun 1, 2021
…microsoft#17683)

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

ContextualMenu: element that's focused when menu is opened now has role = "menu"

- Moved `role = menu`, `aria-label` and `aria-labelledby` attributes to container div which is the element that gains focus when menu is opened.
- Updated all elements in between container div and first menu item to have `role = presentation`.
- Updated Button tests to account for role changes as element with attributes `role='menu'`, `aria-label` and `aria-labelledby` were moved from `onRenderMenuList` to the container div.
- Removed 1 `onRenderMenuList` unit test from `ContextualMenu` since `role` will be static as `"presentation"` moving forward.
msft-fluent-ui-bot pushed a commit that referenced this pull request Jun 3, 2021
#### Pull request checklist

- [x] Addresses an existing issue: Fixes #17628 https://dev.azure.com/microsoftdesign/fluent-ui/_workitems/edit/10740
- [x] Include a change request file using `$ yarn change`

#### Description of changes

(give an overview)

#### Focus areas to test
contextual menu with nexted section
(optional)

Cherry pick of #17628 & #17683
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.

ContextualMenu aria-controls reference and focused element should be a menu
6 participants