Make "Open in Terminal" button accessible #4631
Conversation
| %> | ||
| <% if Configuration.login_clusters.count > 0 %> | ||
| <button type="button" class="btn btn-sm btn-outline-dark dropdown-toggle dropdown-toggle-split" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false"></button> | ||
| <button type="button" class="btn btn-sm btn-outline-dark dropdown-toggle dropdown-toggle-split" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false" aria-labelledby="files-shell-button"></button> |
There was a problem hiding this comment.
While aria- attributes are available, the general rule is to try not to use them. Or use them as a last resort.
Thinking in this way - I wonder if we shouldn't just internationalize Open in Terminal and use it here and above (I suppose using here as the title?).
Also - should we do aria-labelledby, let's try to prefer _ separator instead just because that's how rails tends to build form IDs, so it's a uniformity thing with Rails.
There was a problem hiding this comment.
While that's usually true for aria- attributes, the title attribute has its own issues and aria-label or aria-labelledby are more consistent - see https://www.w3.org/WAI/ARIA/apg/practices/names-and-descriptions/#cardinalrulesofnaming
There was a problem hiding this comment.
I think a bigger issue is that the tiny side button doesn't do the same thing as "Open in Terminal", it opens a cluster dropdown. Based on my read of the linked guide, aria-label seems to be the best choice here, maybe with text like "Select Cluster". We could also add more description to the dropdown items, like "Open in #{cluster} cluster".
The reason I see aria-label as the best option is that the main drawback is it overrides text contained by that element, but trying this out I just hear "Menu button collapsed sub-menu" when the dropdown button is focused, so there is no contained content that we have to be worried about skipping.
There was a problem hiding this comment.
Yea I'm fine with aria-label, though I think we should still internationalize this.
github-project-automation
bot
moved this from Changes Requested
to Merged/Closed
in PR Review Pipeline
|
When testing this out, I noticed that the screen reader repeats "Activate to sort column ascending" when focused on any file or folder in the files view. Should this be fixed here or in its own separate issue? |
ashton22305
added
the
status/awaiting review
|
I would make a separate issue for that, just because it deals with the files table instead of the open in terminal button |
Fixes #2694
The
Open in terminalbutton itself was already able to be read by a screen reader, but the dropdown to select each cluster was not very specific. So, it is labeled with the text in theOpen in terminalbutton.