-
Notifications
You must be signed in to change notification settings - Fork 21
chore(components, accessibility): improve post-header semantics
#6841
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 1711a9f The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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 improves the accessibility semantics of the post-header component by wrapping the header content in a semantic <header> element, adding a required caption property to post-mainnavigation for aria-label support, and fixing event listener attachment to use shadowRoot instead of the host element.
Key Changes:
- Added semantic
<header>wrapper element with explicitrole='banner'to thepost-headercomponent - Introduced a required
captionproperty topost-mainnavigationthat provides an accessible label via aria-label - Moved click event listener from host element to shadowRoot for proper event handling within Shadow DOM
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/components/src/components/post-header/post-header.tsx |
Wrapped header content in <header role='banner'> element and moved click event listener to shadowRoot |
packages/components/src/components/post-header/post-header.scss |
Added header selector to transition styles for consistency |
packages/components/src/components/post-mainnavigation/post-mainnavigation.tsx |
Added required caption property and applied it as aria-label to the nav element |
packages/components/src/components/post-mainnavigation/readme.md |
Auto-generated documentation update showing the new caption property |
packages/documentation/src/stories/components/header/header.stories.ts |
Updated story to use shorter caption text ("Main" instead of "Main navigation") |
packages/components/src/components.d.ts |
Auto-generated TypeScript definitions for the new caption property |
packages/components/src/components/post-mainnavigation/post-mainnavigation.tsx
Show resolved
Hide resolved
packages/components/src/components/post-mainnavigation/post-mainnavigation.tsx
Show resolved
Hide resolved
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.
Moved the click event listener from the host element to the shadow root
to fix an accessibility issue where screen readers were incorrectly announcing
the banner landmark as "clickable banner landmark".
post-header semanticspost-header semantics
|



📋 Summary
This PR improves the semantic structure and accessibility of the
post-headerandpost-mainnavigationcomponents.Key Question
Original Question from [#6687]
Decision:
Yes, placing
<nav>inside<header>is correct and follows W3C best practices.Why this is valid:
Both W3C and MDN provide examples showing navigation elements nested within the header. According to MDN:
Source: [MDN - WAI-ARIA Basics](https://developer.mozilla.org/de/docs/Learn_web_development/Core/Accessibility/WAI-ARIA_basics?utm_source=chatgpt.com#wegweiserlandmarken)
Trusted Source Examples:
Changes Made
1. Added Semantic
<header>Element withrole="banner"Accessibility Tree Comparison:
BEFORE:
AFTER:
Why
role="banner"is needed:2. Added Required
captionProp topost-mainnavigation3. Moved Event Listener to Shadow Root
Problem with host listener:
When the event listener is attached to the host element, screen readers detect the entire component as interactive and announce it as "clickable banner landmark", which is incorrect and confusing for users with assistive technology.
Solution:
Changed from
this.host.addEventListener()tothis.host.shadowRoot.addEventListener().