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

bug: ion-router-outlet not marked aria-hidden when multiple overlays are presented and some dismissed #28180

Closed
3 tasks done
nagashimam opened this issue Sep 15, 2023 · 4 comments · Fixed by #28183
Closed
3 tasks done
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@nagashimam
Copy link

nagashimam commented Sep 15, 2023

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

When multiple overlays are presented, and some of them are dismissed, the ion-router-outlet element isn't marked as aria-hidden.

As a result, assistive technologies like screen readers can access contents within the ion-router-outlet element, even though they are visually obscured by the remaining overlays.

Expected Behavior

The ion-router-outlet element should remain marked as aria-hidden even after some of the overlays are dismissed, and it should be marked as not aria-hidden only after all overlays are dismissed.

Steps to Reproduce

To reproduce the bug, follow these steps:

  1. Access the StackBlitz page.

    • At this point, confirm that ion-router-outlet isn't marked as aria-hidden.
  2. Click the "OPEN ION-MODAL" button.

    • Confirm that ion-router-outlet is marked as aria-hidden.
  3. Click the "OPEN ION-ALERT" button.

    • Once again, confirm that ion-router-outlet is marked as aria-hidden.
  4. Click the "OK" button.

    • Now, confirm that ion-router-outlet is NOT marked as aria-hidden.

Code Reproduction URL

StackBlitz
GitHub repo

Ionic Info

Ionic:

Ionic CLI : 7.1.1 (/Users/mac/.anyenv/envs/nodenv/versions/18.17.1/lib/node_modules/@ionic/cli)
Ionic Framework : @ionic/angular 7.3.3
@angular-devkit/build-angular : 16.2.1
@angular-devkit/schematics : 16.2.1
@angular/cli : 16.2.1
@ionic/angular-toolkit : 9.0.0

Capacitor:

Capacitor CLI : 5.3.0
@capacitor/android : not installed
@capacitor/core : 5.3.0
@capacitor/ios : not installed

Utility:

cordova-res : not installed globally
native-run : 1.7.2

System:

NodeJS : v18.17.1 (/Users/mac/.anyenv/envs/nodenv/versions/18.17.1/bin/node)
npm : 9.6.7
OS : macOS Big Sur

Additional Information

Suggestions on how to fix

Currently, the dismiss function always calls setRootAriaHidden(false) in the ionic-framework/core/src/utils/overlays.ts file:

export const setRootAriaHidden = (hidden = false) => {
  // ...
};

export const dismiss = async <OverlayDismissOptions>(
  // ...
): Promise<boolean> => {
  // ...
  setRootAriaHidden(false);
}

To address the issue, we should consider calling setRootAriaHidden(false) only when there's a single overlay remaining:

export const dismiss = async <OverlayDismissOptions>(
  // ...
): Promise<boolean> => {
  // ...
  const overlays = getOverlays(document);
  if (overlays.length === 1) {
    setRootAriaHidden(false);
  }
}

This change ensures that the ion-router-outlet element is marked as not aria-hidden only when the last overlay is dismissed, which aligns with the expected behavior.

Screen Recordings

@ionitron-bot ionitron-bot bot added the triage label Sep 15, 2023
@nagashimam nagashimam changed the title bug: ion-router-outlet not marked aria-hidden with multiple overlays and some dismissed bug: ion-router-outlet not marked aria-hidden with multiple overlays are presented and some dismissed Sep 15, 2023
@nagashimam nagashimam changed the title bug: ion-router-outlet not marked aria-hidden with multiple overlays are presented and some dismissed bug: ion-router-outlet not marked aria-hidden when multiple overlays are presented and some dismissed Sep 15, 2023
@liamdebeasi liamdebeasi self-assigned this Sep 15, 2023
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Sep 15, 2023
@liamdebeasi liamdebeasi removed their assignment Sep 15, 2023
@liamdebeasi
Copy link
Contributor

Thanks for the report. Here is a dev build if you would like to test the proposed fix:

npm install @ionic/[email protected]

Note: You may need to install this in a local project. StackBlitz sometimes has trouble installing dev builds for Angular.

@nagashimam
Copy link
Author

@liamdebeasi
I tried the version in my local project, and it works as expected. Thank you for your quick response!

github-merge-queue bot pushed a commit that referenced this issue Sep 19, 2023
Issue number: resolves #28180

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

When presenting an overlay, we remove the root (usually
`ion-router-outlet`) from the accessibility tree. This makes it so you
cannot accidentally focus elements behind the overlay. When dismissing
an overlay we re-add the root to the accessibility tree. However, we
fail to consider if there are multiple presented overlays. For example,
if you present a modal, then an alert, then dismiss the alert, then the
root is re-added to the accessibility tree even though the modal is
still presented.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- The root is now re-added to the accessibility tree only if it is the
last presented overlay.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.4.1-dev.11694783260.13da477f`
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #28183, and a fix will be available in an upcoming release of Ionic Framework.

liamdebeasi added a commit that referenced this issue Sep 22, 2023
Issue number: resolves #28180

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

When presenting an overlay, we remove the root (usually
`ion-router-outlet`) from the accessibility tree. This makes it so you
cannot accidentally focus elements behind the overlay. When dismissing
an overlay we re-add the root to the accessibility tree. However, we
fail to consider if there are multiple presented overlays. For example,
if you present a modal, then an alert, then dismiss the alert, then the
root is re-added to the accessibility tree even though the modal is
still presented.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- The root is now re-added to the accessibility tree only if it is the
last presented overlay.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.4.1-dev.11694783260.13da477f`
liamdebeasi added a commit that referenced this issue Sep 26, 2023
Issue number: resolves #28180

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

When presenting an overlay, we remove the root (usually
`ion-router-outlet`) from the accessibility tree. This makes it so you
cannot accidentally focus elements behind the overlay. When dismissing
an overlay we re-add the root to the accessibility tree. However, we
fail to consider if there are multiple presented overlays. For example,
if you present a modal, then an alert, then dismiss the alert, then the
root is re-added to the accessibility tree even though the modal is
still presented.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- The root is now re-added to the accessibility tree only if it is the
last presented overlay.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.4.1-dev.11694783260.13da477f`
@ionitron-bot
Copy link

ionitron-bot bot commented Oct 19, 2023

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants