Uh oh!
There was an error while loading. Please reload this page.
fix(web): make the image diff mode switcher keyboard operable - #42267
Closed
Ayaan Gazali (ayaangazali) wants to merge 1 commit into
Closed
fix(web): make the image diff mode switcher keyboard operable#42267Ayaan Gazali (ayaangazali) wants to merge 1 commit into
Ayaan Gazali (ayaangazali) wants to merge 1 commit into
Conversation
The Diff / Actual / Expected / Side by side / Slider switcher in the screenshot diff view is five plain divs with an onClick, no role, no tabIndex and no key handling. Tab walks past all of them, so a keyboard user looking at a failed visual comparison cannot change which image is shown, and a screen reader is never told the controls exist. Render them as native buttons in a tablist, the same way tabs and chips were converted in microsoft#41434 and microsoft#42149, with a focus ring from the shared --vscode-focusBorder token. The inline style object is replaced by a class, so the file gets its own stylesheet like the other components.
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
The
Diff/Actual/Expected/Side by side/Sliderrow in the screenshot diff view is five bare divs with anonClick(packages/web/src/shared/imageDiffView.tsx:102-106). Norole, notabIndex, no key handling. Tab walks past the whole row, so someone looking at a failed visual comparison cannot change which image is shown without a mouse, and a screen reader is never told the controls exist. Which mode is active is signalled only byfontWeight, so that is invisible to assistive tech too.ImageDiffViewis shared, so this covers three surfaces: the html reporter result view and error view, and the trace viewer attachments tab.The fix is the approach this repo already settled on in #41434 and used again for chips in #42149: native elements plus a focus ring, no key handlers, Enter and Space come from the platform. Here that means
<button role="tab" aria-selected>inside arole="tablist", matching howTabbedPaneTabis written.aria-selectedalso gives the active mode a machine-readable representation it did not have.The inline
modeStyleobject is replaced by a class in a newimageDiffView.css, which is how every other component in this tree is structured, and is what lets the focus ring use the shared--vscode-focusBordertoken instead of the browser default.Rendering is unchanged. I measured every switcher's box and computed font weight before and after against the same story, and the position, size and weight are identical for all five.
Test
imageDiffView.spec.tsgets one keyboard case: focusSide by side, press Enter, assert it becomes selected and that two images are shown. On current main it fails attoBeFocusedbecause no element matchesrole=tabat all.reporter-html.spec.tsasserted the row via.locator('div'), which is now.getByRole('tab'). That test still pins the same five labels in the same order.Fixes#42266
apologies if I have misjudged anything here, I traced it myself and leaned on claude code to check my approach lined up with how the repo already does keyboard support rather than inventing a new pattern. very happy to be corrected. freshman in college trying to contribute something real :)