From fcc50d6d167bbb2da6b9d35e2f212e9e89069588 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Fri, 30 Jan 2026 07:01:34 -0800 Subject: [PATCH] fix(modal): fixing safe area in certain situations, adding some tests --- core/src/components/modal/modal.tsx | 50 ++++++++++------- .../modal/test/safe-area/index.html | 55 +++++++++++++++++-- .../popover/test/safe-area/index.html | 27 ++++++++- 3 files changed, 105 insertions(+), 27 deletions(-) diff --git a/core/src/components/modal/modal.tsx b/core/src/components/modal/modal.tsx index bf57c36f07..c41ade9c72 100644 --- a/core/src/components/modal/modal.tsx +++ b/core/src/components/modal/modal.tsx @@ -75,6 +75,7 @@ export class Modal implements ComponentInterface, OverlayInterface { @State() private isSheetModal = false; private currentBreakpoint?: number; private wrapperEl?: HTMLElement; + private shadowEl?: HTMLElement; private backdropEl?: HTMLIonBackdropElement; private dragHandleEl?: HTMLButtonElement; private sortedBreakpoints?: number[]; @@ -914,13 +915,9 @@ export class Modal implements ComponentInterface, OverlayInterface { return; } - // Phone-sized fullscreen modals inherit safe areas and use wrapper padding - if (!isTablet) { - this.applyFullscreenSafeArea(); - return; - } - - // Check if tablet modal is fullscreen via CSS custom properties + // Check if modal is fullscreen via CSS custom properties + // This applies to both phone and tablet sizes - custom modals may have + // non-fullscreen dimensions even on phones (e.g., --height: 70%) const computedStyle = getComputedStyle(this.el); const width = computedStyle.getPropertyValue('--width').trim(); const height = computedStyle.getPropertyValue('--height').trim(); @@ -928,9 +925,12 @@ export class Modal implements ComponentInterface, OverlayInterface { if (isFullscreen) { this.applyFullscreenSafeArea(); - } else { - // Centered dialog doesn't touch edges + } else if (isTablet) { + // Centered dialog on tablet doesn't touch edges this.zeroAllSafeAreas(); + } else { + // Non-fullscreen modal on phone - use coordinate-based detection + // to determine which edges it touches (e.g., bottom-aligned custom modals) } } @@ -953,19 +953,27 @@ export class Modal implements ComponentInterface, OverlayInterface { } /** - * Updates wrapper padding based on footer presence. + * Updates wrapper and shadow padding based on footer presence. * Called initially and when footer is dynamically added/removed. + * Both elements must be styled identically to prevent visual mismatches. */ private updateFooterPadding() { if (!this.wrapperEl) return; const hasFooter = this.el.querySelector('ion-footer') !== null; + // Apply to both wrapper and shadow to keep them in sync + const elements = [this.wrapperEl, this.shadowEl].filter(Boolean) as HTMLElement[]; + if (hasFooter) { - this.wrapperEl.style.removeProperty('padding-bottom'); - this.wrapperEl.style.removeProperty('box-sizing'); + elements.forEach((el) => { + el.style.removeProperty('padding-bottom'); + el.style.removeProperty('box-sizing'); + }); } else { - this.wrapperEl.style.setProperty('padding-bottom', 'var(--ion-safe-area-bottom, 0px)'); - this.wrapperEl.style.setProperty('box-sizing', 'border-box'); + elements.forEach((el) => { + el.style.setProperty('padding-bottom', 'var(--ion-safe-area-bottom, 0px)'); + el.style.setProperty('box-sizing', 'border-box'); + }); } } @@ -993,11 +1001,13 @@ export class Modal implements ComponentInterface, OverlayInterface { this.footerObserver?.disconnect(); this.footerObserver = undefined; - // Clear wrapper styles that may have been set for safe-area handling - if (this.wrapperEl) { - this.wrapperEl.style.removeProperty('padding-bottom'); - this.wrapperEl.style.removeProperty('box-sizing'); - } + // Clear wrapper and shadow styles that may have been set for safe-area handling + [this.wrapperEl, this.shadowEl].forEach((el) => { + if (el) { + el.style.removeProperty('padding-bottom'); + el.style.removeProperty('box-sizing'); + } + }); // Clear safe-area CSS variable overrides const style = this.el.style; @@ -1613,7 +1623,7 @@ export class Modal implements ComponentInterface, OverlayInterface { part="backdrop" /> - {mode === 'ios' && } + {mode === 'ios' && }
/** * Simulate safe-area insets for testing. - * These values represent typical iPad safe areas. + * Values represent combined scenarios: top/bottom from portrait devices, + * left/right from landscape orientation or devices with side notches. */ :root { --ion-safe-area-top: 44px; --ion-safe-area-bottom: 34px; - --ion-safe-area-left: 0px; - --ion-safe-area-right: 0px; + --ion-safe-area-left: 44px; + --ion-safe-area-right: 44px; } .fullscreen-modal { --width: 100%; --height: 100%; } + + /* Visual indicators for safe areas */ + .safe-area-indicator { + position: fixed; + background: rgba(255, 0, 0, 0.2); + pointer-events: none; + z-index: 99999; + } + + .safe-area-top { + top: 0; + left: 0; + right: 0; + height: var(--ion-safe-area-top); + } + + .safe-area-bottom { + bottom: 0; + left: 0; + right: 0; + height: var(--ion-safe-area-bottom); + } + + .safe-area-left { + top: 0; + bottom: 0; + left: 0; + width: var(--ion-safe-area-left); + } + + .safe-area-right { + top: 0; + bottom: 0; + right: 0; + width: var(--ion-safe-area-right); + } + +
+
+
+
+
@@ -41,7 +84,11 @@ -

Test safe-area handling in modals.

+

Test safe-area handling in modals. Red overlays indicate safe areas (top, bottom, left, right).

+

+ Landscape simulation: Left and right safe areas are set to 44px to test devices with side + notches. +

diff --git a/core/src/components/popover/test/safe-area/index.html b/core/src/components/popover/test/safe-area/index.html index 271d5fa02c..cb65329f5f 100644 --- a/core/src/components/popover/test/safe-area/index.html +++ b/core/src/components/popover/test/safe-area/index.html @@ -16,12 +16,13 @@ /** * Simulate safe-area insets for testing. * These values represent typical Android edge-to-edge safe areas. + * Left/right values simulate landscape orientation or devices with side notches. */ :root { --ion-safe-area-top: 44px; --ion-safe-area-bottom: 34px; - --ion-safe-area-left: 0px; - --ion-safe-area-right: 0px; + --ion-safe-area-left: 44px; + --ion-safe-area-right: 44px; } /* Visual indicator for safe areas */ @@ -46,6 +47,20 @@ height: var(--ion-safe-area-bottom); } + .safe-area-left { + top: 0; + bottom: 0; + left: 0; + width: var(--ion-safe-area-left); + } + + .safe-area-right { + top: 0; + bottom: 0; + right: 0; + width: var(--ion-safe-area-right); + } + /* Position triggers at different locations */ .bottom-trigger { position: fixed; @@ -67,6 +82,8 @@
+
+
@@ -77,7 +94,11 @@

Test that popovers are positioned away from unsafe areas (shown in red).

-

The popover should be moved up/down to avoid overlapping the safe-area zones.

+

The popover should be moved up/down/left/right to avoid overlapping the safe-area zones.

+

+ Landscape simulation: Left and right safe areas are set to 44px to test devices with side + notches. +