Accessibility insights fixes

[srietkerk]

Mostly fixes microsoft/pxt-microbit#5424

The meta-viewport failed rule (and why I'm not fixing it)

The "mostly" lies in the fact that I left the meta-viewport failed rule. It's failing because we don't allow zooming on mobile. This is something that I can't nicely fix with our single-page app. It would be nice to allow for zooming on the homepage and disable zooming in the editor on mobile, but giving a pattern like that opens a lot of problems. A big one is that when I zoomed in on the home page and then switched over to the editor, the zoom would persist and just cause all sorts of problems. I could try setting the zoom styling when initially visiting the editor, but that property is not supported everywhere and not recommended by MDN. If we want to allow for zooming on mobile, we need to investigate a bit more and have a more sustainable solution.

It was also requested that I use this PR as documentation of sorts, so I'll take some time to explain the changes that I made and why.

The aria-required-children failed rule

This rule was failing because when a component is set to have role=menu || menubar, if that component has children, it is expecting that all of those children have role=menuitem. That fixed the problem in this case (the change seen in headerbar.tsx).

A note about menus and menuitems
I wanted to share, though, something that I learned through toying with this and the other "required-children" rule failures. Only specific HTML tags can have the role=menuitem. It is expected that if something has role=menuitem, it is an interactable element. So if you tried to apply role=menuitem directly to the img tag, then you would get another Accessibility Insights rule failure about "allowed aria roles". Allowed aria roles vary per tag and the context of the child.

The nested-interactive failed rule

This rule was failing because for the 'My Projects' heading, the whole div was set to have role=button and then we also had the 'View All' text have role=button. From a high level, it doesn't make sense to have a button nested in a button. Even though it's true that clicking on the whole heading would allow you to see all of your projects, screen readers and using keyboard navigation would only land on the 'View All' "button" since that's the thing that has the tabindex on it. Since that's the case, it doesn't really make sense to have the whole div have the role of button. By removing the role=button on the "My Projects" column div, the roles are assigned as expected. This is the change in webapp/src/projects.tsx.

Since I was already in Accessibility Insights mode, I also looked at the failures we had in the editor which is where the other changes come from.

The Project Naming Area

The fixes here are found in identity.tsx, sui.tsx, and editortoolbar.tsx. Pictured above is the "editortoolbar". That whole area is given the aria-role "region", which is just a way to group elements together that landmarks them as an important part of the webpage. Each region in this toolbar was a menu before -- the download button, the project stuff, and the undo/redo/zoom controls. The download and right most controls make sense to be grouped together. However, the naming area is not the same scenario. The input, save button, GitHub button, and (when logged in) the cloud save status don't really have a good grouping. They're already in the "editortoolbar" region, so to try to group them when they're functionally different is a stretch. That's why I removed the role=menu from the projectNameArea div. Because of that, I needed to change the save button's role (since we assign `role="menuitem" to editortoolbarbuttons when there is no role assigned.

The cloud save status also needed to get updated. When there is something on the page that a user cannot interact with, you can give that component role=presentation and aria will effectively ignore that component. When there is a component that is strictly visual, though, it should not have an aria-label because it will cause problems for screen readers. Any component that does not have an aria role or doesn't have an implicit aria role based on the html component should not have an aria label. That is where the updates in sui.tsx and identity.tsx come in. We were applying an aria-label to everything that had text or a title, and if that component doesn't have a role, it will cause problems.

The editor toggle

The editor toggle container has role=listbox. A listbox is expecting that all of its children have the role role=option. The editor dropdown was menuitem, and that is not a possible child of a listbox role parent. This failure was fixed by changing the dropdown's role to role=option.

[riknoll]

capitalization?

[srietkerk]

Preference?
between:

1. Editor Toggle
2. Editor toggle

[srietkerk]

Nvm, I chose "Editor toggle" based on what I saw other aria labels doing.