OOUI IndexLayout markup

How does OOUI handle this? The OOUI IndexLayout widget was the inspiration for Codex Tabs component. It uses the following markup structure (I've removed classes for clarity here):

<div class="...">
    <!-- Tabs header element -->
    <div class="oo-ui-menuLayout-menu" aria-hidden="false">
        <div class="...">
            <!-- Tab list -->
            <div role="tablist" aria-multiselectable="false" tabindex="0" aria-activedescendant="ooui-575">
                <!-- Individual Tab header item -->
                <div aria-selected="true" tabindex="-1" role="tab" id="ooui-575" aria-controls="ooui-576">
                    <span class="oo-ui-labelElement-label">One tab</span>
                </div>
                <div aria-selected="false" tabindex="-1" role="tab" id="ooui-577" aria-controls="ooui-578">
                    <span class="oo-ui-labelElement-label">Two tab</span>
                </div>
                <div aria-selected="false" tabindex="-1" role="tab" id="ooui-579" aria-controls="ooui-580">
                    <span class="oo-ui-labelElement-label">Three tab</span>
                </div>
                <div aria-selected="false" tabindex="-1" role="tab" id="ooui-581" aria-controls="ooui-582">
                    <span class="oo-ui-labelElement-label">Four tab</span>
                </div>
            </div>
        </div>
    </div>
    <!-- Tabs content element -->
    <div class="oo-ui-menuLayout-content">
        <div class="...">
            <!-- Individual tab panel -->
            <div role="tabpanel" aria-labelledby="ooui-575" id="ooui-576">
                <p>First tab</p>
            </div>
            <div role="tabpanel" aria-labelledby="ooui-577" id="ooui-578" aria-hidden="true" hidden="until-found">
                <p>Second tab</p>
            </div>
            <div role="tabpanel" aria-labelledby="ooui-579" id="ooui-580" aria-hidden="true" hidden="until-found">
                <p>Third tab</p>
            </div>
            <div role="tabpanel" aria-labelledby="ooui-581" id="ooui-582" aria-hidden="true" hidden="until-found">
                <p>Fourth tab</p>
            </div>
        </div>
    </div>
</div>

Takeaways:

• role="presentation" is not used anywhere here, leading me to think that this role is not necessary in the Codex tabs markup
• role="tab" gets applied directly to the child elements of the tablist element; in OOUI both are <div>s. I know that there was some concern about applying role="tab" to an element that was not a button or a link (<li> or <label> are the ones we were debating using in Codex). But OOUI just applies the relevant roles to <div> elements. I think that Codex Tabs should probably just apply role="tab" directly to the <li> children of the <ul role="tablist"> element and leave the markup inside of the <li> items alone.
• aria-controls – it looks like we are not currently using aria-controls attributes on the items inside the tablist in the Codex tabs implementation. I assume we probably want to start doing that - in that case we'd want to add an aria-controls attribute to each <li> item in the tabs header row with a value matching the ID of the relevant Tab component.
• hidden="until-found" — this is something I wasn't aware of until @Volker_E brought it to my attention. OOUI IndexLayout uses this attribute (you can see the Phab task outlining the reasoning why here: T313804 and the patch here). However it looks like this attribute also lead to some bugs (see the last comment in Gerrit for more details). So if we do introduce hidden="until-found", we may also need to make other changes in order for things to work properly.

Our use of role="presentation" and role="tab" comes from this article, which the original Vue implementation of the Tabs component is based on. The article explains that it's needed to prevent screen readers from announcing the <li> elements as list items. I think moving the role="tab" attribute from the <a>s to the <li>s could address that, but the MDN docs say that role="tab" should be set on "an interactive element inside a tablist". The <li> is not interactive, but the <a> is, so the existing structure makes sense to me.

However, the MDN documentation recommends, as a best practice, using <button role="tab"> elements for the tab. The examples on that page use a <div role="tablist"> as the parent of the <button>s, and there are no intermediate elements. If we moved to this structure, we would have to style the <button> elements to not appear button-like, but in exchange we would be able to get rid of the styles unsetting/overriding the list styling on the <ul> and <li> elements. It could also allow us to bring the CSS-only markup a little closer to the Vue markup, by using a <button type="submit"> for the tab label in the CSS-only version (this would allow us to merge the current input+label pair into one element).

(This leads me to think: is there a reason the CSS-only tabs use forms rather than links? I'm sure there was a reason to diverge so much from the Vue markup, but I can't find it in the Gerrit or Phabricator discussions about the CSS-only tabs implementation. If we wanted to use links for the CSS-only tabs, then I think the current ul+li+a structure makes sense, but if we're gonna use form elements in the CSS-only tabs anyway, then I think switching to div+button for both CSS-only and Vue would make more sense.)

Another best practice the MDN docs explicitly recommend, and which the Inclusive Components article also recommends, is setting tabindex="0" on the active tab and tabindex="-1" on the inactive tabs. We don't do this, we instead set tabindex="-1" on all tabs, and set tabindex="0" on the tabs header (the parent of the tablist that also contains the scroller buttons). This is because of our keyboard handling (we apply a different visual treatment to tabs focused through the Tab key than those focused by clicking them), but we should look into whether we can achieve this effect while still following this best practice.

Relatedly, the MDN docs for both role="tab" and role="tabpanel" recommend that when the Tab key is pressed when the role="tab" element is focused, the focus should go to the role="tabpanel" element, which is why the MDN example sets tabindex="0" on all the tabpanels. We should consider doing this as well (right now we only focus the tabpanel when the user presses the down arrow, but that seems non-standard).

Re aria-controls I think you're right that we should add that.

The MDN documentation also appears to recommend using hidden rather than aria-hidden on the elements with role="tabpanel" (the tab contents). Making this change would keep the door open to using hidden="until-found" in the future. I think we should file a separate task to consider how and where to use `hidden="until-found", because there are a lot of shared considerations between the Tabs and Accordion components.

In summary, I propose we:

• Change <ul role="tablist"> to <div role="tablist">
• Change <li role="presentation"><a role="tab"> to <button role="tab"> and update styles accordingly
• In the CSS-only markup, change <input type="submit" name="tab" value="xyz"><label role="tab"> to <button role="tab" type="submit" name="tab" value="xyz">
• Add aria-controls attribute to the role="tab" elements
• Remove the aria-hidden attribute on the <section> elements, and add the hidden attribute instead (except for the one that is currently visible)
• Set tabindex="0" on the <section> elements, and remove the down arrow behavior
• Consider setting tabindex="0" on the active tab, if we can do that while keeping the visual distinction between tabs focused through clicking and those focused through the Tab key

Based on today's engineering discussion, it sounds like we're in agreement that the best way to move forward here is to stage examples of both approaches and test them using a screen-reader browser extension.

I'll update the acceptance criteria accordingly.

Change 933995 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[design/codex@main] [WIP, DNM] Tabs: Re-work markup for accessibility

https://gerrit.wikimedia.org/r/933995

I'm still working on this, but I wanted to post a couple of initial findings.

• The Codex Tabs component makes extensive use of [role='tab'] as a CSS selector, so changing around the ARIA markup will also mean that the styles (which are quite complex) may need to be significantly re-worked
• The current state of the Tabs component definitely leaves some things to be desired in terms of accessibility when compared with the equivalent widget in OOUI. Below is an example screen reader test (using the ScreenReader extension for Chrome, AKA ChromeVox).

OOUI Tabs screen reader example

Codex tabs screen reader example

The Codex tabs fail to provide updated narration in the screen reader when the user navigates between tabs via keyboard, only providing announcements when clicked; the OOUI tabs widget provides correct narration regardless of how the user switches tabs.

I wonder if that's because the tab is not focused at that time. Codex uses a trick where, for keyboard navigation, the parent div is focused instead, so that we can style the fake-focused tab differently. Maybe we should abandon that, and instead rely on :focus-visible.

Change 934626 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[design/codex@main] [WIP] Tabs: Rework tablist for better accessibility

https://gerrit.wikimedia.org/r/934626

After studying the W3C guidelines for how to represent the Tabs UI pattern in an accessible way, I think that we have a basic outline of a solution:

• Tab items in the list should be <button> elements, not links – this is true for both the CSS-only and the JS-enabled versions of the component
• Our current focus handling code will likely need to change, but we can visually keep things looking the same with minor style changes.

An initial patch demonstrating this approach is available here: https://gerrit.wikimedia.org/r/934626, and you can see a live demo here: https://934626--wikimedia-codex.netlify.app/components/demos/tabs.html#demos

Since the scope of this task was to decide how we want to proceed, I wonder if we should consider it closed (I think we're all in agreement that this is the correct solution once we work out a few remaining issues), and if we should file a new task for actually making the Tabs component fully accessable in accordance with W3C guidelines. @CCiufo-WMF thoughts? The patch can be retained and improved of course.

Since the scope of this task was to decide how we want to proceed, I wonder if we should consider it closed (I think we're all in agreement that this is the correct solution once we work out a few remaining issues), and if we should file a new task for actually making the Tabs component fully accessable in accordance with W3C guidelines. @CCiufo-WMF thoughts? The patch can be retained and improved of course.

Are you suggesting closing this task once the patch is merged, or closing it now and opening a new task an attaching your open patch? I think it would make sense to do the latter.

Change 934626 abandoned by Eric Gardner:

[design/codex@main] [WIP] Tabs: Rework tablist for better accessibility

Reason:

Abandoned in favor of https://gerrit.wikimedia.org/r/c/design/codex/+/941530

https://gerrit.wikimedia.org/r/934626

Change 933995 abandoned by Eric Gardner:

[design/codex@main] [WIP, DNM] Tabs: Re-work markup for accessibility

Reason:

See https://gerrit.wikimedia.org/r/c/design/codex/+/941530

https://gerrit.wikimedia.org/r/933995