bug: focus is not trapped when presenting an overlay from another overlay

jeroenkroese commented 2 years ago

Prerequisites

[X] I have read the Contributing Guidelines.
[X] I agree to follow the Code of Conduct.
[X] I have searched for existing issues that already report this problem, without success.

Ionic Framework Version

[ ] v4.x
[ ] v5.x
[X] v6.x
[ ] Nightly

Current Behavior

When an alert is opened from an action sheet, it doesn't immediately trap the focus within the alert. Instead, the focus cycles through the underlying page elements up until it reaches the alert buttons. Once it has reached the alert button, the focus remains properly trapped.

Expected Behavior

The focus should be immediately trapped within the alert, just like it would when opening the alert from a button.

Steps to Reproduce

Open action sheet by clicking ellipsis icon in the top-right
Click action sheet button to open an alert
Hit "Tab" key

Code Reproduction URL

https://github.com/jeroenkroese/IonicAlertFocusBug

Ionic Info

[WARN] Error loading @capacitor/ios package.json: Error: Cannot find module '@capacitor/ios/package'

   Require stack:
   - /usr/local/lib/node_modules/@ionic/cli/lib/project/index.js
   - /usr/local/lib/node_modules/@ionic/cli/lib/index.js
   - /usr/local/lib/node_modules/@ionic/cli/index.js
   - /usr/local/lib/node_modules/@ionic/cli/bin/ionic

[WARN] Error loading @capacitor/android package.json: Error: Cannot find module '@capacitor/android/package'

   Require stack:
   - /usr/local/lib/node_modules/@ionic/cli/lib/project/index.js
   - /usr/local/lib/node_modules/@ionic/cli/lib/index.js
   - /usr/local/lib/node_modules/@ionic/cli/index.js
   - /usr/local/lib/node_modules/@ionic/cli/bin/ionic

Ionic:

Ionic CLI : 6.20.1 (/usr/local/lib/node_modules/@ionic/cli) Ionic Framework : @ionic/angular 6.2.8 @angular-devkit/build-angular : 14.2.3 @angular-devkit/schematics : 14.2.3 @angular/cli : 14.2.3 @ionic/angular-toolkit : 6.1.0

Capacitor:

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

Utility:

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

System:

NodeJS : v16.16.0 (/usr/local/Cellar/node@16/16.16.0/bin/node) npm : 8.18.0 OS : macOS Monterey

Additional Information

No response

sean-perkins commented 2 years ago

@jeroenkroese thanks for reporting this bug!

I see the same behavior as you describe. It does look like the ion-alert receives focus when presented, but then the button behind the alert receives focus.

Focus should be trapped within the presented alert. We will track this as a bug and prioritize.

liamdebeasi commented 2 years ago

This line is the culprit: https://github.com/ionic-team/ionic-framework/blob/23470f6429916052e635f7c6a3c1acbf297c361d/core/src/utils/overlays.ts#L478

When an overlay is dismissed, we return focus to the previously focused element (typically the element that was clicked to present the overlay). However, this code always assumes that no other overlay is present. In this case, focus is returned to the ellipsis button.

edit: Updated title to be more specific. Issue is related to the underlying focus trapping logic rather than one particular component.

ionic-team / ionic-framework