Skip to content

Commit f332f62

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

File tree

7 files changed

+92
-17
lines changed

7 files changed

+92
-17
lines changed

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

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,11 @@ export const createSheetGesture = (
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
100100
const el = baseEl as HTMLIonModalElement & { focusTrap?: boolean; showBackdrop?: boolean };
101-
if (el.focusTrap === false || el.showBackdrop === false) {
101+
const focusTrapAttr = el.getAttribute?.('focus-trap');
102+
const showBackdropAttr = el.getAttribute?.('show-backdrop');
103+
const focusTrapDisabled = el.focusTrap === false || focusTrapAttr === 'false';
104+
const backdropDisabled = el.showBackdrop === false || showBackdropAttr === 'false';
105+
if (focusTrapDisabled || backdropDisabled) {
102106
return;
103107
}
104108
baseEl.style.setProperty('pointer-events', 'auto');
@@ -241,10 +245,12 @@ export const createSheetGesture = (
241245
* ion-backdrop and .modal-wrapper always have pointer-events: auto
242246
* applied, so the modal content can still be interacted with.
243247
*/
244-
const shouldEnableBackdrop =
245-
currentBreakpoint > backdropBreakpoint &&
246-
(baseEl as HTMLIonModalElement & { focusTrap?: boolean }).focusTrap !== false &&
247-
(baseEl as HTMLIonModalElement & { showBackdrop?: boolean }).showBackdrop !== false;
248+
const modalEl = baseEl as HTMLIonModalElement & { focusTrap?: boolean; showBackdrop?: boolean };
249+
const focusTrapAttr = modalEl.getAttribute?.('focus-trap');
250+
const showBackdropAttr = modalEl.getAttribute?.('show-backdrop');
251+
const focusTrapDisabled = modalEl.focusTrap === false || focusTrapAttr === 'false';
252+
const backdropDisabled = modalEl.showBackdrop === false || showBackdropAttr === 'false';
253+
const shouldEnableBackdrop = currentBreakpoint > backdropBreakpoint && !focusTrapDisabled && !backdropDisabled;
248254
if (shouldEnableBackdrop) {
249255
enableBackdrop();
250256
} else {
@@ -591,10 +597,16 @@ export const createSheetGesture = (
591597
* Backdrop should become enabled
592598
* after the backdropBreakpoint value
593599
*/
600+
const modalEl = baseEl as HTMLIonModalElement & {
601+
focusTrap?: boolean;
602+
showBackdrop?: boolean;
603+
};
604+
const focusTrapAttr = modalEl.getAttribute?.('focus-trap');
605+
const showBackdropAttr = modalEl.getAttribute?.('show-backdrop');
606+
const focusTrapDisabled = modalEl.focusTrap === false || focusTrapAttr === 'false';
607+
const backdropDisabled = modalEl.showBackdrop === false || showBackdropAttr === 'false';
594608
const shouldEnableBackdrop =
595-
currentBreakpoint > backdropBreakpoint &&
596-
(baseEl as HTMLIonModalElement & { focusTrap?: boolean }).focusTrap !== false &&
597-
(baseEl as HTMLIonModalElement & { showBackdrop?: boolean }).showBackdrop !== false;
609+
currentBreakpoint > backdropBreakpoint && !focusTrapDisabled && !backdropDisabled;
598610
if (shouldEnableBackdrop) {
599611
enableBackdrop();
600612
} else {

core/src/components/modal/modal.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1237,6 +1237,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
12371237
const isHandleCycle = handleBehavior === 'cycle';
12381238
const isSheetModalWithHandle = isSheetModal && showHandle;
12391239

1240+
const focusTrapAttr = this.el.getAttribute('focus-trap');
12401241
return (
12411242
<Host
12421243
no-router
@@ -1253,7 +1254,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
12531254
[`modal-sheet`]: isSheetModal,
12541255
[`modal-no-expand-scroll`]: isSheetModal && !expandToScroll,
12551256
'overlay-hidden': true,
1256-
[FOCUS_TRAP_DISABLE_CLASS]: focusTrap === false,
1257+
[FOCUS_TRAP_DISABLE_CLASS]: focusTrap === false || focusTrapAttr === 'false',
12571258
...getClassMap(this.cssClass),
12581259
}}
12591260
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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,7 @@ export class Popover implements ComponentInterface, PopoverInterface {
687687
const { onLifecycle, parentPopover, dismissOnSelect, side, arrow, htmlAttributes, focusTrap } = this;
688688
const desktop = isPlatform('desktop');
689689
const enableArrow = arrow && !parentPopover;
690+
const focusTrapAttr = this.el.getAttribute('focus-trap');
690691

691692
return (
692693
<Host
@@ -704,7 +705,7 @@ export class Popover implements ComponentInterface, PopoverInterface {
704705
'overlay-hidden': true,
705706
'popover-desktop': desktop,
706707
[`popover-side-${side}`]: true,
707-
[FOCUS_TRAP_DISABLE_CLASS]: focusTrap === false,
708+
[FOCUS_TRAP_DISABLE_CLASS]: focusTrap === false || focusTrapAttr === 'false',
708709
'popover-nested': !!parentPopover,
709710
}}
710711
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: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -539,11 +539,18 @@ 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 & {
543+
focusTrap?: boolean;
544+
showBackdrop?: boolean;
545+
};
546+
const focusTrapAttr = overlayEl.getAttribute?.('focus-trap');
547+
const showBackdropAttr = overlayEl.getAttribute?.('show-backdrop');
548+
const focusTrapDisabled = overlayEl.focusTrap === false || focusTrapAttr === 'false';
549+
const backdropDisabled = overlayEl.showBackdrop === false || showBackdropAttr === 'false';
550+
const shouldTrapFocus = overlayEl.tagName !== 'ION-TOAST' && !focusTrapDisabled;
544551
// Only lock out root content when backdrop is active. Developers relying on showBackdrop=false
545552
// expect background interaction to remain enabled.
546-
const shouldLockRoot = shouldTrapFocus && overlayEl.showBackdrop !== false;
553+
const shouldLockRoot = shouldTrapFocus && !backdropDisabled;
547554

548555
overlay.presented = true;
549556
overlay.willPresent.emit();
@@ -681,11 +688,21 @@ export const dismiss = async <OverlayDismissOptions>(
681688
*/
682689
const overlaysLockingRoot = presentedOverlays.filter((o) => {
683690
const el = o as HTMLIonOverlayElement & { focusTrap?: boolean; showBackdrop?: boolean };
684-
return el.tagName !== 'ION-TOAST' && el.focusTrap !== false && el.showBackdrop !== false;
691+
const focusTrapAttr = el.getAttribute?.('focus-trap');
692+
const showBackdropAttr = el.getAttribute?.('show-backdrop');
693+
const focusTrapDisabled = el.focusTrap === false || focusTrapAttr === 'false';
694+
const backdropDisabled = el.showBackdrop === false || showBackdropAttr === 'false';
695+
return el.tagName !== 'ION-TOAST' && !focusTrapDisabled && !backdropDisabled;
685696
});
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;
697+
const overlayEl = overlay.el as HTMLIonOverlayElement & {
698+
focusTrap?: boolean;
699+
showBackdrop?: boolean;
700+
};
701+
const focusTrapAttr = overlayEl.getAttribute?.('focus-trap');
702+
const showBackdropAttr = overlayEl.getAttribute?.('show-backdrop');
703+
const focusTrapDisabled = overlayEl.focusTrap === false || focusTrapAttr === 'false';
704+
const backdropDisabled = overlayEl.showBackdrop === false || showBackdropAttr === 'false';
705+
const locksRoot = overlayEl.tagName !== 'ION-TOAST' && !focusTrapDisabled && !backdropDisabled;
689706

690707
/**
691708
* 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)