Skip to content

Add missing tabindex and keyboard functions #311

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.

Sign up for GitHub

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

Closed
ahamelers wants to merge 1 commit into from
+5 −5
Closed

Add missing tabindex and keyboard functions #311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/templates/endpoint-template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ function endpointBodyTemplate(path) {
export default function endpointTemplate() {
return html`
<div style="display:flex; justify-content:flex-end; padding-right: 1rem; font-size: 14px; margin-top: 16px;">
<span @click="${(e) => expandCollapseAll.call(this, e, true)}" style="color:var(--primary-color); cursor: pointer;">Expand</span>
<span @click="${(e) => expandCollapseAll.call(this, e, true)}" style="color:var(--primary-color); cursor: pointer;" role="button" tabindex="0" @keydown="${(e) => { if (e.key === 'Enter') { e.target.click(); }}}">Expand</span>

@wparad wparad Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking at this and wondering if it wouldn't make more sense to change this to a button and the other one to a link, that feels like a better fix, and we'll have to adjust the css to account for that. The PR description says there are "a lot of these", but I only count 4 in this PR. If there are only four we can inline the css, otherwise we'll likely want to a new css class to handle this stuff.

@ahamelers ahamelers Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking at this and wondering if it wouldn't make more sense to change this to a button and the other one to a link, that feels like a better fix

That would be ideal, and much cleaner in my opinion!

The PR description says there are "a lot of these", but I only count 4 in this PR.

There are others where much more extensive changes (usually around aria-expanded and aria-controls) are needed to the same element, and I was hoping to the keyboard navigation AND other needed changes to those together in separate PRs, to prevent merge conflicts.

@wparad wparad Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard for me to understand what that would entail to give advice. Is there such a thing as a perfectly configurable accessible button that then we can go and put everywhere? Or are there complex changes needed for each button? For me it mainly feels like a styling problem, and aria label application, is there something else too? Do you have some examples we can iterate on to figure out the complexity? You've looked at this already, what are you thinking exactly?

&nbsp;|&nbsp;
<span @click="${(e) => expandCollapseAll.call(this, e, false)}" style="color:var(--primary-color); cursor: pointer;">Collapse</span>
<span @click="${(e) => expandCollapseAll.call(this, e, false)}" style="color:var(--primary-color); cursor: pointer;" role="button" tabindex="0" @keydown="${(e) => { if (e.key === 'Enter') { e.target.click(); }}}">Collapse</span>
</div>
${(this.resolvedSpec && this.resolvedSpec.tags || []).map((tag) => html`
<div class='regular-font method-section-gap section-tag ${tag.expanded ? 'expanded' : 'collapsed'}'>
Expand Down
4 changes: 2 additions & 2 deletions src/templates/expanded-endpoint-template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ export function expandedEndpointBodyTemplate(path, tag) {
${this.renderStyle === 'read' ? html`<div class='divider' part="operation-divider"></div>` : ''}
<div class='expanded-endpoint-body observe-me ${path.method}' part="section-operation ${path.elementId}" id='${path.elementId}'>
${(this.renderStyle === 'focused' && tag && tag.name !== 'General ⦂')
? html`<h1 style="display: inline; align-self: center" class="title tag-link" role="heading" aria-level="1" data-content-id="${tag.elementId}"
@click="${(e) => this.scrollToEventTarget(e, false)}"> ${tag?.name} ${tagSummary}</h1>`
? html`<h1 style="display: inline; align-self: center" class="title" data-content-id="${tag.elementId}"
><span class="tag-link" role="link" tabindex="0" @click="${(e) => this.scrollToEventTarget(e, false)}" @keydown="${(e) => { if (e.key === 'Enter') { e.target.click(); }}}"> ${tag?.name} ${tagSummary}</span></h1>`
Comment on lines -23 to +24

@wparad wparad Jul 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is using tag-link for, and is it safe to mov elike that?

: ''}
<slot name="${tag.elementId}"></slot>

Expand Down
2 changes: 1 addition & 1 deletion src/templates/navbar-template.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export default function navbarTemplate() {
<div class="nav-bar-section-wrapper">
<div class="nav-bar-paths-under-tag">
${component.subComponents.filter(s => componentIsInSearch(this.matchPaths, s)).map((p) => html`
<div class='nav-bar-path' data-content-id='cmp--${p.id}' id='link-cmp--${p.id}' @click='${(e) => this.scrollToEventTarget(e, false)}'>
<div class='nav-bar-path' data-content-id='cmp--${p.id}' id='link-cmp--${p.id}' @click='${(e) => this.scrollToEventTarget(e, false)}' role="link" tabindex="0" @keydown = '${(e) => { if (e.key === 'Enter') { e.target.click(); }}}'>
<span> ${p.name} </span>
</div>`
)}
Expand Down
Loading