FIX: add a skip-to-main-content link for keyboard users - #2598
Open
Akshar Gothi (akshar27) wants to merge 4 commits into
Open
FIX: add a skip-to-main-content link for keyboard users#2598Akshar Gothi (akshar27) wants to merge 4 commits into
Akshar Gothi (akshar27) wants to merge 4 commits into
Conversation
The application shell has no bypass control, so keyboard users must
tab through every shell control (Take a tour, primary navigation,
Feedback, Security, Theme) before reaching route content on every
page. This does not satisfy the WCAG 2.4.1 bypass-blocks requirement.
Add a visually-hidden-until-focused skip link as the first focusable
element in MainLayout, targeting the existing main landmark. Give
<main> a stable id and tabIndex={-1} so activating the link moves
keyboard focus directly into it, not just the scroll position.
Fixes microsoft#2597
|
Akshar Gothi (@akshar27) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
…2e coverage) Per @romanlutz's review: - Hide the skip link with transform: translateY(-100%) instead of a fixed top: -40px offset, so it stays fully off-screen regardless of its own rendered height (text zoom, a different font, or longer copy could otherwise leave part of a taller link visible over the top bar). - Disable the reveal transition under prefers-reduced-motion. - Add a real e2e test (Playwright) that presses Tab, asserts the link is focused and visible, presses Enter, and asserts focus lands on #main-content - the Jest test only checked markup and would still pass if the CSS reveal or the focus-on-activate behavior broke.
Author
|
Both addressed in c77392b:
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
You'll need to accept the CLA otherwise we can't accept the contribution. Akshar Gothi (@akshar27) |
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
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.
Description
The application shell has no skip link or equivalent bypass control. On every route, keyboard users must Tab through the same shell controls (Take a tour, primary navigation, Feedback, Security, Theme) before reaching page content — 10 Tab presses on the Home page per the issue. This doesn't satisfy the bypass-blocks expectation in WCAG 2.4.1.
Adds a visually-hidden-until-focused skip link as the very first focusable element in
MainLayout, targeting the existingmainlandmark referenced in the issue (MainLayout.tsx:89). Gave<main>a stableid="main-content"andtabIndex={-1}so activating the link moves keyboard focus directly into the landmark (not just the scroll position, which a plain anchor jump alone wouldn't reliably do for a non-naturally-focusable element).The skip link reuses the existing Fluent UI brand tokens (
colorBrandBackground/colorNeutralForegroundOnBrand) already used elsewhere in the app for prominent elements, so it's visually consistent with the rest of the design system when it appears on focus.Fixes #2597
Tests and Documentation
MainLayout.test.tsxasserting: the skip link renders withhref="#main-content",<main>has the matchingidandtabIndex={-1}, and the skip link is the first focusable element amonga[href]/button/[tabindex]in the rendered tree (this is what guarantees it's reached on the very first Tab press).npm test— all 1409 tests pass (70 suites).npm run lint/npm run type-check/npm run build— all clean.npm run dev, with a minimal stub for/api/auth/configreturning empty client/tenant IDs to exercise the app's existing no-auth dev path): confirmed via the DOM that (1) the skip link is the first focusable element within the app root, (2) it's positioned off-screen by default (top: -40px) and only reveals itself when it receives keyboard-driven focus (Griffel's generated:focus-visiblerule setstop: 0), and (3) activating it movesdocument.activeElementto the<main>landmark (id="main-content"), not just the scroll position. I couldn't get the CDP-synthesized Tab key to trigger Chromium's native focus traversal reliably in the browser-automation environment I have access to, so I verified the same underlying behavior directly instead — happy for a reviewer to also spot-check with a real Tab press.Notes
akshar27/PyRIT) — a one-time manual step outside this PR. I'll push a retrigger commit once it's on, if CI doesn't pick this PR up.