Clear the report's six axe-core accessibility violations and close the gate (fixes #462) - #467
Merged
tylervick merged 1 commit intoAug 14, 2026
Conversation
…462) `HTMLTemplates.swift` contained zero `alt=` and zero `title=` attributes, which was the whole cause of both gating findings: image-alt (critical, six nodes) and frame-title (serious). Four more were moderate or minor. All six are fixed here. - Every `<img>` carries an `alt`. Attachment and screenshot-flow images take the attachment's display name; the two preview panes ship `alt=""`, which is how a decorative placeholder declares itself, and inherit the name of whatever is put in them. - Both frames are named: `#text-attachment` and the run's `#logs-iframe`. - `#device-header` becomes an `h2`, so heading levels no longer skip h1 to h4. - `#left-sidebar`, `#main-content` and `#right-sidebar` become `nav`, `main` and `aside`, which answers landmark-one-main and region together. Every rule and script hook is id- or class-based, so nothing else moves. - `#file-attachment` stops being an empty `h2`. It holds a download link, not a section heading, so it is a `p` — and the two rules that centred it as a heading now name it directly. The gate goes back to `['critical', 'serious']`, so a new critical or serious finding fails the build; moderate and minor stay informational, which is the level #462 prescribed. The test's name says what it asserts again. The two rules that survive without a browser — every image has alt text, every frame has a name — are also restated as `AccessibilitySeamTests`, so they hold in the `test` jobs and not only in `visual`. One knock-on: `alt` puts the attachment display name in a second place, and that name is a divergence the modern reader already declares. The `attachmentDisplayNames` allow-list entry now masks both sites; masking only the text line would have let `alt` reintroduce a difference already accounted for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
tylervick
added a commit
that referenced
this pull request
Aug 14, 2026
…462) (#467) `HTMLTemplates.swift` contained zero `alt=` and zero `title=` attributes, which was the whole cause of both gating findings: image-alt (critical, six nodes) and frame-title (serious). Four more were moderate or minor. All six are fixed here. - Every `<img>` carries an `alt`. Attachment and screenshot-flow images take the attachment's display name; the two preview panes ship `alt=""`, which is how a decorative placeholder declares itself, and inherit the name of whatever is put in them. - Both frames are named: `#text-attachment` and the run's `#logs-iframe`. - `#device-header` becomes an `h2`, so heading levels no longer skip h1 to h4. - `#left-sidebar`, `#main-content` and `#right-sidebar` become `nav`, `main` and `aside`, which answers landmark-one-main and region together. Every rule and script hook is id- or class-based, so nothing else moves. - `#file-attachment` stops being an empty `h2`. It holds a download link, not a section heading, so it is a `p` — and the two rules that centred it as a heading now name it directly. The gate goes back to `['critical', 'serious']`, so a new critical or serious finding fails the build; moderate and minor stay informational, which is the level #462 prescribed. The test's name says what it asserts again. The two rules that survive without a browser — every image has alt text, every frame has a name — are also restated as `AccessibilitySeamTests`, so they hold in the `test` jobs and not only in `visual`. One knock-on: `alt` puts the attachment display name in a second place, and that name is a divergence the modern reader already declares. The `attachmentDisplayNames` allow-list entry now masks both sites; masking only the text line would have let `alt` reintroduce a difference already accounted for. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes #462. Stacked on #466 — both touch
HTMLTemplates.swiftand both invalidate the same two goldens, so they cannot be reviewed againstmainindependently. Retarget tomainonce #466 merges.HTMLTemplates.swiftcontained zeroalt=and zerotitle=attributes anywhere in the file, which was the whole cause of both gating findings. All six violations are fixed.image-alt<img>carriesaltframe-title#text-attachmentand#logs-iframeare namedheading-order#device-headerh4 → h2, so levels no longer skip h1→h4landmark-one-main#main-contentdiv →<main>region<nav>/<aside>witharia-labelsempty-heading#file-attachmenth2 →paxe now reports zero violations at any impact, not just zero gating ones.
Choices worth a second look
Alt text is the display name, not the file name. A real
.xcresultnames its payloads with an 80-character opaque id; reading that aloud is worse than the missingaltit replaces.TestScreenshotFlowgained aNAMEplaceholder for this. The two preview panes shipalt=""— how a decorative placeholder declares itself — and inherit the name of whatevershowScreenshot/showGifputs in them.#file-attachmentis apnow. It holds a download link, not a section heading; it was empty at load because its text is set by JS. The two CSS rules that centred it as a heading name it directly instead, so it looks identical, andquerySelector("#right-sidebar h2")still resolves to the placeholder.Landmarks changed elements, nothing else. Every CSS rule and script hook involved is id- or class-based.
The gate
GATING_IMPACTSgoes back to['critical', 'serious'], so a new critical or serious finding fails the build. Left at[]the expectation wasexpect([]).toEqual([]), which cannot fail. Moderate and minor stay informational — the level #462 prescribed, and it keeps the failure mode proportionate. The test's name asserts what it checks again.The two rules that survive without a browser are also restated as
AccessibilitySeamTests, so they hold in thetestjobs and not only invisual, which is a separate workflow.One knock-on worth flagging
altputs the attachment display name in a second place, and that name is a divergence the modern reader already declares.DifferentialTestscaught it immediately. TheattachmentDisplayNamesallow-list entry now masks both sites; masking only the text line would have letaltreintroduce a difference already accounted for. No new allow-list entry, andtestEveryAllowListEntryStillMasksARealDivergencestill passes.Verified locally: 129 Swift tests, 0 failures; 11 Playwright tests, 0 failures. Goldens refreshed and read.
🤖 Generated with Claude Code