Accessibility warnings with menu stories

[fredvisser]

The Storybook Accessibility add-on notes that we're using the <header> tag in a naive way.

Changing the <header> tag used in the story to…

<header aria-label="${x => x.text}" role="group">

…resolves the issues, but it be good to review if those settings are appropriate, if we should document that, or otherwise make it easy for clients to do the right thing.

The above story uses a statically defined and styled header to mimic the existing SystemLink user menu. If there are broader use cases, we should make this more robust to theming.

[rajsite]

I wonder if semantically it works better to encourage placement in an <aside> which is one of the placements discussed in the header accessibility:

<aside>
<header></header>
<section></section>
<footer></footer>
</aside>

Edit: originally had <main> in there but it sounds like main should be unique to the document and only used for the primary content.

We lose the ability to style as part of the drawer then which isn't a big deal necessarily. We could make one of those layout elements we've discussed that gives those sections and some styling. Edit: Or maybe we just always have the aside as part of the shadow root.

[fredvisser]

It looks like the <aside> tag is specific to sidebar content common in written articles. I'd argue that <section> is the catch-all that might be more appropriate.

[mollykreis]

Adding a <section> in the storybook example breaks all the styling. Should we support using and not using <section>s? Require all consumers to wrap the content in at least one <section>?

[rajsite]

@mollykreis the feature here is that when using certain elements within a menu it will apply some styles for you. Those would have to be updated to the new items chosen

[mollykreis]

@rajsite, my question was whether we were fine always requiring usages of the menu to wrap the menu's content in a <section>, or if we wanted to ensure the component worked and was styled as desired regardless of whether or not a <section> was added by the consumer.

That said, I'm not seeing a good way to style the elements within a <section> within the slot. Are we open to a completely different mechanism for distinguishing a header from a menu item, such as having the creator of the menu apply a class to the header items? Is there a better convention to follow in this case?

[mollykreis]

Some possible solutions:

1. Create a new element (e.g. <nimble-menu-header>) that can be placed within the menu to represent a header row. The menu styling can be modified to style the new element rather than <header>.

   • Pros:
     • the usage is clear and fairly error resistant
   • Cons:
     • requires the work to create a new element

2. Create styling for a class that can be set on anything that should be a header (e.g. nimble-menu-header). The menu styling can be modified to style an element with the new class than <header>.

   • Pros:
     • easy to implement
   • Cons:
     • potentially error prone; a consumer of nimble could use the class in unintended ways, such as applying the class to a menu item itself
     • I haven't seen this styling approach used within nimble (this may not be a problem)

3. Make no code changes within nimble. Document that a <header> added within the menu should specify role="group" aria-label="<unique_label>".

   • Pros:
     • no implementation required
   • Cons:
     • no way to ensure clients are doing this correctly

I lean towards option 1 because it seems the most robust, and I don't think the implementation cost of creating a very simple new element will be very high.

If we move forward with option 1 we can decide whether or not we want to create a wrapper for <hr> elements as well. At this point, I don't see a pressing reason to do that, unless we like the consistency of everything within the menu being a nimble element.

[rajsite]

Yea I agree that option 1 seems less likely for a user to mess up.

My general concern is that custom elements do have some overhead and if they don't provide unique functional value adding them unnecessarily is like a death by a 1000 cuts for performance. There have been questions like should would have a nimble-span or nimble-text and I'd advocate against that. Users can consume the theme aware tokens for font in a stylesheet instead of adding a large number of nimble-span custom elements to a page just to encapsulate a small bit of font style.

My guess is that there is likely some aria roles the element should be configuring on itself as a nimble-menu-header or something, also by existing as a custom element it's a clearer api for users as a analog with nimble-menu-item, and there are not likely to be a large number of instances of it on a page. So a custom element for the nimble-menu-header is probably a reasonable choice here too.

[rajsite]

Looking at the aria configuration for menuitems I wonder if makes for sense to make a menu group type element. ie

<nimble-menu>
  <nimble-menu-group>
  Edit Commands
    <nimble-menu-item>Cut</nimble-menu-item>
    <nimble-menu-item>Copy</nimble-menu--item>
    <nimble-menu-item>Paste</nimble-menu-item>
  </nimble-menu-group>
<hr>
<nimble-menu-item>Logout</nimble-menu-item>
</nimble-menu>

A similar pattern is shown in example 2 on the mdn aria role group page.

And nimble-menu-group would configure role="group" on itself and label itself with its text content (like we are doing for icon button accessibility)

Edit: From looking at the foundation menu source it doesn't look like it would handle groups that well: https://github.com/microsoft/fast/blob/6db5ed1a1fa14a1dfab17154fcf005c682fccace/packages/web-components/fast-foundation/src/menu/menu.ts#L319

Maybe we should request groups within a menu as a feature?