fix(trace-viewer): label filter inputs and keyboard-accessible error source - #42469
Open
Changsu Seong (scs0209) wants to merge 2 commits into
Open
fix(trace-viewer): label filter inputs and keyboard-accessible error source#42469Changsu Seong (scs0209) wants to merge 2 commits into
Changsu Seong (scs0209) wants to merge 2 commits into
Conversation
Author
|
@microsoft-github-policy-service agree |
Changsu Seong (scs0209)
force-pushed
the
fix/trace-viewer-a11y-42463
branch
from
August 31, 2026 00:39
3556ad4 to
640eca2
Compare
…source Add aria-labels to the network and UI mode filter searchboxes, and replace the mouse-only error source span with a native button that exposes an accessible name and focus ring. Closes microsoft#42463 Co-authored-by: Cursor <cursoragent@cursor.com>
Changsu Seong (scs0209)
force-pushed
the
fix/trace-viewer-a11y-42463
branch
from
August 31, 2026 00:40
640eca2 to
39efc67
Compare
Dmitry Gozman (dgozman)
left a comment
Dmitry Gozman (dgozman)
left a comment
Collaborator
There was a problem hiding this comment.
Thank you, this looks pretty good. Let's drop two unnecessary tests, and we can land this.
| await expect(summary).toHaveAttribute('aria-expanded', 'false'); | ||
| }); | ||
|
|
||
| test('should expose an accessible name for the filter input', async ({ runUITest }) => { |
Collaborator
There was a problem hiding this comment.
Let's drop this test, it does not add much.
There was a problem hiding this comment.
Done — removed in f4727a6.
| await expect(traceViewer.errorMessages.nth(0)).toHaveText('Expect failed'); | ||
| }); | ||
|
|
||
| test('should open error source from the keyboard', async ({ page, server, runAndTrace }) => { |
Collaborator
There was a problem hiding this comment.
Let's drop this test, it does not add much.
There was a problem hiding this comment.
Done — removed in f4727a6.
Per review feedback, remove standalone filter-name and keyboard source tests; the network filter assertion in the existing spec is enough. Co-authored-by: Cursor <cursoragent@cursor.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Test results for "tests 1"2 failed 7 flaky51273 passed, 1241 skipped Merge workflow run. |
Contributor
Test results for "MCP"1 failed 8304 passed, 1367 skipped Merge workflow run. |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
aria-labelto the network tab and UI mode filter searchboxes (matching the existingFilter actions/ html-reporter pattern).@ file:linespan in the Errors tab with a native<button>that has an accessible name and:focus-visiblering.Fixes #42463
Test plan
tests/library/trace-viewer.spec.ts— network filter exposessearchboxname; error source link opens Source tab via keyboardtests/playwright-test/ui-mode-test-filters.spec.ts— UI mode filter input exposes accessible nameNotes
Thanks to Ayaan Gazali (@ayaangazali) for filing the issue and sharing context on prior a11y fixes (#41434, #42336). I claimed this after their go-ahead in #42463 (comment).
I noticed #42465 also targets this issue; this PR is scoped only to the trace-viewer a11y items in #42463 (no unrelated changes), adds regression tests, uses
aria-label={Go to source: ${longLocation}}on the error source control, and adds:focus-visiblestyling per the trace-viewer focus pattern.