Skip to content

Commit a40d957

Browse files
authored
fix(modal): allow sheet modals to skip focus trap (#30689)
Issue number: resolves #30684 --------- <!-- 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. --> Recently, we [fixed some issues with aria-hidden in modals](#30563), unfortunately at this time we neglected modals that opt out of focus trapping. As a result, a lot of modals that disable focus trapping still have it happening and it doesn't get cleaned up properly on dismiss. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> We're now properly checking for and skipping focus traps on modals that do not want them. ## Does this introduce a breaking change? - [ ] Yes - [X] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information I created regression tests for Angular in this to prevent this from happening again. I initially tried to do this with core, but the issue doesn't seem to reproduce with core. <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Current dev build: ``` 8.7.5-dev.11758652700.103435a3 ```
1 parent 5a06503 commit a40d957

35 files changed

+1094
-25
lines changed

core/src/components/modal/gestures/sheet.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ export const createSheetGesture = (
9595
const contentAnimation = animation.childAnimations.find((ani) => ani.id === 'contentAnimation');
9696

9797
const enableBackdrop = () => {
98+
// Respect explicit opt-out of focus trapping/backdrop interactions
99+
// If focusTrap is false or showBackdrop is false, do not enable the backdrop or re-enable focus trap
100+
const el = baseEl as HTMLIonModalElement & { focusTrap?: boolean; showBackdrop?: boolean };
101+
if (el.focusTrap === false || el.showBackdrop === false) {
102+
return;
103+
}
98104
baseEl.style.setProperty('pointer-events', 'auto');
99105
backdropEl.style.setProperty('pointer-events', 'auto');
100106

@@ -235,7 +241,10 @@ export const createSheetGesture = (
235241
* ion-backdrop and .modal-wrapper always have pointer-events: auto
236242
* applied, so the modal content can still be interacted with.
237243
*/
238-
const shouldEnableBackdrop = currentBreakpoint > backdropBreakpoint;
244+
const shouldEnableBackdrop =
245+
currentBreakpoint > backdropBreakpoint &&
246+
(baseEl as HTMLIonModalElement & { focusTrap?: boolean }).focusTrap !== false &&
247+
(baseEl as HTMLIonModalElement & { showBackdrop?: boolean }).showBackdrop !== false;
239248
if (shouldEnableBackdrop) {
240249
enableBackdrop();
241250
} else {
@@ -582,7 +591,10 @@ export const createSheetGesture = (
582591
* Backdrop should become enabled
583592
* after the backdropBreakpoint value
584593
*/
585-
const shouldEnableBackdrop = currentBreakpoint > backdropBreakpoint;
594+
const shouldEnableBackdrop =
595+
currentBreakpoint > backdropBreakpoint &&
596+
(baseEl as HTMLIonModalElement & { focusTrap?: boolean }).focusTrap !== false &&
597+
(baseEl as HTMLIonModalElement & { showBackdrop?: boolean }).showBackdrop !== false;
586598
if (shouldEnableBackdrop) {
587599
enableBackdrop();
588600
} else {

core/src/utils/overlays.ts

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -494,10 +494,8 @@ export const setRootAriaHidden = (hidden = false) => {
494494

495495
if (hidden) {
496496
viewContainer.setAttribute('aria-hidden', 'true');
497-
viewContainer.setAttribute('inert', '');
498497
} else {
499498
viewContainer.removeAttribute('aria-hidden');
500-
viewContainer.removeAttribute('inert');
501499
}
502500
};
503501

@@ -529,15 +527,37 @@ export const present = async <OverlayPresentOptions>(
529527
* focus traps.
530528
*
531529
* All other overlays should have focus traps to prevent
532-
* the keyboard focus from leaving the overlay.
530+
* the keyboard focus from leaving the overlay unless
531+
* developers explicitly opt out (for example, sheet
532+
* modals that should permit background interaction).
533+
*
534+
* Note: Some apps move inline overlays to a specific container
535+
* during the willPresent lifecycle (e.g., React portals via
536+
* onWillPresent). Defer applying aria-hidden/inert to the app
537+
* root until after willPresent so we can detect where the
538+
* overlay is finally inserted. If the overlay is inside the
539+
* view container subtree, skip adding aria-hidden/inert there
540+
* to avoid disabling the overlay.
533541
*/
534-
if (overlay.el.tagName !== 'ION-TOAST') {
535-
setRootAriaHidden(true);
536-
document.body.classList.add(BACKDROP_NO_SCROLL);
537-
}
542+
const overlayEl = overlay.el as HTMLIonOverlayElement & { focusTrap?: boolean; showBackdrop?: boolean };
543+
const shouldTrapFocus = overlayEl.tagName !== 'ION-TOAST' && overlayEl.focusTrap !== false;
544+
// Only lock out root content when backdrop is active. Developers relying on showBackdrop=false
545+
// expect background interaction to remain enabled.
546+
const shouldLockRoot = shouldTrapFocus && overlayEl.showBackdrop !== false;
538547

539548
overlay.presented = true;
540549
overlay.willPresent.emit();
550+
551+
if (shouldLockRoot) {
552+
const root = getAppRoot(document);
553+
const viewContainer = root.querySelector('ion-router-outlet, #ion-view-container-root');
554+
const overlayInsideViewContainer = viewContainer ? viewContainer.contains(overlayEl) : false;
555+
556+
if (!overlayInsideViewContainer) {
557+
setRootAriaHidden(true);
558+
}
559+
document.body.classList.add(BACKDROP_NO_SCROLL);
560+
}
541561
overlay.willPresentShorthand?.emit();
542562

543563
const mode = getIonMode(overlay);
@@ -653,22 +673,28 @@ export const dismiss = async <OverlayDismissOptions>(
653673
* For accessibility, toasts lack focus traps and don't receive
654674
* `aria-hidden` on the root element when presented.
655675
*
656-
* All other overlays use focus traps to keep keyboard focus
657-
* within the overlay, setting `aria-hidden` on the root element
658-
* to enhance accessibility.
659-
*
660-
* Therefore, we must remove `aria-hidden` from the root element
661-
* when the last non-toast overlay is dismissed.
676+
* Overlays that opt into focus trapping set `aria-hidden`
677+
* on the root element to keep keyboard focus and pointer
678+
* events inside the overlay. We must remove `aria-hidden`
679+
* from the root element when the last focus-trapping overlay
680+
* is dismissed.
662681
*/
663-
const overlaysNotToast = presentedOverlays.filter((o) => o.tagName !== 'ION-TOAST');
664-
665-
const lastOverlayNotToast = overlaysNotToast.length === 1 && overlaysNotToast[0].id === overlay.el.id;
682+
const overlaysLockingRoot = presentedOverlays.filter((o) => {
683+
const el = o as HTMLIonOverlayElement & { focusTrap?: boolean; showBackdrop?: boolean };
684+
return el.tagName !== 'ION-TOAST' && el.focusTrap !== false && el.showBackdrop !== false;
685+
});
686+
const overlayEl = overlay.el as HTMLIonOverlayElement & { focusTrap?: boolean; showBackdrop?: boolean };
687+
const locksRoot =
688+
overlayEl.tagName !== 'ION-TOAST' && overlayEl.focusTrap !== false && overlayEl.showBackdrop !== false;
666689

667690
/**
668-
* If this is the last visible overlay that is not a toast
691+
* If this is the last visible overlay that is trapping focus
669692
* then we want to re-add the root to the accessibility tree.
670693
*/
671-
if (lastOverlayNotToast) {
694+
const lastOverlayTrappingFocus =
695+
locksRoot && overlaysLockingRoot.length === 1 && overlaysLockingRoot[0].id === overlayEl.id;
696+
697+
if (lastOverlayTrappingFocus) {
672698
setRootAriaHidden(false);
673699
document.body.classList.remove(BACKDROP_NO_SCROLL);
674700
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { expect, test } from '@playwright/test';
2+
3+
test.describe('Modals: Dynamic Wrapper', () => {
4+
test.beforeEach(async ({ page }) => {
5+
await page.goto('/lazy/modal-dynamic-wrapper');
6+
});
7+
8+
test('should render dynamic component inside modal', async ({ page }) => {
9+
await page.locator('#open-dynamic-modal').click();
10+
11+
await expect(page.locator('ion-modal')).toBeVisible();
12+
await expect(page.locator('#dynamic-component-loaded')).toBeVisible();
13+
});
14+
15+
test('should allow interacting with background content while sheet is open', async ({ page }) => {
16+
await page.locator('#open-dynamic-modal').click();
17+
18+
await expect(page.locator('ion-modal')).toBeVisible();
19+
20+
await page.locator('#background-action').click();
21+
22+
await expect(page.locator('#background-action-count')).toHaveText('1');
23+
});
24+
25+
test('should prevent interacting with background content when focus is trapped', async ({ page }) => {
26+
await page.locator('#open-focused-modal').click();
27+
28+
await expect(page.locator('ion-modal')).toBeVisible();
29+
30+
// Attempt to click the background button via coordinates; click should be intercepted by backdrop
31+
const box = await page.locator('#background-action').boundingBox();
32+
if (box) {
33+
await page.mouse.click(box.x + box.width / 2, box.y + box.height / 2);
34+
}
35+
36+
await expect(page.locator('#background-action-count')).toHaveText('0');
37+
});
38+
});
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { expect, test } from '@playwright/test';
2+
3+
test.describe('Modals: Inline Sheet', () => {
4+
test.beforeEach(async ({ page }) => {
5+
await page.goto('/lazy/modal-sheet-inline');
6+
});
7+
8+
test('should open inline sheet modal', async ({ page }) => {
9+
await page.locator('#present-inline-sheet-modal').click();
10+
11+
await expect(page.locator('ion-modal')).toBeVisible();
12+
await expect(page.locator('#current-breakpoint')).toHaveText('0.2');
13+
await expect(page.locator('ion-modal ion-item')).toHaveCount(4);
14+
});
15+
16+
test('should expand to 0.75 breakpoint when searchbar is clicked', async ({ page }) => {
17+
await page.locator('#present-inline-sheet-modal').click();
18+
await expect(page.locator('#current-breakpoint')).toHaveText('0.2');
19+
20+
await page.locator('ion-modal ion-searchbar').click();
21+
22+
await expect(page.locator('#current-breakpoint')).toHaveText('0.75');
23+
});
24+
25+
test('should allow interacting with background content while sheet is open', async ({ page }) => {
26+
await page.locator('#present-inline-sheet-modal').click();
27+
28+
await expect(page.locator('ion-modal')).toBeVisible();
29+
30+
await page.locator('#background-action').click();
31+
32+
await expect(page.locator('#background-action-count')).toHaveText('1');
33+
});
34+
});
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { expect, test } from '@playwright/test';
2+
3+
test.describe('Modals: Dynamic Wrapper (standalone)', () => {
4+
test.beforeEach(async ({ page }) => {
5+
await page.goto('/standalone/modal-dynamic-wrapper');
6+
});
7+
8+
test('should render dynamic component inside modal', async ({ page }) => {
9+
await page.locator('#open-dynamic-modal').click();
10+
11+
await expect(page.locator('ion-modal')).toBeVisible();
12+
await expect(page.locator('#dynamic-component-loaded')).toBeVisible();
13+
});
14+
15+
test('should allow interacting with background content while sheet is open', async ({ page }) => {
16+
await page.locator('#open-dynamic-modal').click();
17+
18+
await expect(page.locator('ion-modal')).toBeVisible();
19+
20+
await page.locator('#background-action').click();
21+
22+
await expect(page.locator('#background-action-count')).toHaveText('1');
23+
});
24+
25+
test('should prevent interacting with background content when focus is trapped', async ({ page }) => {
26+
await page.locator('#open-focused-modal').click();
27+
28+
await expect(page.locator('ion-modal')).toBeVisible();
29+
30+
// Attempt to click the background button via coordinates; click should be intercepted by backdrop
31+
const box = await page.locator('#background-action').boundingBox();
32+
if (box) {
33+
await page.mouse.click(box.x + box.width / 2, box.y + box.height / 2);
34+
}
35+
36+
await expect(page.locator('#background-action-count')).toHaveText('0');
37+
});
38+
});
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { expect, test } from '@playwright/test';
2+
3+
test.describe('Modals: Inline Sheet (standalone)', () => {
4+
test.beforeEach(async ({ page }) => {
5+
await page.goto('/standalone/modal-sheet-inline');
6+
});
7+
8+
test('should open inline sheet modal', async ({ page }) => {
9+
await page.locator('#present-inline-sheet-modal').click();
10+
11+
await expect(page.locator('ion-modal')).toBeVisible();
12+
await expect(page.locator('#current-breakpoint')).toHaveText('0.2');
13+
await expect(page.locator('ion-modal ion-item')).toHaveCount(4);
14+
});
15+
16+
test('should expand to 0.75 breakpoint when searchbar is clicked', async ({ page }) => {
17+
await page.locator('#present-inline-sheet-modal').click();
18+
await expect(page.locator('#current-breakpoint')).toHaveText('0.2');
19+
20+
await page.locator('ion-modal ion-searchbar').click();
21+
22+
await expect(page.locator('#current-breakpoint')).toHaveText('0.75');
23+
});
24+
25+
test('should allow interacting with background content while sheet is open', async ({ page }) => {
26+
await page.locator('#present-inline-sheet-modal').click();
27+
28+
await expect(page.locator('ion-modal')).toBeVisible();
29+
30+
await page.locator('#background-action').click();
31+
32+
await expect(page.locator('#background-action-count')).toHaveText('1');
33+
});
34+
});

packages/angular/test/base/src/app/lazy/app-lazy/app.routes.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ export const routes: Routes = [
3737
{ path: 'template-form', component: TemplateFormComponent },
3838
{ path: 'modals', component: ModalComponent },
3939
{ path: 'modal-inline', loadChildren: () => import('../modal-inline').then(m => m.ModalInlineModule) },
40+
{ path: 'modal-sheet-inline', loadChildren: () => import('../modal-sheet-inline').then(m => m.ModalSheetInlineModule) },
41+
{ path: 'modal-dynamic-wrapper', loadChildren: () => import('../modal-dynamic-wrapper').then(m => m.ModalDynamicWrapperModule) },
4042
{ path: 'view-child', component: ViewChildComponent },
4143
{ path: 'keep-contents-mounted', loadChildren: () => import('../keep-contents-mounted').then(m => m.OverlayAutoMountModule) },
4244
{ path: 'overlays-inline', loadChildren: () => import('../overlays-inline').then(m => m.OverlaysInlineModule) },
@@ -90,4 +92,3 @@ export const routes: Routes = [
9092
]
9193
},
9294
];
93-

packages/angular/test/base/src/app/lazy/home-page/home-page.component.html

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@
3535
Modals Test
3636
</ion-label>
3737
</ion-item>
38+
<ion-item routerLink="/lazy/modal-sheet-inline">
39+
<ion-label>
40+
Modal Sheet Inline Test
41+
</ion-label>
42+
</ion-item>
43+
<ion-item routerLink="/lazy/modal-dynamic-wrapper">
44+
<ion-label>
45+
Modal Dynamic Wrapper Test
46+
</ion-label>
47+
</ion-item>
3848
<ion-item routerLink="/lazy/router-link">
3949
<ion-label>
4050
Router link Test
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { Component, ComponentRef, Input, OnDestroy, OnInit, ViewChild, ViewContainerRef } from "@angular/core";
2+
3+
@Component({
4+
selector: 'app-dynamic-component-wrapper',
5+
template: `
6+
<ion-content>
7+
<ng-container #container></ng-container>
8+
</ion-content>
9+
`,
10+
standalone: false
11+
})
12+
export class DynamicComponentWrapperComponent implements OnInit, OnDestroy {
13+
14+
@Input() componentRef?: ComponentRef<unknown>;
15+
@ViewChild('container', { read: ViewContainerRef, static: true }) container!: ViewContainerRef;
16+
17+
ngOnInit(): void {
18+
if (this.componentRef) {
19+
this.container.insert(this.componentRef.hostView);
20+
}
21+
}
22+
23+
ngOnDestroy(): void {
24+
this.componentRef?.destroy();
25+
}
26+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { Component, EventEmitter, Output } from "@angular/core";
2+
3+
@Component({
4+
selector: 'app-dynamic-modal-content',
5+
template: `
6+
<ion-header>
7+
<ion-toolbar>
8+
<ion-title>Dynamic Sheet Content</ion-title>
9+
</ion-toolbar>
10+
</ion-header>
11+
<ion-content class="ion-padding">
12+
<p id="dynamic-component-loaded">Dynamic component rendered inside wrapper.</p>
13+
<ion-button id="dismiss-dynamic-modal" (click)="dismiss.emit()">Close</ion-button>
14+
</ion-content>
15+
`,
16+
standalone: false
17+
})
18+
export class DynamicModalContentComponent {
19+
@Output() dismiss = new EventEmitter<void>();
20+
}

0 commit comments

Comments
 (0)