Skip to content

Commit 4437209

Browse files
committed
fix(modal): cleaning up some repetitive code, adding links for test pages, caching page parent to prevent excessive searching
1 parent 4ad27fb commit 4437209

File tree

4 files changed

+38
-25
lines changed

4 files changed

+38
-25
lines changed

core/src/components/modal/modal.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ export class Modal implements ComponentInterface, OverlayInterface {
100100
private parentRemovalObserver?: MutationObserver;
101101
// Cached original parent from before modal is moved to body during presentation
102102
private cachedOriginalParent?: HTMLElement;
103+
// Cached ion-page ancestor for child route passthrough
104+
private cachedPageParent?: HTMLElement | null;
103105

104106
lastFocus?: HTMLElement;
105107
animation?: Animation;
@@ -778,7 +780,9 @@ export class Modal implements ComponentInterface, OverlayInterface {
778780
* passthrough on child route page wrappers and nested router outlets.
779781
*/
780782
private setupChildRoutePassthrough() {
781-
const pageParent = this.getOriginalPageParent();
783+
// Cache the page parent for cleanup
784+
this.cachedPageParent = this.getOriginalPageParent();
785+
const pageParent = this.cachedPageParent;
782786

783787
// Skip ion-app (controller modals) and pages with other content (inline modals)
784788
if (!pageParent || pageParent.tagName === 'ION-APP') {
@@ -827,7 +831,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
827831
* Removes passthrough styles added by setupChildRoutePassthrough.
828832
*/
829833
private cleanupChildRoutePassthrough() {
830-
const pageParent = this.getOriginalPageParent();
834+
const pageParent = this.cachedPageParent;
831835
if (!pageParent) {
832836
return;
833837
}
@@ -839,6 +843,9 @@ export class Modal implements ComponentInterface, OverlayInterface {
839843
routerOutlet.style.removeProperty('pointer-events');
840844
routerOutlet.removeAttribute('data-overlay-passthrough');
841845
}
846+
847+
// Clear the cached reference
848+
this.cachedPageParent = undefined;
842849
}
843850

844851
private sheetOnDismiss() {

core/src/utils/overlays.ts

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,20 @@ let lastId = 0;
3838

3939
export const activeAnimations = new WeakMap<OverlayInterface, Animation[]>();
4040

41+
type OverlayWithFocusTrapProps = HTMLIonOverlayElement & {
42+
focusTrap?: boolean;
43+
showBackdrop?: boolean;
44+
backdropBreakpoint?: number;
45+
};
46+
47+
/**
48+
* Determines if the overlay's backdrop is always active (no background interaction).
49+
* Returns false if showBackdrop=false or backdropBreakpoint > 0.
50+
*/
51+
const isBackdropAlwaysActive = (el: OverlayWithFocusTrapProps): boolean => {
52+
return el.showBackdrop !== false && !((el.backdropBreakpoint ?? 0) > 0);
53+
};
54+
4155
const createController = <Opts extends object, HTMLElm>(tagName: string) => {
4256
return {
4357
create(options: Opts): Promise<HTMLElm> {
@@ -539,16 +553,9 @@ export const present = async <OverlayPresentOptions>(
539553
* view container subtree, skip adding aria-hidden/inert there
540554
* to avoid disabling the overlay.
541555
*/
542-
const overlayEl = overlay.el as HTMLIonOverlayElement & {
543-
focusTrap?: boolean;
544-
showBackdrop?: boolean;
545-
backdropBreakpoint?: number;
546-
};
556+
const overlayEl = overlay.el as OverlayWithFocusTrapProps;
547557
const shouldTrapFocus = overlayEl.tagName !== 'ION-TOAST' && overlayEl.focusTrap !== false;
548-
// Only lock out root content when backdrop is always active. Developers relying on
549-
// showBackdrop=false or backdropBreakpoint expect background interaction at some point.
550-
const backdropAlwaysActive = overlayEl.showBackdrop !== false && !((overlayEl.backdropBreakpoint ?? 0) > 0);
551-
const shouldLockRoot = shouldTrapFocus && backdropAlwaysActive;
558+
const shouldLockRoot = shouldTrapFocus && isBackdropAlwaysActive(overlayEl);
552559

553560
overlay.presented = true;
554561
overlay.willPresent.emit();
@@ -685,21 +692,11 @@ export const dismiss = async <OverlayDismissOptions>(
685692
* is dismissed.
686693
*/
687694
const overlaysLockingRoot = presentedOverlays.filter((o) => {
688-
const el = o as HTMLIonOverlayElement & {
689-
focusTrap?: boolean;
690-
showBackdrop?: boolean;
691-
backdropBreakpoint?: number;
692-
};
693-
const backdropAlwaysActive = el.showBackdrop !== false && !((el.backdropBreakpoint ?? 0) > 0);
694-
return el.tagName !== 'ION-TOAST' && el.focusTrap !== false && backdropAlwaysActive;
695+
const el = o as OverlayWithFocusTrapProps;
696+
return el.tagName !== 'ION-TOAST' && el.focusTrap !== false && isBackdropAlwaysActive(el);
695697
});
696-
const overlayEl = overlay.el as HTMLIonOverlayElement & {
697-
focusTrap?: boolean;
698-
showBackdrop?: boolean;
699-
backdropBreakpoint?: number;
700-
};
701-
const backdropAlwaysActive = overlayEl.showBackdrop !== false && !((overlayEl.backdropBreakpoint ?? 0) > 0);
702-
const locksRoot = overlayEl.tagName !== 'ION-TOAST' && overlayEl.focusTrap !== false && backdropAlwaysActive;
698+
const overlayEl = overlay.el as OverlayWithFocusTrapProps;
699+
const locksRoot = overlayEl.tagName !== 'ION-TOAST' && overlayEl.focusTrap !== false && isBackdropAlwaysActive(overlayEl);
703700

704701
/**
705702
* If this is the last visible overlay that is trapping focus

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@
100100
Modal Dynamic Wrapper Test
101101
</ion-label>
102102
</ion-item>
103+
<ion-item routerLink="/standalone/modal-child-route">
104+
<ion-label>
105+
Modal Child Route Test
106+
</ion-label>
107+
</ion-item>
103108
<ion-item routerLink="/standalone/programmatic-modal">
104109
<ion-label>
105110
Programmatic Modal Test

packages/react/test/base/src/pages/overlay-components/OverlayComponents.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ const OverlayHooks: React.FC<OverlayHooksProps> = () => {
6464
<IonIcon icon={star} />
6565
<IonLabel>Modal Teleport</IonLabel>
6666
</IonTabButton>
67+
<IonTabButton tab="modalSheetChildRoute" href="/overlay-components/modal-sheet-child-route/child">
68+
<IonIcon icon={star} />
69+
<IonLabel>Sheet Child</IonLabel>
70+
</IonTabButton>
6771
<IonTabButton tab="picker" href="/overlay-components/picker">
6872
<IonIcon icon={logoIonic} />
6973
<IonLabel>Picker</IonLabel>

0 commit comments

Comments
 (0)