Skip to content

fix: Ensure aria-hidden applies to shadow root #315

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

Merged
merged 10 commits into from
May 7, 2025
Merged

Conversation

khiga8
Copy link
Contributor

@khiga8 khiga8 commented May 7, 2025

Relates to: https://github.com/github/pull-requests/issues/17192

Problem

When aria-hidden="true" is set on an relative-time element, the attribute doesn't always persist into the shadow root. As a result, even when aria-hidden is applied, the text rendered in relative-time can end up composing the accessible name (instead of being hidden from the AT tree).

Solution

This makes update to render the text content within aria-hidden="true", when aria-hidden="true" is set on the shadow root.

Demo

Before (no sound):

Before.mp4

video description: I'm on the demo page with the developer console open. The markup shows aria-hidden being toggled between true and false relative-time upon pressing the button. I switch to the accessibility tree mode. I see the accessible name for the button remains: With aria-hidden on Dec 31, 1969 despite toggling.

After (no sound):

After.mp4

video description: I'm on the demo page with the developer console open. The markup shows aria-hidden being toggled between true and false relative-time upon pressing the button. I switch to the accessibility tree mode. I see the accessible name for the button toggle between: With aria-hidden on Dec 31, 1969 and With aria-hidden as we'd expect.

@khiga8 khiga8 marked this pull request as ready for review May 7, 2025 16:57
@Copilot Copilot AI review requested due to automatic review settings May 7, 2025 16:57
@khiga8 khiga8 requested a review from a team as a code owner May 7, 2025 16:57
@khiga8 khiga8 changed the title Ensure aria-hidden applies to shadow root fix: Ensure aria-hidden applies to shadow root May 7, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 ensures that when aria-hidden="true" is set on a <relative-time> element, the attribute also applies inside its shadow root, hiding the rendered text from assistive technologies.

  • Added 'aria-hidden' to the element’s observedAttributes so attribute changes are handled.
  • Introduced a private #updateRenderRootContent method to wrap content in a hidden <span> when aria-hidden="true".
  • Added new tests to verify shadow-root behavior and updated the example page with a toggle button.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
test/relative-time.js Added [aria-hidden] test suite to assert shadow-root behavior
src/relative-time-element.ts Observes aria-hidden and updates rendering via #updateRenderRootContent
examples/index.html Demo button toggling aria-hidden on <relative-time>

khiga8 and others added 4 commits May 7, 2025 13:00
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link

@JoyceZhu JoyceZhu left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Member

@jonrohan jonrohan left a comment

Choose a reason for hiding this comment

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

Nice catch

@khiga8 khiga8 merged commit 33cde64 into main May 7, 2025
4 checks passed
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.

3 participants