-
Notifications
You must be signed in to change notification settings - Fork 52
Accessible tabs (tablists, tabs, tabpanels) #314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/2.4
Are you sure you want to change the base?
Changes from all commits
95dd9c3
4542287
1604f91
ce3cc07
95040ea
185c9c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ import formatXml from 'xml-but-prettier'; | |
|
|
||
| import { getI18nText } from '../languages/index.js'; | ||
| import { schemaInObjectNotation, getTypeInfo, generateExample, isPatternProperty } from '../utils/schema-utils.js'; | ||
| import { toMarkdown } from '../utils/common-utils.js'; | ||
| import { toMarkdown, handleTabs } from '../utils/common-utils.js'; | ||
| import './schema-tree.js'; | ||
| import getRequestFormTable from './request-form-table.js'; | ||
| import './tag-input.js'; | ||
|
|
@@ -302,7 +302,7 @@ export default class ApiRequest extends LitElement { | |
| } | ||
|
|
||
| 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> | ||
| <div style="display:block; overflow-x:auto; max-width:100%;"> | ||
| <table role="presentation" class="m-table" style="width:100%; word-break:break-word;"> | ||
| ${tableRows} | ||
|
|
@@ -575,7 +575,7 @@ export default class ApiRequest extends LitElement { | |
|
|
||
| return html` | ||
| <div class='request-body-container' data-selected-request-body-type="${this.selectedRequestBodyType}"> | ||
| <div class="table-title top-gap row"> | ||
| <div class="table-title top-gap row" role="heading" aria-level="${this.renderStyle === 'focused' ? 4 : 5}" id="request-body-header"> | ||
| ${getI18nText('operations.request-body')} ${this.request_body.required ? html`<span class="mono-font" style='color:var(--red)'>*</span>` : ''} | ||
| <span style = "font-weight:normal; margin-left:5px"> ${this.selectedRequestBodyType}</span> | ||
| <span style="flex:1"></span> | ||
|
|
@@ -586,12 +586,12 @@ export default class ApiRequest extends LitElement { | |
| ${reqBodySchemaHtml || reqBodyDefaultHtml | ||
| ? html` | ||
| <div class="tab-panel col" style="border-width:0 0 1px 0;"> | ||
| <div class="tab-buttons row" role="group" @click="${(e) => { if (e.target.tagName.toLowerCase() === 'button') { this.activeSchemaTab = e.target.dataset.tab; } }}"> | ||
| <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> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
| <button class="tab-btn ${this.activeSchemaTab !== 'model' ? 'active' : ''}" id="schema-body-button" role="tab" aria-controls="schema-body-body" aria-selected="${this.activeSchemaTab !== 'model'}" tabindex="${this.activeSchemaTab !== 'model' ? 0 : '-1'}" data-tab="body">${bodyTabNameUseBody ? getI18nText('operations.body') : getI18nText('operations.form')}</button> | ||
| </div> | ||
| ${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>`} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how can both of these be tabindex 0? |
||
| </div>` | ||
| : html`${reqBodyFileInputHtml}` | ||
| } | ||
|
|
@@ -627,22 +627,26 @@ export default class ApiRequest extends LitElement { | |
| ${!hasResponse ? '' : html`<button class="m-btn" part="btn btn-outline" @click="${this.clearResponseData}">${getI18nText('operations.clear-response')}</button>`} | ||
| </div> | ||
| <div class="tab-panel col" style="border-width:0 0 1px 0;"> | ||
| <div id="tab_buttons" class="tab-buttons row" @click="${(e) => { | ||
| if (e.target.classList.contains('tab-btn') === false) { return; } | ||
| this.activeResponseTab = e.target.dataset.tab; | ||
| }}"> | ||
| <br> | ||
| <div style="width: 100%"> | ||
| <button class="tab-btn ${!hasResponse || this.activeResponseTab === 'curl' ? 'active' : ''}" data-tab = 'curl'>${getI18nText('operations.request')}</button> | ||
| ${!hasResponse ? '' : html` | ||
| <button class="tab-btn ${this.activeResponseTab === 'response' ? 'active' : ''}" data-tab = 'response'>${getI18nText('operations.response')}</button> | ||
| <button class="tab-btn ${this.activeResponseTab === 'headers' ? 'active' : ''}" data-tab = 'headers'>${getI18nText('operations.response-headers')}</button>` | ||
| } | ||
| </div> | ||
| ${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>` | ||
| } | ||
|
Comment on lines
+630
to
+645
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand what is going on here, in this whole block. |
||
| </div> | ||
| ${this.responseIsBlob | ||
| ? html` | ||
| <div class="tab-content col" style="flex:1; display:${this.activeResponseTab === 'response' ? 'flex' : 'none'};"> | ||
| <div class="tab-content col" id="req-response-panel" style="flex:1; display:${this.activeResponseTab === 'response' ? 'flex' : 'none'};" role="tabpanel" tabindex="0" aria-labelledby="req-response-button"> | ||
| ${this.responseBlobType === 'image' | ||
| ? html`<img style="max-height:var(--resp-area-height, 300px); object-fit:contain;" class="mar-top-8" src="${this.responseBlobUrl}"></img>` | ||
| : '' | ||
|
|
@@ -658,14 +662,14 @@ export default class ApiRequest extends LitElement { | |
| </div> | ||
| </div>` | ||
| : html` | ||
| <div class="tab-content col m-markdown" style="flex:1; display:${this.activeResponseTab === 'response' ? 'flex' : 'none'};" > | ||
| <div class="tab-content col m-markdown" style="flex:1; display:${this.activeResponseTab === 'response' ? 'flex' : 'none'};" role="tabpanel" tabindex="0" id="req-response-panel" aria-labelledby="req-response-button"> | ||
| <syntax-highlighter style="min-height: 60px" mime-type="${this.responseContentType}" .content="${this.responseText}" .label="${getI18nText('operations.response')}"/> | ||
| </div>` | ||
| } | ||
| <div class="tab-content col m-markdown" style="flex:1;display:${this.activeResponseTab === 'headers' ? 'flex' : 'none'};" > | ||
| <div class="tab-content col m-markdown" style="flex:1;display:${this.activeResponseTab === 'headers' ? 'flex' : 'none'};" role="tabpanel" tabindex="0" aria-labelledby="req-response-headers-button" id="req-response-headers-panel"> | ||
| <syntax-highlighter style="min-height: 60px" language="http" .content="${this.responseHeaders}" .label="${getI18nText('operations.response-headers')}"/> | ||
| </div> | ||
| <div class="tab-content m-markdown col" style="flex:1;display:${this.activeResponseTab === 'curl' ? 'flex' : 'none'};"> | ||
| <div class="tab-content m-markdown col" style="flex:1;display:${this.activeResponseTab === 'curl' ? 'flex' : 'none'};" role="${hasResponse ? 'tabpanel' : 'presentation'}" tabindex="{hasResponse ? 0 : '-1'}" id="req-panel" aria-labelledby="req-button"> | ||
| <syntax-highlighter style="min-height: 60px" language="shell" .content="${curlSyntax.trim()}" .label="${getI18nText('operations.request')}"/> | ||
| </div> | ||
| </div>`; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||
| import { LitElement, html, css } from 'lit'; | ||||||
| import { unsafeHTML } from 'lit/directives/unsafe-html.js'; | ||||||
| import { schemaInObjectNotation, generateExample, getTypeInfo } from '../utils/schema-utils.js'; | ||||||
| import { toMarkdown } from '../utils/common-utils.js'; | ||||||
| import { toMarkdown, handleTabs } from '../utils/common-utils.js'; | ||||||
| import { getI18nText } from '../languages/index.js'; | ||||||
| import FontStyles from '../styles/font-styles.js'; | ||||||
| import FlexStyles from '../styles/flex-styles.js'; | ||||||
|
|
@@ -103,7 +103,7 @@ export default class ApiResponse extends LitElement { | |||||
| render() { | ||||||
| return html` | ||||||
| <div class="col regular-font response-panel ${this.renderStyle}-mode"> | ||||||
| <div class=" ${this.callback === 'true' ? 'tiny-title' : 'req-res-title'} " role="heading" aria-level="${this.renderStyle === 'focused' ? 3 : 4}"> | ||||||
| <div class=" ${this.callback === 'true' ? 'tiny-title' : 'req-res-title'} " role="heading" aria-level="${this.renderStyle === 'focused' ? 3 : 4}" id="response-title"> | ||||||
| ${this.callback === 'true' ? getI18nText('operations.callback-response') : getI18nText('operations.response')} | ||||||
| </div> | ||||||
| <div> | ||||||
|
|
@@ -159,7 +159,7 @@ export default class ApiResponse extends LitElement { | |||||
| this.headersForEachRespStatus[statusCode] = tempHeaders; | ||||||
| this.mimeResponsesForEachStatus[statusCode] = allMimeResp; | ||||||
| } | ||||||
| return html`<div class='row' style='flex-wrap:wrap' role="group"> | ||||||
| return html`<div class='row' style='flex-wrap:wrap' role="tablist" aria-labelledby="response-title" @keydown="${handleTabs}"> | ||||||
| ${Object.keys(this.responses).map((respStatus) => html` | ||||||
| ${respStatus === '$$ref' // Swagger-Client parser creates '$$ref' object if JSON references are used to create responses - this should be ignored | ||||||
| ? '' | ||||||
|
|
@@ -173,7 +173,11 @@ export default class ApiResponse extends LitElement { | |||||
| this.selectedMimeType = undefined; | ||||||
| } | ||||||
| }}" | ||||||
| aria-current="${this.selectedStatus === respStatus}" | ||||||
| role="tab" | ||||||
| aria-selected="${this.selectedStatus === respStatus}" | ||||||
| aria-controls="status${respStatus}" | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
And similar for all the other ones, we should use a separator for all elements values / keys |
||||||
| tabindex="${this.selectedStatus === respStatus ? '0' : '-1'}" | ||||||
| id="button${respStatus}" | ||||||
| class='m-btn small ${this.selectedStatus === respStatus ? 'primary' : ''}' | ||||||
| part="btn--resp ${this.selectedStatus === respStatus ? 'btn-fill--resp' : 'btn-outline--resp'} btn-response-status" | ||||||
| style='margin: 8px 4px 0 0; text-transform: capitalize'> | ||||||
|
|
@@ -184,7 +188,7 @@ export default class ApiResponse extends LitElement { | |||||
| </div> | ||||||
|
|
||||||
| ${Object.keys(this.responses).map((status) => html` | ||||||
| <div style = 'display: ${status === this.selectedStatus ? 'block' : 'none'}' > | ||||||
| <div id="status${status}" role="tabpanel" aria-labelledby="button${status}" tabindex="0" style="display: ${status === this.selectedStatus ? 'block' : 'none'}"> | ||||||
| <div class="top-gap"> | ||||||
| <span class="resp-descr m-markdown ">${unsafeHTML(toMarkdown(this.responses[status] && this.responses[status].description || ''))}</span> | ||||||
| ${(this.headersForEachRespStatus[status] && this.headersForEachRespStatus[status].length > 0) | ||||||
|
|
@@ -196,23 +200,21 @@ export default class ApiResponse extends LitElement { | |||||
| ? '' | ||||||
| : html` | ||||||
| <div class="tab-panel col"> | ||||||
| <div class="tab-buttons row" role="group" @click="${(e) => { if (e.target.tagName.toLowerCase() === 'button') { this.activeSchemaTab = e.target.dataset.tab; } }}" > | ||||||
| <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'>${getI18nText('operations.example')}</button> | ||||||
| <div class="tab-buttons row" role="tablist" @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="resp-model-button" aria-controls="resp-model-body" role="tab" aria-selected="${this.activeSchemaTab === 'model'}" tabindex="${this.activeSchemaTab === 'model' ? 0 : '-1'}" data-tab='model'>${getI18nText('operations.model')}</button> | ||||||
| <button class="tab-btn ${this.activeSchemaTab !== 'model' ? 'active' : ''}" id="resp-body-button" aria-controls="resp-body-body" role="tab" aria-selected="${this.activeSchemaTab !== 'model'}" tabindex="${this.activeSchemaTab !== 'model' ? 0 : '-1'}" data-tab='body'>${getI18nText('operations.example')}</button> | ||||||
| <div style="flex:1"></div> | ||||||
| ${Object.keys(this.mimeResponsesForEachStatus[status]).length === 1 | ||||||
| ? html`<span class='small-font-size gray-text' style='align-self:center; margin-top:8px;'> ${Object.keys(this.mimeResponsesForEachStatus[status])[0]} </span>` | ||||||
| : html`${this.mimeTypeDropdownTemplate(Object.keys(this.mimeResponsesForEachStatus[status]))}` | ||||||
| } | ||||||
| </div> | ||||||
| ${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>`} | ||||||
|
Comment on lines
-208
to
+217
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's going on here |
||||||
| </div> | ||||||
| ` | ||||||
| }`) | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,8 @@ export default css` | |
| color:var(--primary-color); | ||
| } | ||
|
|
||
| .tab-btn:focus-visible { | ||
| .tab-btn:focus-visible, | ||
| .tab-btn.active:focus-visible { | ||
|
Comment on lines
+45
to
+46
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's going on here? |
||
| color:var(--secondary-color); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -188,3 +188,29 @@ export function getSanitizedEmail(emailRaw) { | |||||
|
|
||||||
| return ''; | ||||||
| } | ||||||
|
|
||||||
| export function handleTabs(e) { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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? |
||||||
| const b = e.target; | ||||||
| if (b.tagName.toLowerCase() !== 'button') { return; } | ||||||
| const buttons = Array.from(b.parentElement.children); | ||||||
| const i = buttons.indexOf(b); | ||||||
| let newIndex = 0; | ||||||
| switch (e.key) { | ||||||
| case 'ArrowRight': | ||||||
| newIndex = (i + 1) % buttons.length; | ||||||
| break; | ||||||
| case 'ArrowLeft': | ||||||
| newIndex = (i - 1 + buttons.length) % buttons.length; | ||||||
| break; | ||||||
| case 'Home': | ||||||
| newIndex = 0; | ||||||
| break; | ||||||
| case 'End': | ||||||
| newIndex = buttons.length - 1; | ||||||
| break; | ||||||
| default: | ||||||
| return; | ||||||
| } | ||||||
| b.parentElement.children[newIndex].focus(); | ||||||
| } | ||||||
|
|
||||||
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.
Why does renderStyle matter, and why aren't we using h4, h5 for this?