[Navigation][Tabs][Carousel][Button] There's no possibility to add labels to the components

[jakublamprecht]

Bug Report

Current Behavior
Navigation component does not allow authors to specify labels for navigation, tabs and carousel (useful if more than one navigation component is present on the page) as W3C suggests here:

https://www.w3.org/WAI/tutorials/menus/structure/#label-menus
https://www.w3.org/TR/wai-aria-practices/#tabpanel

Using aria-label would probably require less work, since using aria-labeledby would come with adding possibility to choose heading level for the label.

Expected behavior/code
Authors can specify labels for navigation, tabs, carousel and button components.

Environment

• Core Components version 2.3.0

[richardhand]

@jakublamprecht - thanks, this one would require an author defined label per navigation so we would have to put some thought into if and how we would like this to be defined in the dialog.

[richardhand]

@gabrielwalt, I see two ways of supporting this, the second being a bit more involved. What do you think?

Option 1: Allow only a label to be defined by an author for accessibility reasons.

Edit Dialog

Field

• Type: Text
• Title: Label
• Description: Label that describes the navigation and distinguishes it from other navigations on the page, for accessibility reasons.

Markup

<nav aria-label="Main Navigation">
  …
</nav>

Option 2: Allow a title to be defined for a navigation, which is displayed on the page and can optionally be displayed as an accessibility label.

Edit Dialog

Field 1

• Type: Text
• Title: Title
• Description: Navigation Title

Field 2

• Type: Select
• Title: Title Element
• Options: H1-H6

Field 3

• Type: Checkbox
• Title: Display title as accessibility label
• Description: If checked, the navigation title is used as an accessibility label only.
• Default: unchecked

Markup

<nav aria-labelledby="myNavigationId-title">
  <h3 id="myNavigationId-title">Main Navigation</h3>
    …
</nav>

or, if Display title as accessibility label is checked:

<nav aria-label="Main Navigation">
  …
</nav>

[jakublamprecht]

@richardhand I changed the title and description of this issue to also include the tabs and carousel components, since it adresses the same issue in all those components.

[richardhand]

Thanks @jakublamprecht - I guess these labels should also be unique (for Carousel and Tabs) per component instance and therefore configurable by an author, rather than a hard-coded value as is the case with aria-label="breadcrumb"? Makes me wonder if we should introduce an accessibility tab per-component dialog (when it's needed).

[jakublamprecht]

From what I've seen yes, they should be authorable, since they're supposed to describe the purpose of the tablist that the label is added too. I guess that any generic information will already be provided by roles/landmarks.

[jakublamprecht]

@richardhand I've found some additional issues regarding the carousel component. According to this:

https://www.w3.org/TR/wai-aria-practices/#carousel

carousel component need to have role="region" and aria-roledescription attributes added. Aria-label is required not only for the carousel, but also for the indicators that implement the tabs pattern and also for the indicators (tabs) and slides (tabpanels).

These are the non-breaking changes.

I've found also, that there's an issue with order of elements in the markup - W3C says that the rotation control buttons (pause and play) are supposed to be the very first elements in tab order, which would require them to be moved in HTL.

Also the pattern mentions that when carousel elements (other than the rotation control buttons) are focused, the carousel should be paused until user clicks the play button. This is not implemented. This would require some additional JavaScript and probably moving those buttons out of "cmp_carousel__content" like this:

<div class="cmp-carousel">
  <div class="cmp-carousel__actions cmp-carousel__actions--rotation-control>
    <play/pause buttons here>
  </div>
  <div class="cmp-carousel__content">
    <div class="cmp-carousel__actions cmp-carousel__actions--slide-control>
      <next/prev buttons here>
    </div>
    ...rest of the markup
  </div>
</div>

This markup structure would help us in detecting whether focus is inside the carousel content and the carousel should be paused (one eventListener would be enough here), or focus is outside the content (e.g on the play/pause button) and we do not have to pause the carousel.

I will provide the fixes for non-breaking changes in this PR and create a separate issue for the other changes, since they probably would require a new version of component to be released.

[richardhand]

@jakublamprecht,

carousel component need to have role="region" and aria-roledescription attributes added.

I think the group role is a better default as you don't necessarily want the Carousel to appear in a page summary or table of contents. We should also add this from the specification for each item: "Each slide container has role group with the property aria-roledescription set to slide."

aria-label is required not only for the carousel, but also for the indicators that implement the tabs pattern and also for the indicators (tabs) and slides (tabpanels).

makes sense to add this.

I will provide the fixes for non-breaking changes in this PR and create a separate issue for the other changes, since they probably would require a new version of component to be released.

Thanks! We can continue the discussion on the other points in the new issue.

[jakublamprecht]

@richardhand That makes sense to change to role to group instead of region.

We should also add this from the specification for each item: "Each slide container has role group with the property aria-roledescription set to slide."

I think this is not necessary, since in the carousel slides have role set to tabpanel and indicators have role tab.

[richardhand]

I think this is not necessary, since in the carousel slides have role set to tabpanel and indicators have role tab.

Right, thought we might still benefit from the role description but seems it's not recommended.

[richardhand]

Code is merged to master and will be available in the upcoming 2.6.0 Release.