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.

Sign up for GitHub

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

Jump to bottom

CommandBar: fix incorrect aria role #15511

Merged
merged 4 commits into from
Oct 16, 2020
Merged

CommandBar: fix incorrect aria role #15511

merged 4 commits into from
Oct 16, 2020

Conversation

xugao
Copy link
Contributor

@xugao xugao commented Oct 14, 2020 •
edited
Loading

Pull request checklist

Description of changes

Fix having incorrectly set role="group" in between menubar and menuitem.

Focus areas to test

(optional)

@msft-github-bot msft-github-bot added the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Oct 14, 2020
@msft-github-bot
Copy link
Contributor

msft-github-bot commented Oct 14, 2020 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type 7.0 Ticks PR Ticks Iterations Status
Avatar mount 812 844 5000
BaseButton mount 925 970 5000
Breadcrumb mount 40115 39950 5000
ButtonNext mount 680 677 5000
Checkbox mount 1586 1597 5000
CheckboxBase mount 1358 1309 5000
ChoiceGroup mount 5019 5062 5000
ComboBox mount 931 972 1000
CommandBar mount 7516 7522 1000
ContextualMenu mount 12537 12426 1000
DefaultButton mount 1150 1176 5000
DetailsRow mount 3616 3777 5000
DetailsRowFast mount 3661 3692 5000
DetailsRowNoStyles mount 3444 3499 5000
Dialog mount 1494 1517 1000
DocumentCardTitle mount 1744 1760 1000
Dropdown mount 2608 2654 5000
FocusTrapZone mount 1695 1677 5000
FocusZone mount 1784 1743 5000
IconButton mount 1808 1809 5000
Label mount 326 339 5000
Layer mount 1987 1975 5000
Link mount 455 462 5000
MenuButton mount 1558 1571 5000
MessageBar mount 2067 2051 5000
Nav mount 3262 3303 1000
OverflowSet mount 1394 1409 5000
Panel mount 1457 1444 1000
Persona mount 859 864 1000
Pivot mount 1417 1461 1000
PrimaryButton mount 1292 1293 5000
Rating mount 7868 7855 5000
SearchBox mount 1351 1300 5000
Shimmer mount 2668 2643 5000
Slider mount 1518 1513 5000
SpinButton mount 5051 5085 5000
Spinner mount 395 391 5000
SplitButton mount 3251 3205 5000
Stack mount 523 507 5000
StackWithIntrinsicChildren mount 1668 1654 5000
StackWithTextChildren mount 4912 4928 5000
SwatchColorPicker mount 10517 10501 5000
TagPicker mount 2776 2799 5000
TeachingBubble mount 48424 48863 5000
Text mount 427 450 5000
TextField mount 1453 1471 5000
ThemeProvider mount 1674 1656 5000
ThemeProvider virtual-rerender 641 639 5000
Toggle mount 858 875 5000
button mount 112 104 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.47 0.49 0.96:1 2000 949
🦄 Button.Fluent 0.13 0.2 0.65:1 5000 644
🔧 Checkbox.Fluent 0.67 0.37 1.81:1 1000 667
🎯 Dialog.Fluent 0.18 0.24 0.75:1 5000 878
🔧 Dropdown.Fluent 3 0.51 5.88:1 1000 2997
🔧 Icon.Fluent 0.16 0.06 2.67:1 5000 787
🎯 Image.Fluent 0.09 0.12 0.75:1 5000 443
🔧 Slider.Fluent 1.62 0.37 4.38:1 1000 1616
🔧 Text.Fluent 0.08 0.03 2.67:1 5000 403
🦄 Tooltip.Fluent 0.12 14.59 0.01:1 5000 585

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AccordionMinimalPerf.default 178 0 Infinity:1
AlertMinimalPerf.default 341 0 Infinity:1
AnimationMinimalPerf.default 451 0 Infinity:1
AttachmentMinimalPerf.default 178 0 Infinity:1
AvatarMinimalPerf.default 537 0 Infinity:1
ButtonMinimalPerf.default 215 0 Infinity:1
ButtonOverridesMissPerf.default 1726 0 Infinity:1
ButtonSlotsPerf.default 636 0 Infinity:1
ButtonUseCssPerf.default 892 0 Infinity:1
CarouselMinimalPerf.default 477 0 Infinity:1
ChatDuplicateMessagesPerf.default 441 0 Infinity:1
ChatMinimalPerf.default 663 0 Infinity:1
ChatWithPopoverPerf.default 513 0 Infinity:1
CheckboxMinimalPerf.default 2910 0 Infinity:1
DialogMinimalPerf.default 859 0 Infinity:1
DividerMinimalPerf.default 402 0 Infinity:1
DropdownManyItemsPerf.default 813 0 Infinity:1
DropdownMinimalPerf.default 2961 0 Infinity:1
EmbedMinimalPerf.default 2051 0 Infinity:1
FlexMinimalPerf.default 326 0 Infinity:1
FormMinimalPerf.default 495 0 Infinity:1
GridMinimalPerf.default 377 0 Infinity:1
HeaderMinimalPerf.default 425 0 Infinity:1
HeaderSlotsPerf.default 882 0 Infinity:1
ImageMinimalPerf.default 422 0 Infinity:1
InputMinimalPerf.default 1375 0 Infinity:1
ItemLayoutMinimalPerf.default 1389 0 Infinity:1
LabelMinimalPerf.default 484 0 Infinity:1
LayoutMinimalPerf.default 458 0 Infinity:1
ListCommonPerf.default 726 0 Infinity:1
ListNestedPerf.default 644 0 Infinity:1
LoaderMinimalPerf.default 779 0 Infinity:1
MenuMinimalPerf.default 928 0 Infinity:1
MenuButtonMinimalPerf.default 1669 0 Infinity:1
PopupMinimalPerf.default 739 0 Infinity:1
PortalMinimalPerf.default 166 0 Infinity:1
ProviderMergeThemesPerf.default 2047 0 Infinity:1
ProviderMinimalPerf.default 1051 0 Infinity:1
RadioGroupMinimalPerf.default 459 0 Infinity:1
ReactionMinimalPerf.default 470 0 Infinity:1
RefMinimalPerf.default 248 0 Infinity:1
SegmentMinimalPerf.default 407 0 Infinity:1
SkeletonMinimalPerf.default 471 0 Infinity:1
SliderMinimalPerf.default 1635 0 Infinity:1
StatusMinimalPerf.default 803 0 Infinity:1
IconMinimalPerf.default 747 0 Infinity:1
TableManyItemsPerf.default 2384 0 Infinity:1
TableMinimalPerf.default 465 0 Infinity:1
TextMinimalPerf.default 397 0 Infinity:1
TextAreaMinimalPerf.default 548 0 Infinity:1
CustomToolbarPrototype.default 3951 0 Infinity:1
ToolbarMinimalPerf.default 1017 0 Infinity:1
TooltipMinimalPerf.default 865 0 Infinity:1
TreeMinimalPerf.default 938 0 Infinity:1
TreeWith60ListItems.default 209 0 Infinity:1
VideoMinimalPerf.default 693 0 Infinity:1
Avatar.Fluent 949 0 Infinity:1
Button.Fluent 644 0 Infinity:1
Checkbox.Fluent 667 0 Infinity:1
Dialog.Fluent 878 0 Infinity:1
Dropdown.Fluent 2997 0 Infinity:1
Slider.Fluent 1616 0 Infinity:1
Text.Fluent 403 0 Infinity:1
Tooltip.Fluent 585 0 Infinity:1
SplitButtonMinimalPerf.default 3936 1 3936:1
AttachmentSlotsPerf.default 1193 1 1193:1
ButtonUseCssNestingPerf.default 1158 1 1158:1
ListWith60ListItems.default 991 1 991:1
Icon.Fluent 787 1 787:1
CardMinimalPerf.default 616 1 616:1
ListMinimalPerf.default 518 1 518:1
Image.Fluent 443 1 443:1
BoxMinimalPerf.default 398 1 398:1

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 14, 2020 •
edited
Loading

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 8f309ec:

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

@size-auditor
Copy link

size-auditor bot commented Oct 14, 2020 •
edited
Loading

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react office-ui-fabric-react-CommandBar 194.354 kB 194.378 kB ExceedsBaseline     24 bytes

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

Baseline commit: 8828fdca589a3b4f9119798fb806163e29961a41 (build)

@khmakoto khmakoto mentioned this pull request Oct 15, 2020
Closed
40 tasks
@xugao xugao merged commit 37759e2 into microsoft:7.0 Oct 16, 2020
@msft-github-bot
Copy link
Contributor

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot
Copy link
Contributor

🎉[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-github-bot msft-github-bot mentioned this pull request Oct 16, 2020
Closed
@khmakoto khmakoto mentioned this pull request Oct 20, 2020
Merged
2 tasks
msft-github-bot pushed a commit that referenced this pull request Oct 20, 2020
@khmakoto
Command bar role (#15614)
07e4870 
<!--
!!!!!!! IMPORTANT !!!!!!!

Due to work we're currently doing to prepare master branch for our version 8 beta release,
please hold-off submitting the PR until around October 12 if it's not urgent.
If it is urgent, please submit the PR targeting the 7.0 branch.

This change does not apply to react-northstar contributors.

See #15222 for more details. Sorry for the inconvenience and short notice.
-->

#### Pull request checklist

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

#### Description of changes

_Cherry-pick of #15511._

_Original PR description:_

Fix having incorrectly set `role="group"` in between `menubar` and `menuitem`.

#### Focus areas to test

(optional)
@joschect joschect mentioned this pull request Oct 22, 2020
Closed
@JustSlone JustSlone mentioned this pull request Oct 23, 2020
Closed
@JustSlone JustSlone mentioned this pull request Oct 30, 2020
Closed
@xugao xugao removed the needs cherry-pick Temporary label for PRs which may need to be cherry-picked to master label Nov 19, 2020
@daramful daramful mentioned this pull request Nov 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@micahgodbolt micahgodbolt micahgodbolt approved these changes

@joschect joschect Awaiting requested review from joschect

Assignees

@tomi-msft tomi-msft

Labels
Area: Accessibility Component: CommandBar
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@xugao @msft-github-bot @micahgodbolt @tomi-msft