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

fix(overlay): do not hide overlay if toast is presented #29140

Merged
merged 9 commits into from Mar 12, 2024
Merged

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Mar 12, 2024

Issue number: resolves #29139


What is the current behavior?

When implementing #28997 we did not consider the case where a Toast could be presented. When presenting a Toast after presenting a Modal the linked change causes the Modal to be hidden from screen readers.

What is the new behavior?

  • If the top-most overlay is a Toast then the closest non-Toast overlay is also not hidden from screen readers.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build: 7.7.5-dev.11710260658.1fc29a6c

@github-actions github-actions bot added the package: core @ionic/core package label Mar 12, 2024
const nextPresentedOverlay = overlays[i + 1] ?? newTopMostOverlay;

// If next overlay has aria-hidden then all remaining overlays will have it too.
if (nextPresentedOverlay.hasAttribute('aria-hidden')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may cause issues if the developer decides to present a new overlay with aria-hidden. However, I don't know how realistic that scenario is because then screen readers would never be able to focus the new overlay even without this change.

@liamdebeasi liamdebeasi marked this pull request as ready for review March 12, 2024 16:44
@liamdebeasi liamdebeasi requested a review from a team as a code owner March 12, 2024 16:44
@liamdebeasi liamdebeasi requested review from thetaPC, sean-perkins and amandaejohnston and removed request for a team and thetaPC March 12, 2024 16:44
core/src/utils/test/overlays/overlays.spec.ts Outdated Show resolved Hide resolved
core/src/utils/overlays.ts Outdated Show resolved Hide resolved
core/src/utils/overlays.ts Outdated Show resolved Hide resolved
core/src/utils/overlays.ts Outdated Show resolved Hide resolved
core/src/utils/overlays.ts Outdated Show resolved Hide resolved
core/src/utils/overlays.ts Outdated Show resolved Hide resolved
liamdebeasi and others added 5 commits March 12, 2024 14:35
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
@liamdebeasi liamdebeasi added this pull request to the merge queue Mar 12, 2024
Merged via the queue into main with commit c0f5e5e Mar 12, 2024
46 checks passed
@liamdebeasi liamdebeasi deleted the FW-6041 branch March 12, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Overlays now have aria-hidden=true when displaying a Toast
3 participants