Skip to content

Commit 8da777b

Browse files
ShaneKShaneK
committed
fix(sheet): disable focus trap with string-based logic as well
1 parent 3b80473 commit 8da777b

File tree

7 files changed

+74
-20
lines changed

7 files changed

+74
-20
lines changed

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,10 @@ export const createSheetGesture = (
9797
const enableBackdrop = () => {
9898
// Respect explicit opt-out of focus trapping/backdrop interactions
9999
// 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) {
100+
const el = baseEl as HTMLIonModalElement & { focusTrap?: boolean | string; showBackdrop?: boolean | string };
101+
const focusTrapDisabled = el.focusTrap === false || el.focusTrap === 'false';
102+
const backdropDisabled = el.showBackdrop === false || el.showBackdrop === 'false';
103+
if (focusTrapDisabled || backdropDisabled) {
102104
return;
103105
}
104106
baseEl.style.setProperty('pointer-events', 'auto');
@@ -241,10 +243,10 @@ export const createSheetGesture = (
241243
* ion-backdrop and .modal-wrapper always have pointer-events: auto
242244
* applied, so the modal content can still be interacted with.
243245
*/
244-
const shouldEnableBackdrop =
245-
currentBreakpoint > backdropBreakpoint &&
246-
(baseEl as HTMLIonModalElement & { focusTrap?: boolean }).focusTrap !== false &&
247-
(baseEl as HTMLIonModalElement & { showBackdrop?: boolean }).showBackdrop !== false;
246+
const modalEl = baseEl as HTMLIonModalElement & { focusTrap?: boolean | string; showBackdrop?: boolean | string };
247+
const focusTrapDisabled = modalEl.focusTrap === false || modalEl.focusTrap === 'false';
248+
const backdropDisabled = modalEl.showBackdrop === false || modalEl.showBackdrop === 'false';
249+
const shouldEnableBackdrop = currentBreakpoint > backdropBreakpoint && !focusTrapDisabled && !backdropDisabled;
248250
if (shouldEnableBackdrop) {
249251
enableBackdrop();
250252
} else {
@@ -591,10 +593,13 @@ export const createSheetGesture = (
591593
* Backdrop should become enabled
592594
* after the backdropBreakpoint value
593595
*/
594-
const shouldEnableBackdrop =
595-
currentBreakpoint > backdropBreakpoint &&
596-
(baseEl as HTMLIonModalElement & { focusTrap?: boolean }).focusTrap !== false &&
597-
(baseEl as HTMLIonModalElement & { showBackdrop?: boolean }).showBackdrop !== false;
596+
const modalEl = baseEl as HTMLIonModalElement & {
597+
focusTrap?: boolean | string;
598+
showBackdrop?: boolean | string;
599+
};
600+
const focusTrapDisabled = modalEl.focusTrap === false || modalEl.focusTrap === 'false';
601+
const backdropDisabled = modalEl.showBackdrop === false || modalEl.showBackdrop === 'false';
602+
const shouldEnableBackdrop = currentBreakpoint > backdropBreakpoint && !focusTrapDisabled && !backdropDisabled;
598603
if (shouldEnableBackdrop) {
599604
enableBackdrop();
600605
} else {

‎core/src/components/modal/modal.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1253,7 +1253,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
12531253
[`modal-sheet`]: isSheetModal,
12541254
[`modal-no-expand-scroll`]: isSheetModal && !expandToScroll,
12551255
'overlay-hidden': true,
1256-
[FOCUS_TRAP_DISABLE_CLASS]: focusTrap === false,
1256+
[FOCUS_TRAP_DISABLE_CLASS]: focusTrap === false || focusTrap === 'false',
12571257
...getClassMap(this.cssClass),
12581258
}}
12591259
onIonBackdropTap={this.onBackdropTap}

‎core/src/components/modal/test/basic/modal.spec.tsx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,18 @@ describe('modal: focus trap', () => {
2828

2929
expect(modal.classList.contains(FOCUS_TRAP_DISABLE_CLASS)).toBe(true);
3030
});
31+
it('should set the focus trap class when disabled via attribute string', async () => {
32+
const page = await newSpecPage({
33+
components: [Modal],
34+
html: `
35+
<ion-modal focus-trap="false"></ion-modal>
36+
`,
37+
});
38+
39+
const modal = page.body.querySelector('ion-modal')!;
40+
41+
expect(modal.classList.contains(FOCUS_TRAP_DISABLE_CLASS)).toBe(true);
42+
});
3143
it('should not set the focus trap class by default', async () => {
3244
const page = await newSpecPage({
3345
components: [Modal],

‎core/src/components/popover/popover.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,7 @@ export class Popover implements ComponentInterface, PopoverInterface {
704704
'overlay-hidden': true,
705705
'popover-desktop': desktop,
706706
[`popover-side-${side}`]: true,
707-
[FOCUS_TRAP_DISABLE_CLASS]: focusTrap === false,
707+
[FOCUS_TRAP_DISABLE_CLASS]: focusTrap === false || focusTrap === 'false',
708708
'popover-nested': !!parentPopover,
709709
}}
710710
onIonPopoverDidPresent={onLifecycle}

‎core/src/components/popover/test/basic/popover.spec.tsx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@ describe('popover: focus trap', () => {
2929

3030
expect(popover.classList.contains(FOCUS_TRAP_DISABLE_CLASS)).toBe(true);
3131
});
32+
it('should set the focus trap class when disabled via attribute string', async () => {
33+
const page = await newSpecPage({
34+
components: [Popover],
35+
html: `
36+
<ion-popover focus-trap="false"></ion-popover>
37+
`,
38+
});
39+
40+
const popover = page.body.querySelector('ion-popover')!;
41+
42+
expect(popover.classList.contains(FOCUS_TRAP_DISABLE_CLASS)).toBe(true);
43+
});
3244
it('should not set the focus trap class by default', async () => {
3345
const page = await newSpecPage({
3446
components: [Popover],

‎core/src/utils/overlays.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -539,11 +539,13 @@ export const present = async <OverlayPresentOptions>(
539539
* view container subtree, skip adding aria-hidden/inert there
540540
* to avoid disabling the overlay.
541541
*/
542-
const overlayEl = overlay.el as HTMLIonOverlayElement & { focusTrap?: boolean; showBackdrop?: boolean };
543-
const shouldTrapFocus = overlayEl.tagName !== 'ION-TOAST' && overlayEl.focusTrap !== false;
542+
const overlayEl = overlay.el as HTMLIonOverlayElement & { focusTrap?: boolean | string; showBackdrop?: boolean | string };
543+
const focusTrapDisabled = overlayEl.focusTrap === false || overlayEl.focusTrap === 'false';
544+
const backdropDisabled = overlayEl.showBackdrop === false || overlayEl.showBackdrop === 'false';
545+
const shouldTrapFocus = overlayEl.tagName !== 'ION-TOAST' && !focusTrapDisabled;
544546
// Only lock out root content when backdrop is active. Developers relying on showBackdrop=false
545547
// expect background interaction to remain enabled.
546-
const shouldLockRoot = shouldTrapFocus && overlayEl.showBackdrop !== false;
548+
const shouldLockRoot = shouldTrapFocus && !backdropDisabled;
547549

548550
overlay.presented = true;
549551
overlay.willPresent.emit();
@@ -680,12 +682,15 @@ export const dismiss = async <OverlayDismissOptions>(
680682
* is dismissed.
681683
*/
682684
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+
const el = o as HTMLIonOverlayElement & { focusTrap?: boolean | string; showBackdrop?: boolean | string };
686+
const focusTrapDisabled = el.focusTrap === false || el.focusTrap === 'false';
687+
const backdropDisabled = el.showBackdrop === false || el.showBackdrop === 'false';
688+
return el.tagName !== 'ION-TOAST' && !focusTrapDisabled && !backdropDisabled;
685689
});
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;
690+
const overlayEl = overlay.el as HTMLIonOverlayElement & { focusTrap?: boolean | string; showBackdrop?: boolean | string };
691+
const focusTrapDisabled = overlayEl.focusTrap === false || overlayEl.focusTrap === 'false';
692+
const backdropDisabled = overlayEl.showBackdrop === false || overlayEl.showBackdrop === 'false';
693+
const locksRoot = overlayEl.tagName !== 'ION-TOAST' && !focusTrapDisabled && !backdropDisabled;
689694

690695
/**
691696
* If this is the last visible overlay that is trapping focus

‎core/src/utils/test/overlays/overlays-scroll-blocking.spec.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,26 @@ describe('overlays: scroll blocking', () => {
3737
expect(body).not.toHaveClass('backdrop-no-scroll');
3838
});
3939

40+
it('should not block scroll when focus-trap attribute is set to "false"', async () => {
41+
const page = await newSpecPage({
42+
components: [Modal],
43+
html: `
44+
<ion-modal focus-trap="false"></ion-modal>
45+
`,
46+
});
47+
48+
const modal = page.body.querySelector('ion-modal')!;
49+
const body = page.doc.querySelector('body')!;
50+
51+
await modal.present();
52+
53+
expect(body).not.toHaveClass('backdrop-no-scroll');
54+
55+
await modal.dismiss();
56+
57+
expect(body).not.toHaveClass('backdrop-no-scroll');
58+
});
59+
4060
it('should not block scroll when the overlay is dismissed', async () => {
4161
const page = await newSpecPage({
4262
components: [Modal],

0 commit comments

Comments
 (0)