Skip to content

fix(a11y): restore the main landmark (A5) - #40

Merged
BrianGenisio merged 3 commits into
mainfrom
fix/a11y-a5-main-landmark
Aug 6, 2026
Merged

fix(a11y): restore the main landmark (A5)#40
BrianGenisio merged 3 commits into
mainfrom
fix/a11y-a5-main-landmark

Conversation

@BrianGenisio

Copy link
Copy Markdown
Contributor

Closes #39.

Finding

A5 — role="log" on <main> removes the main landmark (serious; WCAG 1.3.1, 2.4.1).

Change

Strip role="log" and aria-live="polite" from #chatHistory so <main> is a landmark again. The A1+A2+A11 foundation PR will add a dedicated status live region; this PR does not introduce a temporary role="log" wrapper.

Also excludes a11y-audits/** from vitest discovery — #38 landed a node:test file that the root suite was picking up.

Baseline

Shrink-only axe baseline drops aria-allowed-role and landmark-one-main in both themes. Remaining region is the sidebar resizer (out of scope here).

Verification

  • npm test — 173 passed | 2 expected fail
  • A11Y_CI=1 axe gate — passed against updated baseline (light + dark)
  • Confirmed axe no longer reports aria-allowed-role / landmark-one-main on empty and settings states

Reference

a11y-audits/8-5-26/audit.md → A5
a11y-audits/8-5-26/resolution-plan.md → Wave 1

Made with Cursor

Brian Genisio and others added 2 commits August 6, 2026 15:38
baseline.test.mjs uses node:test in its own package; vitest was picking it
up from the repo root and failing the app suite.

Co-authored-by: Cursor <cursoragent@cursor.com>
role=\"log\" on <main> overrode the landmark and triggered axe
aria-allowed-role and landmark-one-main. Live-region semantics move to a
dedicated child in the A1 foundation PR.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5a2f0161-0797-4713-893d-45e5da7c6060

📥 Commits

Reviewing files that changed from the base of the PR and between 3e12961 and a4e7f01.

📒 Files selected for processing (1)
  • vitest.config.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • vitest.config.js

📝 Walkthrough

Walkthrough

The chat history <main> element no longer uses role="log" or aria-live="polite". The axe baseline removes the related aria-allowed-role violations and empty-state landmark-one-main violations. Vitest preserves default exclusions and excludes a11y-audits.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary accessibility change: restoring the main landmark for A5.
Description check ✅ Passed The description explains the landmark fix, baseline updates, Vitest exclusion, linked issue, and verification results.
Linked Issues check ✅ Passed The changes satisfy issue #39 by restoring the main landmark, removing conflicting attributes, updating the baseline, and covering both themes.
Out of Scope Changes check ✅ Passed All changes support issue #39 or the documented Vitest test-discovery correction; no unrelated code changes are present.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@vitest.config.js`:
- Around line 1-6: Update the Vitest configuration’s test.exclude entry in
defineConfig to preserve the framework defaults by spreading
configDefaults.exclude before adding the a11y-audits exclusion, ensuring the
default .git and node_modules patterns remain active.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f2852deb-e14b-4caf-b15b-097a2b4d4ab2

📥 Commits

Reviewing files that changed from the base of the PR and between 6bca6c6 and 3e12961.

📒 Files selected for processing (3)
  • a11y-audits/tools/baseline.json
  • public/index.html
  • vitest.config.js
💤 Files with no reviewable changes (1)
  • a11y-audits/tools/baseline.json

Comment thread vitest.config.js Outdated
Spread configDefaults.exclude so .git and node_modules patterns stay active.

Co-authored-by: Cursor <cursoragent@cursor.com>
@BrianGenisio

Copy link
Copy Markdown
Contributor Author

Addressed review: vitest.config.js now spreads configDefaults.exclude before the a11y-audits exclusion.

@BrianGenisio
BrianGenisio merged commit 873b241 into main Aug 6, 2026
1 check passed
@BrianGenisio
BrianGenisio deleted the fix/a11y-a5-main-landmark branch August 6, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[a11y][A5] role="log" on <main> removes the main landmark

1 participant