Uh oh!
There was an error while loading. Please reload this page.
fix(html): make remaining mouse-only controls keyboard accessible - #42334
Closed
Ayaan Gazali (ayaangazali) wants to merge 1 commit into
Closed
fix(html): make remaining mouse-only controls keyboard accessible#42334Ayaan Gazali (ayaangazali) wants to merge 1 commit into
Ayaan Gazali (ayaangazali) wants to merge 1 commit into
Conversation
Four controls were unreachable with the keyboard, in the same way tabs, chips, expandable titles and the image diff switcher were before microsoft#41434, microsoft#42149, microsoft#42310 and microsoft#42311. The settings gear and the Metadata toggle declared role="button" without a tabIndex, so assistive tech announced a button that could not be operated. The tag chips and the trace viewer "Show all" carried no role at all, so they were not announced as controls in the first place. All four now render native buttons with a focus ring. The error and warning badge in the action list is deliberately left alone: it sits inside a treeitem, so giving it an accessible name adds that name to every action row, which is a regression in the tree rather than an improvement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale
Four controls are still reachable only with a mouse, the same shape fixed for tabs in #41434, chips
in #42149, and expandable titles and the image diff switcher in #42310 and #42311.
headerView.tsx:136<div role='button' title='Settings'>testFilesView.tsx:84<div className='metadata-toggle' role='button'>labels.tsx:31<span className='label' onClick>actionList.tsx:115<div className='action-list-show-all' onClick>The first two are the worse pair: they declare
role="button"and then cannot take focus, soassistive tech is told there is a button that cannot be operated. The other two carry no role at
all, so they are not announced as controls in the first place.
All four become native buttons with a
:focus-visiblering, no key handlers. The html reporterfiles use
--color-accent-fg, matchingchip.cssandtabbedPane.css;actionList.cssuses--vscode-focusBorder, matching the rest of the trace viewer. The gear also gainsaria-haspopup="dialog"andaria-expanded, and the Metadata toggle gainsaria-expanded, whichneither had.
labels.tsxonly becomes a button on the clickable branch.Labelhas anhrefbranch thatalready renders a real anchor, but
LabelsClickViewcalls it withouthref, so the tag chipsalways took the bare-span path.
One thing I deliberately did not change.
actionList.tsx:167, the error/warning badge thatreveals the console, is also mouse-only. I made it a button, and it broke
ui-mode-trace.spec.ts:778: because the badge sits inside atreeitem, giving it an accessiblename appends that name to every action row,
- treeitem "Evaluate 2ms Reveal console". That is aregression in the accessibility tree rather than an improvement, so it needs a different approach
and is better done separately than bundled here.
Test
headerView.spec.tsgets one keyboard case for the gear: focus it, assert focus lands, press Enter,assert
aria-expandedflips. On current main it fails attoBeFocusedwithReceived: inactive.Green: html-reporter component tests 18,
reporter-html.spec.ts198,trace-viewer.spec.ts113,ui-mode-trace.spec.ts27,flintclean.Fixes#42323
for context, my last three PRs here were closed with the CLA check still pending, so if that is
what blocks this one too please just say so and I will get it sorted rather than have it linger.
also, reading how you structured
titleSuffixin #42310 and the--vscode-focusBorderfallback in#42311 is what made me check the token per package here instead of copying one blindly, so thanks
for that. freshman in college, learning a lot from these reviews :)