fix(web): make sortable grid column headers keyboard accessible - #42471
Open
Miodrag Obradovic (Kjubikstronk) wants to merge 1 commit into
Open
fix(web): make sortable grid column headers keyboard accessible#42471Miodrag Obradovic (Kjubikstronk) wants to merge 1 commit into
Miodrag Obradovic (Kjubikstronk) wants to merge 1 commit into
Conversation
Author
|
@microsoft-github-policy-service agree |
1 similar comment
Author
|
@microsoft-github-policy-service agree |
Contributor
Test results for "MCP"8305 passed, 1367 skipped Merge workflow run. |
Contributor
Test results for "tests 1"6 flaky51283 passed, 1242 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
1 participant
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.
The sortable column headers in
GridViewcarry theironClickon a plain<div>with no role, notabIndexand no key handler, so they cannot be reached or activated from the keyboard and are not announced as controls.GridViewbacks the trace viewer's Network tab, so sorting by Name, Method, Status, Content-Type, Size or Duration is keyboard-inaccessible.This follows the pattern from #42310 and #42336 and uses a real
<button>withall: unsetrather thanrole="button"on a div, with the same:focus-visibleoutline treatment as.expandable-title-button.The button is only rendered when the grid is actually sortable. Previously the header was always clickable and the handler was guarded with
model.setSorting &&, buttoggleSortingalready doesmodel.setSorting?.()internally, so that guard was redundant. Moving the condition to render time means a non-sortable column no longer presents an inert clickable element.One test in
trace-viewer.spec.ts, written in the style of the one added in #42449: focus the Name header, confirm focus, press Enter, and assert the404request sorts to the top. The default order isframe.html, style.css, 404, script.jsper the existingshould have network requeststest, so that assertion is not vacuous. Without the change the test fails at the first assertion, because no button exists to find.Verified locally on chromium: the full
trace-viewer.spec.tspasses at 121/121, and every test matchingnetworkpasses at 152 passed / 5 skipped.tscreports nothing inpackages/weband eslint is clean on the changed file.I did not add
aria-sort, since doing it properly needsrole="columnheader"insiderole="row"/role="grid"and that restructures the component. Happy to follow up separately if you would like the full grid semantics.