Skip to content

Commit dc057df

Browse files
committed
refactor(ui-scripts): put a11y findings behind the badge, not in the row
Feedback on the first pass: the report showed the same findings three times over - boxes on every "Actual" thumbnail, a card list under each row, and outlines in the HTML view - which buried the image comparison the report exists for. Findings now live in one place, the lightbox's A11y view, reached by clicking a row's badge. The row is back to a three-up comparison plus a "N a11y" pill colored by worst impact. Removed: the a11y overlay and legend from the HTML view (it iframes the live page again, nothing more), the impact filter chips, the "Show markers on screenshots" toggle, and the embedded a11y JSON payload that only the HTML view's legend needed. Boxes and cards are rendered into an inert <template> per row and cloned when the A11y view opens, keeping one renderer. Markers carry their own rule so the overview's rule filter no longer needs a rendered card to consult. Also fixes the contrast violations the report surfaced in the test pages themselves - 403 violating elements down to 15. Two causes, both in the pages rather than the library: custom-icons styled its labels with Tailwind literals (text-gray-500 / text-gray-700) that ignore the theme and failed on light and dark alike, and eleven pages had bare text nodes inheriting the browser default black, unreadable on the dark theme's near-black surface. Both now use Text/Heading, which take their color from the active theme. The 15 remaining are deliberate: inverse/on-color text demos rendered on the page background, text on View's colored alert/success/danger/warning surfaces, a color="inherit" demo, a raw input in a FormField demo, and FileDrop's own renderLabel - that last one is a library finding, not a page one. These twelve pages will show as Changed on the next run; baselines refresh on merge.
1 parent 9493349 commit dc057df

16 files changed

Lines changed: 280 additions & 285 deletions

File tree

.github/workflows/visual-regression.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ jobs:
186186
187187
**Accessibility (axe):** ${{ steps.summary.outputs.a11y }}
188188
189-
📊 **[View full report](https://instructure.design/visual-regression/pr-${{ github.event.pull_request.number }}/index.html)** — every violation is boxed and numbered on its screenshot, with the offending element named and contrast failures shown as color swatches.
189+
📊 **[View full report](https://instructure.design/visual-regression/pr-${{ github.event.pull_request.number }}/index.html)** — click a screenshot's ⚠ badge to see each violation boxed on the image, with the offending element named and contrast failures shown as color swatches.
190190
191191
${{ steps.summary.outputs.diff_images }}
192192

docs/contributing/testing/visual-regression.md

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ Top-bar controls:
8383
- **Filter buttons** (`All`, `Changed`, `New`, `Removed`, `Unchanged`, `⚠ A11y`) — the active one is highlighted.
8484
- **Search input** — live, debounced substring filter on the screenshot name.
8585
- **Theme chips** — narrow to one theme's screenshots.
86-
- **Impact chips** and **Show markers on screenshots** — see below.
8786
- **Lightbox** — click any thumbnail to open a fullscreen viewer. Inside:
8887
- `Baseline`, `Actual`, `Diff` buttons switch between the three images.
8988
- `Slider` mode overlays baseline and actual with a drag handle so you can scrub the boundary back and forth.
@@ -99,13 +98,11 @@ axe runs against every page and theme, and the report shows what it found **on t
9998
While the page is still on screen, `spec.cy.ts` records each violating element's bounding box, a human-readable name for it, and — for contrast failures — the foreground/background colors and ratios axe computed (see `cypress/support/a11y-capture.ts`). The report then renders:
10099

101100
- **A run-level overview** at the top of the page: one line per rule with its impact, how many elements it affects, and how many screenshots it appears on. The same rule tripping in `canvas`, `light`, and `dark` collapses to a single line. Click a rule to narrow the list to just the screenshots it affects.
102-
- **Numbered boxes on the “Actual” image**, colored by impact. The numbers match the cards below, so “#3” is the same finding everywhere it appears. Uncheck **Show markers on screenshots** when they get in the way of comparing pixels.
101+
- **A `⚠ N a11y` badge** in each affected screenshot's header, colored by the worst impact on it. That badge is the only a11y marking in the list — rows stay a clean three-up image comparison. **Click it** to open the lightbox's **A11y** view.
102+
- **The A11y view**: the screenshot at full size with every violating element boxed and numbered, colored by impact, and the findings listed beside it. Clicking a card highlights its box and vice versa. Findings are ordered worst-impact-first, so `#1` is always the most serious thing on that screenshot.
103103
- **One card per offending element**, leading with a plain-language headline ("Text contrast is too low") rather than the rule id, naming the element (`button “Archive this item”`), and suggesting a fix. Contrast failures show both colors as swatches with the measured and required ratios. The raw axe output — rule id, selector, and `failureSummary` — sits behind **Technical details**.
104-
- **Impact chips** (`critical` / `serious` / `moderate` / `minor`) that narrow both the list and the boxes on each image.
105104

106-
Clicking a card's number badge opens the lightbox in **A11y** mode, scrolled to that finding. In that view, clicking a card highlights its box and vice versa.
107-
108-
Findings are ordered worst-impact-first, so `#1` on a screenshot is always the most serious thing on it.
105+
The boxes and cards are rendered into an inert `<template>` inside each row and cloned when the A11y view opens, so there's one renderer rather than a server-side and a client-side copy.
109106

110107
Plain-language copy comes from a rule dictionary in `packages/ui-scripts/lib/commands/visual-diff.ts` (`RULE_COPY`). A rule that isn't in it still renders correctly — it just falls back to axe's own `help` text. Add an entry when a new rule starts showing up.
111108

packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
a11yBadge,
4040
a11yMarkers,
4141
a11yCards,
42+
a11yTemplate,
4243
a11ySummary,
4344
a11yOverview
4445
} from '../commands/visual-diff.ts'
@@ -493,9 +494,13 @@ describe('a11yBadge', () => {
493494
expect(a11yBadge(findings)).toContain('data-impact="critical"')
494495
})
495496

497+
it('is a button, since it opens the A11y view', () => {
498+
expect(a11yBadge(findings)).toContain('<button type="button"')
499+
})
500+
496501
it('pluralizes the tooltip', () => {
497502
expect(a11yBadge(findings.slice(0, 1))).toContain(
498-
'1 accessibility issue, worst impact: critical'
503+
'1 accessibility issue (worst: critical)'
499504
)
500505
expect(a11yBadge(findings)).toContain('2 accessibility issues')
501506
})
@@ -523,6 +528,10 @@ describe('a11yMarkers', () => {
523528
expect(a11yMarkers(findings, page)).toContain('data-impact="critical"')
524529
})
525530

531+
it('tags each box with its rule, since the cards it pairs with are inert', () => {
532+
expect(a11yMarkers(findings, page)).toContain('data-rule="color-contrast"')
533+
})
534+
526535
it('renders nothing when the run recorded no page size', () => {
527536
expect(a11yMarkers(findings, null)).toBe('')
528537
})
@@ -533,6 +542,31 @@ describe('a11yMarkers', () => {
533542
})
534543
})
535544

545+
describe('a11yTemplate', () => {
546+
const { findings, page } = findingsFor(
547+
'button-dark.png',
548+
normalizeA11y(CAPTURE)
549+
)
550+
551+
it('renders nothing when there are no findings', () => {
552+
expect(a11yTemplate([], page)).toBe('')
553+
})
554+
555+
it('parks the boxes and cards in an inert template', () => {
556+
const html = a11yTemplate(findings, page)
557+
expect(html.startsWith('<template class="a11y-findings">')).toBe(true)
558+
expect(html.endsWith('</template>')).toBe(true)
559+
expect(html).toContain('class="mk"')
560+
expect(html).toContain('class="v-card"')
561+
})
562+
563+
it('still emits the cards when there are no boxes to draw', () => {
564+
const html = a11yTemplate(findings, null)
565+
expect(html).not.toContain('class="mk"')
566+
expect(html).toContain('class="v-card"')
567+
})
568+
})
569+
536570
describe('a11yCards', () => {
537571
const { findings } = findingsFor('button-dark.png', normalizeA11y(CAPTURE))
538572
const html = a11yCards(findings)

0 commit comments

Comments
 (0)