[Carousel] Lack of tabindex on cmp-carousel__indicator

[rogemus]

Bug Report

Current Behavior

• Right now the user is unable to navigate to cmp-carousel__indicator using only the keyboard.
• No corresponding slide index on the indicators and because of that is required additional js logic to change slide by clicking on the indicator.

Expected behavior/code

• tabindex on .cmp-carousel__indicator
• corresponding slide index on .cmp-carousel__indicator as data attribute

Environment

• Core Components version 2.3.3-SNAPSHOT

Possible Solution

<li data-sly-repeat.item="${carousel.items}"
    role="tab"
    tabindex="0"
    data-cmp-slide-index="${itemList.index}"
    class="cmp-carousel__indicator${itemList.first ? ' cmp-carousel__indicator--active' : ''}"
    data-cmp-hook-carousel="indicator">
    ${item.title}
</li>

[richardhand]

@rogemus - Thanks for the report. I'm assuming here that you aren't using the default client library provided with the component core.wcm.components.carousel.v1 [0] and instead would like to use a 3rd party widget? If you have any insight into why you decided to take that approach, we would love to get your feedback.

Right now the user is unable to navigate to cmp-carousel__indicator using only the keyboard.

The Carousel (and Tabs) JS widgets were designed to manage tabindex such that only one indicator may be tabbed to tabindex=0, all others are not tabbable tabindex=-1. Navigation between indicators is handled with left (up), right (down) keys, rather than tab keys. This means a user can tab between the main actions of the Carousel easily (left action, right action and indicators).

This seems to be incompatible with the way your 3rd party widget expects the accessibility to work. In this case, I would suggest you add a tab index in your proxy component template to fit your usage.

I have also created a pull request [1] that sets tabindex and aria-selected attributes in the template to setup the initial state and to clarify the default accessibility behaviour.

No corresponding slide index on the indicators and because of that is required additional js logic to change slide by clicking on the indicator.

It should be fairly simple one-liner to add an indicator index check if you have the htmlElement in your click handler [].indexOf.call(element.parentNode.children, element);. As reading an additional data attribute doesn't provide much more convenience (although probably slightly more performant), I think I would rather not expose one.

[0] - https://github.com/adobe/aem-core-wcm-components/blob/master/content/src/content/jcr_root/apps/core/wcm/components/carousel/v1/carousel/clientlibs/site [1] - https://github.com/adobe/aem-core-wcm-components/pull/575

[rogemus]

hi, thx you for your quick replay ;) we want a user to be able to focus on .cmp-carousel__indicator and then using tab to go to next indicator.

As far as I know, this is a recommended behavior according to https://www.w3.org/WAI/tutorials/carousels/functionality/

data-cmp-slide-index

I think adding this prop will be beneficial to performance. In order to find the index of the corresponding slide, we could just read prop value instead of traversing dom.

[richardhand]

Hi @rogemus, I don't see any recommendations in the link you provided [0] regards managing tabindex or keyboard interaction. In our default widget, we follow the tab list (carousel indicators are essentially a tab list) keyboard interaction recommendations at [1]. I think both approaches are common, but we decided to follow those guidelines as they allow for quick tabbing to tab panel content and the rest of the document more efficiently.

If you would like a separate behaviour, then I would suggest ensuring your 3rd party widget manages the tabindex of the indicators as expected.

data-cmp-slide-index

The recommendation in the carousel functionality document seems to rather be that the index as a data attribute is useful for accessibility reasons, but I'm not sure if that's the case or how that would manifest in a screen reader. Regarding the performance improvement, I'm still not sure it's anything of concern (maybe you have some stats?) - the items can easily be cached on first read and their index looked up - I would still prefer to keep the markup as lean as possible and not add any data attributes unless they are absolutely necessary.

[0] https://www.w3.org/WAI/tutorials/carousels/functionality/ [1] https://www.w3.org/TR/2010/WD-wai-aria-practices-20100916/#tabpanel

[rogemus]

Sadly we can't treat tabs in tab component and these indicators as the same thing. Because there is no content inside of cmp-carousel__indicator. If the user is focused on this element and press tab he/she should be focused on the next cmp-carousel__indicator. In the case of tabs component on next tab keypress, user should be focused on the content inside of tab component.

https://www.w3.org/WAI/tutorials/carousels/functionality/#add-navigation-buttons In their example, you can see that they used <button> for inside <li> and thanks to that they don't need to apply any tabindex. But if you tab thought that navigation example you can see that focus jumps from one item to next etc.

[richardhand]

@rogemus,

If the user is focused on this element and press tab he/she should be focused on the next cmp-carousel__indicator

Where do you get this should definition? The Carousel tutorial actually recommends using tabindex=-1 if using a list item without content - and to manage the tabindex/focus via javascript, which is what we do in the Carousel. As I say, I have seen both approaches used in the wild, and given that the Indicators are of role tablist we decided to use the associated accessibility spec and to be consistent with the Tabs Core Component.

[richardhand]

Closing this as the out of the box Carousel widget already augments the markup with a managed tabindex that follows the tablist design pattern. For an alternative behaviour the markup can be adjusted to add tabindex=0 to each indicator to allow tabbing between them.