Skip to content

[test] Automate 2.4.7 Focus Visible - #48945

Draft
michelengelen wants to merge 13 commits into
mui:masterfrom
michelengelen:a11y/stack/focus-visible
Draft

[test] Automate 2.4.7 Focus Visible#48945
michelengelen wants to merge 13 commits into
mui:masterfrom
michelengelen:a11y/stack/focus-visible

Conversation

@michelengelen

@michelengelen michelengelen commented Aug 11, 2026

Copy link
Copy Markdown
Member

Automates 2.4.7 Focus Visible across the seven components that rate it, by comparing each control focused and unfocused in a real browser.

axe has no rule for this criterion, and the unit tests that exist for it are skipIf(isJsdom()) — so they never actually run. It was rated on source review alone.

Why a pixel comparison rather than a computed-style diff: MUI's focus indicator is usually the ripple, a child element that appears in the DOM. Diffing styles on the control itself would miss it entirely and report a false failure. The capture is padded by 8px so an outline painted outside the control's box still counts.

I mutation-tested the assertion rather than trusting a green run: stripping outlines, box-shadows and the ripple makes 4 of the 7 fail. The remaining three have additional real indicators the sabotage did not remove — TextField's notched-outline border colour, for instance.

The rating stays 🔁 Hybrid, not ⚙️ Automated. The test proves an indicator exists, not that it has enough contrast — that is 1.4.11, which is still manual. What changes is the evidence flag: six components drop their 🚩, taking verified coverage from 184/230 to 190/230.

Follow-up is 1.4.11 itself. A spike confirmed it can be measured without new dependencies by screenshotting an element, letting the browser decode it into a canvas, and sampling pixels — that reproduced the Switch thumb-vs-track ratio at 2.26:1, inside the 1.8–2.6:1 range measured by hand in the report.

Important

How to review this PR

Layer 13 of 14 in a stacked series (#48915#48926). Its branch is built on a11y/stack/scorecard (#48925). GitHub cannot chain PR bases across a fork, so every PR in the series targets master instead.

Review only the last commitfda28fb. Everything above it belongs to the layers below, and disappears as those merge.

Part of the WCAG conformance effort (#14187).

@code-infra-dashboard

code-infra-dashboard Bot commented Aug 11, 2026

Copy link
Copy Markdown

Deploy preview

Bundle size

Bundle Parsed size Gzip size
@mui/material 0B(0.00%) 0B(0.00%)
@mui/lab 0B(0.00%) 0B(0.00%)
@mui/private-theming 0B(0.00%) 0B(0.00%)
@mui/system 0B(0.00%) 0B(0.00%)
@mui/utils 0B(0.00%) 0B(0.00%)

Details of bundle changes


Check out the code infra dashboard for more information about this PR.

mj12albert and others added 13 commits August 11, 2026 13:50
Lets a fixture assert every axe rule it exercises rather than only the
CSS-dependent visual ones, which is what the per-component WCAG reports
need.
Adds a table-driven Playwright suite covering 1.4.10 Reflow, 1.4.4 Resize
Text and 1.4.12 Text Spacing across the assessed components. axe has no rule
for any of them, so they were rated Manual on evidence from source review.
Adds `pnpm a11y:scorecard`, which parses the count table out of every
`<Component>/accessibility.md`, regenerates the index table, and emits the
JSON that the public conformance page is built from. `--check` fails when
either output is stale so a new report cannot land without its rollup.
Compares each control focused and unfocused in a real browser. axe has no
rule for this criterion, and the existing unit tests for it are skipped
under jsdom, so it rested on source review alone.

Drops the evidence flag on the six components that carried one. The rating
stays Hybrid rather than Automated: the test proves an indicator exists,
not that it has enough contrast, which is 1.4.11 and still manual.
@michelengelen
michelengelen force-pushed the a11y/stack/focus-visible branch from da38130 to fda28fb Compare August 11, 2026 12:25
@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Aug 17, 2026
@michelengelen

Copy link
Copy Markdown
Member Author

PR review

The pixel-comparison approach is sound — animations: 'disabled' plus Playwright's default caret hiding remove the incidental-difference sources, and the padded clip catches indicators that paint outside the control. The report bookkeeping checks out: 46→40 flags, one per updated report, and ToggleButton correctly needs no update because its browser-gated unit test already covers 2.4.7. Nothing blocking; one logic slip and two stack-hygiene items.

Bugs (2)

1. 🟡 tabTo treats a null activeElement as "reached"

Location: test/regressions/index.test.js:645

(target) => document.activeElement?.closest(target) !== null,

When activeElement is null, the optional chain yields undefined, and undefined !== null is true — the helper reports the target reached in exactly the case the ?. guards against.

Failure scenario: focus lands nowhere (rare in Chromium, but that is the case this guard exists for); the test proceeds and fails with the misleading "looks identical focused and unfocused" error instead of "could not reach with the Tab key".

Fix: use loose inequality: document.activeElement?.closest(target) != null.

2. 🟡 The pnpm-lock.yaml hunk commits drift caused by #48925

Location: pnpm-lock.yaml (execa specifier ^10.0.1^10.0.0)

This hunk records the accidental execa downgrade that #48925 made in package.json (see the 🔴 finding there). It does not belong to the Focus Visible work.

Failure scenario: #48925 restores master's ^10.0.1; this hunk then reintroduces a mismatched lockfile.

Fix: drop the hunk once #48925's package.json is restored.

Docs (1)

3. 🟡 The suite's JSDoc block appears twice in the merged file

Location: test/regressions/index.test.js:565 and :575

#48936 shipped the "Registers 2.4.7 Focus Visible" comment ahead of the function; this PR adds the identical block again above registerFocusVisibleSuites. The merged file carries both. Best fixed by removing the orphan from #48936 and keeping this PR's copy.

Verdict

Approve after nits - the mechanism and bookkeeping are correct; fix the null-check and the two cross-PR leftovers.


🤖 Review generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility a11y PR: out-of-date The pull request has merge conflicts and can't be merged. test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants