Accessible tabs (tablists, tabs, tabpanels) - #314
Conversation
ahamelers
changed the title
wparad
left a comment
wparad
left a comment
There was a problem hiding this comment.
Please start with a design not just code. I want to hear your design, what you considered and discarded and why this is the best solution. And how it compares to what everyone else is doing.
This is not just feedback for this PR, but every PR you create. We need to start with WHY and also why a specific implementation is the right one.
This is another example, and it is a rhetorical question:
Why are we using role="heading" instead of using
and
and why does render style focused need 4, but everything else is 5?
Implementations like this where we are hacking in the aria, should give us pause and consider both why we are doing it, and why the specific hack is the right one.
You told me in another PR that listing what was considered and discarded, i.e. was NOT in the PR, was "useless" to you. In this case, I haven't considered and discarded anything, because the tab and related roles and their functionality (examples and links to more examples in https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/tab_role) has been the recommended standard for accessibly handling tabbed content since at least 2014 https://www.w3.org/TR/2014/REC-wai-aria-20140320/roles#tab. Additionally, the point of these accessibility updates is that the existing design does not change. They are adding the screen reader and keyboard functionality that are missing from the current iteration of your project, without modifying the look and feel or the experience for users who are working visually and with a mouse (or by touch). |
Why? I think we need to consider what changing the design would mean in these places |
|
My goal is to make the existing design accessible. Do you not believe tabs are a good way to display this content? |
And my goal is to make sure the design we end up with is accessible and that it is maintainable. I want to understand from you the right way to do tabs, and then evaluate if we can port our needs over to tabs. There is no reason to keep the current implementation when we can keep the same UI / UX (minus the missing accessibility features) and use a better implementation. Don't assume the current implementation aka the design is current, assume the current UI and the UX design is. I think you might be conflating these two things. What I see in this PR looks like a hack attempting to avoid using the correct components to deliver the best experience for users and making it maintainable. Just like you migrated the button infrastructure over to using real buttons instead of And I want to help be part of the decision, right now I am missing the justification piece, or rather it feels like the justification is "we prefer to hack in accessibility without considering if there is a better alternative", I want to understand if we do consider alternatives, are any of them better? |
|
Okay, I think I now understand the question! To have a horizontal list of tabs related to content that all appear in the same space, there is no set of HTML native elements or components that would be able to replace the code for tabbed content. The aria roles for tabs/tablists/tabpanels are not a "hack", they are a way of making a visually functional implementation—a list of buttons that each control and hide and expose content—functional for screen readers, and creating an expected way of keyboard navigation that creates a single menu out of the buttons, while associating the tab content as the next tabbable element when one is selected. For a design that can perform content disclosure without aria roles or additional javascript (beyond polyfills), the only real option is the relatively new details and summary elements (https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/details). These would not be displayed in the same manner as a list of tabs, generally appearing in sequence vertically rather than horizontally, and would ultimately have an appearance more like expandable sections of a page on Wikipedia. Another alternative would be to turn each tab button into a heading instead, and expose all the hidden content so it is all immediately visible on screen, creating very large pages. I think this is not recommendable, just because that is a lot of information, and would probably require an additional level of navigation to be made available for users to get to the information they want quickly. In this way the tabbable content acts as a kind of navigation shortcut. |
|
I totally agree with what you are saying. It feels like what we should do here then is to create a new component, call it a tabContainer or something, which can take a list of tabs, and probably triggers a bunch handlers. Before we take the next step. Then we can encapsulate all the javascript in a single component and just reuse that component everywhere. To be transparent, I need to avoid us having the exact same javascript logic in multiple places for the exact same implementation. We have the same problem in multiple places, that means we should use the same component, we don't have a shared component, so I theorize we should. What would that component likely look like? Does that suggestion make sense? |
|
I modified the JavaScript to be applicable across all the things, and moved it into a single location in a previous commit. Please check the code! Making a single tabContainer component would likely also be doable, and was already something that should maybe have been indicated, considering the very repetitive elements, classes, and general structure. On the other hand, there's just enough variety in the code declaring which element is "selected" for each component to make it a bit of a pain, so I can see why it wasn't already built that way. Multiple "tabContainers" would appear on the same page at the same time so care would need to be taken to have it create unique IDs each time it's used. |
| return ''; | ||
| } | ||
|
|
||
| export function handleTabs(e) { |
There was a problem hiding this comment.
| export function handleTabs(e) { | |
| export function handleTabListKeyPress(e) { |
Also I'm not seeing why this shouldn't be a full component which tabs a list of child components as slots. Would slots work here, I forget, can you have the same slot filled multiple times and have it render correctly? I think so.
Maybe a question is, are you familiar with html component slots?
| <button class="tab-btn ${this.activeSchemaTab === 'model' ? 'active' : ''}" aria-current="${this.activeSchemaTab === 'model'}" data-tab="model" >${getI18nText('operations.model')}</button> | ||
| <button class="tab-btn ${this.activeSchemaTab !== 'model' ? 'active' : ''}" aria-current="${this.activeSchemaTab !== 'model'}" data-tab="body">${bodyTabNameUseBody ? getI18nText('operations.body') : getI18nText('operations.form')}</button> | ||
| <div class="tab-buttons row" role="tablist" aria-labelledby="request-body-header" @click="${(e) => { if (e.target.tagName.toLowerCase() === 'button') { this.activeSchemaTab = e.target.dataset.tab; } }}" @keydown="${handleTabs}"> | ||
| <button class="tab-btn ${this.activeSchemaTab === 'model' ? 'active' : ''}" id="schema-model-button" role="tab" aria-controls="schema-model-body" aria-selected="${this.activeSchemaTab === 'model'}" tabindex="${this.activeSchemaTab === 'model' ? 0 : '-1'}" data-tab="model" >${getI18nText('operations.model')}</button> |
There was a problem hiding this comment.
Why do we need to be changing the tabindex to -1 when it is not selected. tab index just tells us the order in which things cycle through, I would expect to never need to do this if we correctly number the tabindex for each of these components. What am I missing?
|
|
||
| return html` | ||
| <div class="table-title top-gap">${title}${paramLocation === 'path' ? html`<span style='color:var(--red);'>*</span>` : ''}</div> | ||
| <div class="table-title top-gap" role="heading" aria-level="${this.renderStyle === 'focused' ? 4 : 5}">${title}${paramLocation === 'path' ? html`<span style='color:var(--red);'>*</span>` : ''}</div> |
There was a problem hiding this comment.
Why does renderStyle matter, and why aren't we using h4, h5 for this?
| ${html`<div class="tab-content col" style="display: ${this.activeSchemaTab === 'model' ? 'block' : 'none'}"> ${reqBodySchemaHtml}</div>`} | ||
| ${html`<div class="tab-content col" style="display: ${this.activeSchemaTab === 'model' ? 'none' : 'block'}"> ${reqBodyDefaultHtml}</div>`} | ||
| ${html`<div class="tab-content col" id="schema-model-body" tabindex="0" aria-labelledby="schema-model-button" role="tabpanel" style="display: ${this.activeSchemaTab === 'model' ? 'block' : 'none'}"> ${reqBodySchemaHtml}</div>`} | ||
| ${html`<div class="tab-content col" id="schema-body-body" tabindex="0" aria-labelledby="schema-body-button" role="tabpanel" style="display: ${this.activeSchemaTab === 'model' ? 'none' : 'block'}"> ${reqBodyDefaultHtml}</div>`} |
There was a problem hiding this comment.
how can both of these be tabindex 0?
| ${hasResponse | ||
| ? html` | ||
| <div id="tab_buttons" class="tab-buttons row" role="tablist" aria-label="${getI18nText('operations.request')} ${getI18nText('operations.response')}" @keydown="${handleTabs}" @click="${(e) => { | ||
| if (e.target.classList.contains('tab-btn') === false) { return; } | ||
| this.activeResponseTab = e.target.dataset.tab; | ||
| }}"> | ||
| <button class="tab-btn ${this.activeResponseTab === 'curl' ? 'active' : ''}" role="tab" aria-selected="${this.activeResponseTab === 'curl'}" tabindex="${this.activeResponseTab === 'curl' ? 0 : '-1'}" aria-controls="req-panel" id="req-button" data-tab = 'curl'>${getI18nText('operations.request')}</button> | ||
| <button class="tab-btn ${this.activeResponseTab === 'response' ? 'active' : ''}" role="tab" aria-selected="${this.activeResponseTab === 'response'}" tabindex="${this.activeResponseTab === 'response' ? 0 : '-1'}" aria-controls="req-response-panel" id="req-response-button" data-tab = 'response'>${getI18nText('operations.response')}</button> | ||
| <button class="tab-btn ${this.activeResponseTab === 'headers' ? 'active' : ''}" role="tab" aria-selected="${this.activeResponseTab === 'headers'}" tabindex="${this.activeResponseTab === 'headers' ? 0 : '-1'}" aria-controls="req-response-headers-panel" id="req-response-headers-button" data-tab = 'headers'>${getI18nText('operations.response-headers')}</button> | ||
| </div>` | ||
| : html` | ||
| <div id="tab_buttons" class="tab-buttons row"> | ||
| /* nonfunctional button should not be a button element*/ | ||
| <div class="tab-btn active" role="heading" aria-level="${this.renderStyle === 'focused' ? 4 : 5}" data-tab = 'curl' id="req-button">${getI18nText('operations.request')}</div> | ||
| </div>` | ||
| } |
There was a problem hiding this comment.
I don't understand what is going on here, in this whole block.
| aria-current="${this.selectedStatus === respStatus}" | ||
| role="tab" | ||
| aria-selected="${this.selectedStatus === respStatus}" | ||
| aria-controls="status${respStatus}" |
There was a problem hiding this comment.
| aria-controls="status${respStatus}" | |
| aria-controls="status-${respStatus}" |
And similar for all the other ones, we should use a separator for all elements values / keys
| ${this.activeSchemaTab === 'body' | ||
| ? html`<div class='tab-content col' style='flex:1;'> | ||
| ${this.mimeExampleTemplate(this.mimeResponsesForEachStatus[status][this.selectedMimeType])} | ||
| </div>` | ||
| : html`<div class='tab-content col' style='flex:1;'> | ||
| ${this.mimeSchemaTemplate(this.mimeResponsesForEachStatus[status][this.selectedMimeType])} | ||
| </div>` | ||
| } | ||
| ${html`<div class='tab-content col' role="tabpanel" tabindex="0" id='resp-body-body' aria-labelledby='resp-body-button' style='flex:1; display: ${this.activeSchemaTab === 'body' ? 'block' : 'none'}'> | ||
| ${this.mimeExampleTemplate(this.mimeResponsesForEachStatus[status][this.selectedMimeType])} | ||
| </div>`} | ||
| ${html`<div class='tab-content col' role="tabpanel" tabindex="0" id='resp-model-body' style='flex:1; display: ${this.activeSchemaTab === 'body' ? 'none' : 'block'}'> | ||
| ${this.mimeSchemaTemplate(this.mimeResponsesForEachStatus[status][this.selectedMimeType])} | ||
| </div>`} |
| .tab-btn:focus-visible, | ||
| .tab-btn.active:focus-visible { |
2 participants
There are many grouped buttons (with class
.tab-buttons,.tab-btn) which show and hide sections of content (class.tab-content) according to a fairly standard tab pattern. But currently, these are not clearly marked or keyboard operational for screen reader or other keyboard users to switch between and then access the revealed content.This PR fixes this issue by using aria roles to indicate tab lists, tab buttons, and tab content panels (see https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/tab_role) and adding the expected keyboard functionality for these roles.