CommandBar does not have relevant role for its child element

[Harkesh030679]

CommandBar does not have relevant role for its child element in accessibility Insight tool

Environment Information

OS: Win 10
Browser: Edge/Chrome
Tool- Accessibility insight tool

Describe the issue:

Step 1- Open web site in chrome browser--https://developer.microsoft.com/en-us/fluentui#/controls/web/commandbar
Step 2- Run Accessibility Insight tool as per below snap and see the issue

Please provide a reproduction of the issue in a codepen:

Step 1- Open web site in chrome browser--https://developer.microsoft.com/en-us/fluentui#/controls/web/commandbar
Step 2- Run Accessibility Insight tool as per below snap and see the issue

Step 3-You can see child element has Role=Group, that should not be relevant role for menubar as suggested by Accessibility insight tool.

Actual behavior:

Child elements do not have relevant role in command bar

Expected behavior:

Child elements should have role=menuitem or any relevant in command bar

Documentation describing expected behavior

[paulgildea]

@Harkesh030679 Thanks for the thorough investigation! That's really appreciated!

@smhigley FYI

@joschect / @JustSlone Would this be considered a breaking change?

[JustSlone]

We generally consider DOM changes breaking. However, given this is causing issues in Accessibility Insights, and it's a pretty minor DOM change. I think we could take it. I am interested in @joschect 's opinion on the risk though.

However, one important note on this change. I'm a bit perplexed as to why the current structure is invalid.
This case, where we have a menubar, then multiple groups of menuitems, seems like exactly the intended use case for role="group". the MDN documentation even has a very similar example see Example 2 here

@smhigley or @jurokapsiar Do you have more context on this particular rule and what the right structure is here? It seems like the group role should be a valid change here grouping the menuitems.

This looks related to this change in axe-core (dequelabs/axe-core#2131) fixes this issue (dequelabs/axe-core#2076).

[miroslavstastny]

FYI - @kolaps33

[brandonthomas]

I'm wondering if the reason Accessibility Insights failed is because the group isn't around menuitemradio? The spec has the following in it:

Not 100% sure but this seems to imply the group role should only be around radio buttons within menu or menubar.

[brandonthomas]

This is also somewhat related to #11832 since the way the AT reads this out is counter to how it behaves. Would be nice to have these fixed all at once.

[JustSlone]

Right, @brandonthomas so this spec (link to menubar spec) suggests that in the menubar case the group role's purpose is specifically to group menuitemradios, and even tho it's seems logical to group menuitems in this way (and in our cause actually improves the semantics of the html) it's not allowed per the ARIA menubar spec.

@smhigley or @kolaps33 You all have more experience interpreting the ARIA spec, any thoughts here?

[JustSlone]

Also here's a good example ARIA menubar example. Plus another reference: https://act-rules.github.io/rules/bc4a75 (tho this doesn't discuss the presence of group).

Per the example, and the ARIA spec, it looks like we should either remove role="group" or change it to role="none". I also followed up in the PR I linked above to see if disallowing this html structure was intentional.

[smhigley]

This is fixed in the ARIA 1.2 spec, where group is a valid descendant of menubar when it contains any of the menuitem, menuitemcheckbox, or menuitemradio roles: https://w3c.github.io/aria/#menubar.

I'd say this is an axe issue, not a fluent one. Though, regardless of what the spec says, it could be worth investigating how group behaves within menus and menubars, then deciding whether to use it, and whether to log screen reader issues.

[brandonthomas]

@smhigley since that is in editor's draft, how should we take that? Can we bank that this will be updated and we're safe to resolve bugs against this issue? I don't want to assume we're compliant and turn out to not be.

[jgage176]

Right now it is still failing Accessibility Insights and showing up as an a11y failure, so I would vote for keeping it open until Accessibility Insights reflects any new standards, as until then will be be showing up on a11y test passes.

[JustSlone]

@joschect It seems like the temp fix would be to pass in role={'none'} on OverflowSet here (perhaps on both overflowsets actually) - CommandBar.base.tsx#L128

Does that seem right?

[JustSlone]

This was fixed in #15511 should be available in v7.147.0