-
Notifications
You must be signed in to change notification settings - Fork 918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Accessibility issues on contact page #15695
Fix: Accessibility issues on contact page #15695
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work - thanks!
There's one small update I'd like to include before merging, see aria-label comment
There's also a small style change from the heading update (h4
text size to h3
text size), but I think the visual hierarchy is still clear and it doesn't not need to be updated stylistically
Co-authored-by: maureenlholland <maureen@silverorange.com>
Hi Maureen! Thanks so much, I commited your suggested change. Let me know if there’s anything else you’d like updated. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15695 +/- ##
==========================================
+ Coverage 79.04% 79.06% +0.01%
==========================================
Files 159 160 +1
Lines 8329 8355 +26
==========================================
+ Hits 6584 6606 +22
- Misses 1745 1749 +4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
Thanks again @camillegonzales ! 🎉
One-line summary
This PR addresses the accessibility issues outlined in Issue #15339.
Significant changes and points to review
Axe-core violations and changes:
role="navigation"
from<ul class="category-tabs" role="navigation">
, since "ARIA role navigation is not allowed for given element."<h4>
tags to<h3>
, since last heading level was<h2>
.<li>
elements must be contained in a<ul>
or<ol>
: Wrapped the<ul>
in a<nav>
element to ensure semantic structure and proper roles, since "List item parent element has a role that is not role="list"."Issue / Bugzilla link
Fixes #15339
Testing
Used browser Inspector tools and performed manual testing to verify functionality and accessibility.