-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
fix(ui): Learn Page Landmark Accessibility issues #8551
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(ui): Learn Page Landmark Accessibility issues #8551
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website Please review the changes when you have a chance. Thank you! 🙏 |
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.
Pull request overview
This PR fixes accessibility violations on the Learn page by replacing non-semantic <div> elements with semantic HTML landmarks, addressing Axe accessibility issues.
Changes:
- Changed
MetaBarwrapper from<div>to<aside>element witharia-label="Article metadata" - Changed
Bannerwrapper from<div>to<section>element withrole="region"andaria-label="Announcement" - Bumped package version from 1.5.4 to 1.5.5 in
packages/ui-components/package.json
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/ui-components/src/Containers/MetaBar/index.tsx | Changed wrapper element from <div> to <aside> with aria-label="Article metadata" to provide semantic landmark |
| packages/ui-components/src/Common/Banner/index.tsx | Changed wrapper from <div> to <section> with role="region" and aria-label="Announcement" |
| packages/ui-components/package.json | Version bump from 1.5.4 to 1.5.5 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
avivkeller
left a comment
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.
(Blocking, as this can't land with the hardcoded values)
|
@avivkeller When time permits, can you re-review it? |
avivkeller
left a comment
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.
LGTM after @ovflowd's comment
|
Thank you @avivkeller @ovflowd :) |
|
Lighthouse Results
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8551 +/- ##
==========================================
+ Coverage 74.96% 74.98% +0.01%
==========================================
Files 103 103
Lines 9036 9037 +1
Branches 312 312
==========================================
+ Hits 6774 6776 +2
+ Misses 2260 2259 -1
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Description
Fixes Axe accessibility violations where Learn page content was not contained within semantic HTML landmarks.
Changes Made
<div>to<aside>element witharia-label="Article metadata".<div>to<section>element withrole="region"andaria-label="Announcement".ui-packagesdirectory, ran command:pnpm version patchValidation (Axe Report)
Before:

After:

Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.