Skip to content
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

ACS-3918 - v15 modal teleport #154

Merged
merged 1 commit into from
Nov 30, 2023
Merged

Conversation

daemoncron
Copy link
Collaborator

@daemoncron daemoncron commented Nov 7, 2023

Description

  • Fixes the issue where screen readers can read content outside the modal in Android Talkback (possibly other combos).
  • Upon mounting, any modal instances will now be teleported to the root of the document.body element. When the modal is opened, other top-level children of the body element that could contain screen-readable content will be marked aria-hidden="true" to prevent them from being read by certain screen readers.

BREAKING CHANGE: This may not be a breaking change for teams, but they will need to test that it works for their specific implementation, and most importantly, their tests. Most likely issues will occur around UI tests and and unit tests. Will have full details in release notes, sending separately to Cedar team.

Note: Hide whitespace before you review to clean up template and snapshot diffs.

Checklist:

Design:

  • (NA, no design changes) Reviewed with designer and meets expectations

Cross-browser testing:

This was covered in internal JIRA ticket, sending link to team separately along with live examples of the issue and fix.

  • Chrome
  • Firefox
  • Edge
  • Safari
  • iOS
  • Android

Unit testing:

  • Sufficient unit test coverage (see unit test best practices for ideas)
  • Snapshot updates are explained with comment and reference the relevant source code change

A11y:

  • Meets WCAG AA standards

Documentation:

  • (NA) API docs created/updated
  • (NA) Examples created/updated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All snapshots just now have teleport-stub wrapper.

'body > *',
':not(script)',
':not(style)',
':not([aria-hidden=true])',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Prevents us from accidentally removing aria-hidden from something that was already aria-hidden="true" on page load.

@daemoncron daemoncron self-assigned this Nov 7, 2023
@benjag benjag changed the base branch from main to release/v15 November 30, 2023 20:12
@benjag benjag marked this pull request as ready for review November 30, 2023 20:12
@benjag benjag merged commit e36245d into release/v15 Nov 30, 2023
1 check passed
@benjag benjag deleted the pr/v15-modal-teleport-ACS-3918 branch November 30, 2023 21:13
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