fix(tabs): Improve accessibility - FRONT-3661 #2576
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,8 @@ | |
| {% set _css_class = 'ecl-tabs' %} | ||
| {% set _extra_attributes = [ | ||
| 'data-ecl-tabs="true"', | ||
| 'data-ecl-auto-init="Tabs"', | ||
| 'role="tablist"']|join(' ') | ||
| %} | ||
|
|
||
| {# Internal logic - Process properties #} | ||
|
|
@@ -62,24 +63,35 @@ | |
| <nav class="{{ _css_class }}"{{ _extra_attributes|raw }}> | ||
| <div class="ecl-tabs__container"> | ||
| <ul class="ecl-tabs__list"> | ||
| {% for _item in _items %} | ||
| {% if _item.path is not empty %} | ||
| <li class="ecl-tabs__item"> | ||
| {% set _item_extra_classes = 'ecl-tabs__link' %} | ||
| {% set _item_extra_attributes = [{ name: 'role', value: 'tab' }] %} | ||
| {% if loop.index != 1 %} | ||
| {% set _item_extra_attributes = _item_extra_attributes|merge([{ name: 'tabindex', value: '-1' }]) %} | ||
| {% endif %} | ||
| {% if _item.is_current is defined and _item.is_current is not empty %} | ||
| {% set _item_extra_classes = _item_extra_classes ~ ' ecl-tabs__link--active' %} | ||
| {% set _item_extra_attributes = _item_extra_attributes|merge([{ name: 'aria-selected', value: 'true' }]) %} | ||
| {% else %} | ||
| {% set _item_extra_attributes = _item_extra_attributes|merge([{ name: 'aria-selected', value: 'false' }]) %} | ||
| {% endif %} | ||
| {% if _menu is not empty %} | ||
| {% set extra_attributes = extra_attributes|merge([{ name: 'data-ecl-has-menu' }]) %} | ||
| {% endif %} | ||
| {% include '@ecl/link/link.html.twig' with { | ||
| link: { | ||
| label: _item.label, | ||
| path: _item.path, | ||
| }, | ||
| extra_classes: _item_extra_classes, | ||
| extra_attributes: _item_extra_attributes, | ||
| } only %} | ||
| </li> | ||
| {% endif %} | ||
| {% endfor %} | ||
| <li class="ecl-tabs__item ecl-tabs__item--more"> | ||
| {% include '@ecl/button/button.html.twig' with { | ||
| label: _more_label, | ||
| variant: 'secondary', | ||
|
|
@@ -92,6 +104,7 @@ | |
| transform: 'rotate-180' | ||
| }, | ||
| extra_classes: 'ecl-tabs__toggle', | ||
| extra_attributes: [{ name: 'tabindex', value: '-1' }], | ||
|
There was a problem hiding this comment. i understand that we are introducing navigation through keyboard arrows, but why disabling the standard navigation via Tab..? It feels really weird to access the first tab with a Tab and then being moved out of the tabs typing again Tab. This way the user really have to know that the only way of navigating our tabs would be using arrows. There was a problem hiding this comment. This is basically the request, to only be able to navigate with arrows and thus be able to go to the content more quickly without having to tab all tabs. https://www.w3.org/WAI/ARIA/apg/example-index/tabs/tabs-manual.html There was a problem hiding this comment. i don't understand, really.. There was a problem hiding this comment. We can discuss this on the next weekly if you want ;) |
||
| } only %} | ||
| </li> | ||
| </ul> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i see that we normally define our internal variable for attributes as a string that gets populated by looping into the extra_attributes param, and it's ok since it's going to be rendered as a string. But when we have "internal" attributes to set, it would feel natural to merge these values into the extra_attributes array and get the _extra_attributes string as the result of the internal and external attributes processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, I properly understand the issue here, we are doing same in expandable and news-ticker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not an issue, just that we are requesting extra attributes as objects with name and value for then processing them to generate a string. if we are about to create internal attributes i would merge these with the extra_attributes array in order to process them in one place consistently. I know that we have other use cases like this, but that doesn't make it right. it was a suggestion, if you like it like this, keep it that way.