Skip to content

Commit 31f9f58

Browse files
committed
fix(modal): decouple sheet height from safe-area and update on resize
1 parent 6f216f0 commit 31f9f58

4 files changed

Lines changed: 183 additions & 25 deletions

File tree

core/src/components/modal/modal.scss

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,14 @@ ion-backdrop {
152152
/**
153153
* Ensure that the sheet modal does not
154154
* completely cover the content.
155+
*
156+
* --ion-modal-offset-top is an internal property set by modal.tsx
157+
* with the resolved root safe-area-top pixel value. This decouples
158+
* the height calculation from --ion-safe-area-top (which is zeroed
159+
* for sheet modals to prevent header double-padding).
155160
*/
156161
:host(.modal-sheet) {
157-
--height: calc(100% - (var(--ion-safe-area-top) + 10px));
162+
--height: calc(100% - (var(--ion-modal-offset-top, 0px) + 10px));
158163
}
159164

160165
:host(.modal-sheet) .modal-wrapper,

core/src/components/modal/modal.tsx

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
getPositionBasedSafeAreaConfig,
5050
applySafeAreaOverrides,
5151
clearSafeAreaOverrides,
52+
getRootSafeAreaTop,
5253
type ModalSafeAreaContext,
5354
} from './safe-area-utils';
5455
import { setCardStatusBarDark, setCardStatusBarDefault } from './utils';
@@ -283,14 +284,35 @@ export class Modal implements ComponentInterface, OverlayInterface {
283284

284285
@Listen('resize', { target: 'window' })
285286
onWindowResize() {
286-
// Only handle resize for iOS card modals when no custom animations are provided
287-
if (getIonMode(this) !== 'ios' || !this.presentingElement || this.enterAnimation || this.leaveAnimation) {
288-
return;
289-
}
287+
if (!this.presented) return;
290288

291289
clearTimeout(this.resizeTimeout);
292290
this.resizeTimeout = setTimeout(() => {
293-
this.handleViewTransition();
291+
const context = this.getSafeAreaContext();
292+
293+
// iOS card modals: handle portrait/landscape view transitions
294+
if (context.isCardModal && !this.enterAnimation && !this.leaveAnimation) {
295+
this.handleViewTransition();
296+
}
297+
298+
// Sheet modals: re-compute the internal offset property since safe-area
299+
// values may change on device rotation (e.g., portrait notch vs landscape).
300+
if (context.isSheetModal) {
301+
this.updateSheetOffsetTop();
302+
}
303+
304+
// Regular (non-sheet, non-card) modals: update safe-area overrides
305+
// since the viewport may have crossed the centered-dialog breakpoint.
306+
if (!context.isSheetModal && !context.isCardModal) {
307+
this.updateSafeAreaOverrides();
308+
309+
// Re-evaluate fullscreen safe-area padding: clear first, then re-apply
310+
if (this.wrapperEl) {
311+
this.wrapperEl.style.removeProperty('height');
312+
this.wrapperEl.style.removeProperty('padding-bottom');
313+
}
314+
this.applyFullscreenSafeArea();
315+
}
294316
}, 50); // Debounce to avoid excessive calls during active resizing
295317
}
296318

@@ -770,8 +792,6 @@ export class Modal implements ComponentInterface, OverlayInterface {
770792
if (this.currentBreakpoint !== breakpoint) {
771793
this.currentBreakpoint = breakpoint;
772794
this.ionBreakpointDidChange.emit({ breakpoint });
773-
// Update safe-area overrides based on new position
774-
this.updateSafeAreaOverrides();
775795
}
776796
}
777797
);
@@ -903,6 +923,10 @@ export class Modal implements ComponentInterface, OverlayInterface {
903923
return false;
904924
}
905925

926+
// Cancel any pending resize timeout to prevent stale updates during dismiss
927+
clearTimeout(this.resizeTimeout);
928+
this.resizeTimeout = undefined;
929+
906930
/**
907931
* Because the canDismiss check below is async,
908932
* we need to claim a lock before the check happens,
@@ -1374,11 +1398,32 @@ export class Modal implements ComponentInterface, OverlayInterface {
13741398
/**
13751399
* Sets initial safe-area overrides before modal animation.
13761400
* Called in present() before animation starts.
1401+
*
1402+
* For sheet modals, the SCSS --height formula uses --ion-modal-offset-top
1403+
* (an internal property) instead of --ion-safe-area-top. We resolve the
1404+
* root safe-area-top to pixels and set --ion-modal-offset-top, decoupling
1405+
* the height calculation from --ion-safe-area-top (which is zeroed for
1406+
* sheets to prevent header content from getting double-offset padding).
13771407
*/
13781408
private setInitialSafeAreaOverrides(): void {
13791409
const context = this.getSafeAreaContext();
13801410
const safeAreaConfig = getInitialSafeAreaConfig(context);
13811411
applySafeAreaOverrides(this.el, safeAreaConfig);
1412+
1413+
// Set the internal offset property with the resolved root safe-area-top value
1414+
if (context.isSheetModal) {
1415+
this.updateSheetOffsetTop();
1416+
}
1417+
}
1418+
1419+
/**
1420+
* Resolves the current root --ion-safe-area-top value and sets the
1421+
* internal --ion-modal-offset-top property on the host element.
1422+
* Called on present and on resize (e.g., device rotation changes safe-area).
1423+
*/
1424+
private updateSheetOffsetTop(): void {
1425+
const safeAreaTop = getRootSafeAreaTop();
1426+
this.el.style.setProperty('--ion-modal-offset-top', `${safeAreaTop}px`);
13821427
}
13831428

13841429
/**
@@ -1389,19 +1434,9 @@ export class Modal implements ComponentInterface, OverlayInterface {
13891434
const { wrapperEl, el } = this;
13901435
const context = this.getSafeAreaContext();
13911436

1392-
// Sheet modals: the wrapper extends beyond the viewport and is translated
1393-
// via breakpoint gestures, making getBoundingClientRect unreliable for
1394-
// edge detection. Instead, use breakpoint value to determine top safe-area.
1395-
if (context.isSheetModal) {
1396-
const needsTopSafeArea = context.currentBreakpoint === 1;
1397-
applySafeAreaOverrides(el, {
1398-
top: needsTopSafeArea ? 'inherit' : '0px',
1399-
bottom: 'inherit',
1400-
left: '0px',
1401-
right: '0px',
1402-
});
1403-
return;
1404-
}
1437+
// Sheet modals: safe-area is fully determined at presentation time
1438+
// (top is always 0px, height is frozen). Nothing to update.
1439+
if (context.isSheetModal) return;
14051440

14061441
// Card modals have fixed safe-area requirements set by initial prediction.
14071442
if (context.isCardModal) return;
@@ -1454,6 +1489,9 @@ export class Modal implements ComponentInterface, OverlayInterface {
14541489
private cleanupSafeAreaOverrides(): void {
14551490
clearSafeAreaOverrides(this.el);
14561491

1492+
// Remove internal sheet offset property
1493+
this.el.style.removeProperty('--ion-modal-offset-top');
1494+
14571495
if (this.wrapperEl) {
14581496
this.wrapperEl.style.removeProperty('height');
14591497
this.wrapperEl.style.removeProperty('padding-bottom');

core/src/components/modal/safe-area-utils.ts

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { win } from '@utils/browser';
2+
import { raf } from '@utils/helpers';
23

34
type SafeAreaValue = '0px' | 'inherit';
45

@@ -37,6 +38,12 @@ const MODAL_INSET_MIN_WIDTH = 768;
3738
const MODAL_INSET_MIN_HEIGHT = 600;
3839
const EDGE_THRESHOLD = 5;
3940

41+
/**
42+
* Cache for resolved root safe-area-top value, invalidated once per frame.
43+
*/
44+
let cachedRootSafeAreaTop: number | null = null;
45+
let cacheInvalidationScheduled = false;
46+
4047
/**
4148
* Determines if the current viewport meets the CSS media query conditions
4249
* that cause regular modals to render as centered dialogs instead of fullscreen.
@@ -48,6 +55,44 @@ const isCenteredDialogViewport = (): boolean => {
4855
.matches;
4956
};
5057

58+
/**
59+
* Resolves the current root --ion-safe-area-top value to pixels.
60+
* Uses a temporary element because getComputedStyle on :root returns
61+
* the declared value of custom properties (e.g. "env(safe-area-inset-top)")
62+
* rather than a resolved number.
63+
*
64+
* Results are cached for the current frame to avoid repeated reflows.
65+
*/
66+
export const getRootSafeAreaTop = (): number => {
67+
if (cachedRootSafeAreaTop !== null) {
68+
return cachedRootSafeAreaTop;
69+
}
70+
71+
const doc = win?.document;
72+
if (!doc?.body) {
73+
return 0;
74+
}
75+
76+
const el = doc.createElement('div');
77+
el.style.cssText =
78+
'position:fixed;visibility:hidden;pointer-events:none;top:0;left:0;' +
79+
'padding-top:var(--ion-safe-area-top,0px);';
80+
doc.body.appendChild(el);
81+
const value = parseFloat(getComputedStyle(el).paddingTop) || 0;
82+
el.remove();
83+
84+
cachedRootSafeAreaTop = value;
85+
if (!cacheInvalidationScheduled) {
86+
cacheInvalidationScheduled = true;
87+
raf(() => {
88+
cachedRootSafeAreaTop = null;
89+
cacheInvalidationScheduled = false;
90+
});
91+
}
92+
93+
return value;
94+
};
95+
5196
/**
5297
* Returns the initial safe-area configuration based on modal type.
5398
* This is called before animation starts and uses configuration-based prediction.
@@ -58,10 +103,14 @@ const isCenteredDialogViewport = (): boolean => {
58103
export const getInitialSafeAreaConfig = (context: ModalSafeAreaContext): SafeAreaConfig => {
59104
const { isSheetModal, isCardModal } = context;
60105

61-
// Sheet modals use bottom safe-area, and top safe-area only when fully expanded
106+
// Sheet modals always zero top safe-area. The sheet height offset from the
107+
// top edge is handled by --ion-modal-offset-top (set in modal.tsx) using
108+
// the resolved root value, so --ion-safe-area-top is never needed for
109+
// height calculation. Keeping it at 0px prevents header content from
110+
// getting double-offset padding.
62111
if (isSheetModal) {
63112
return {
64-
top: context.currentBreakpoint === 1 ? 'inherit' : '0px',
113+
top: '0px',
65114
bottom: 'inherit',
66115
left: '0px',
67116
right: '0px',

core/src/components/modal/test/safe-area/modal.e2e.ts

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ import { configs, test, Viewports } from '@utils/test/playwright';
88
* Safe-area handling is position-based and not affected by text direction.
99
* Testing only LTR to avoid redundant test runs.
1010
*/
11+
12+
/**
13+
* The test page (index.html) sets these root safe-area values.
14+
* Keep in sync with the :root block in test/safe-area/index.html.
15+
*/
16+
const TEST_SAFE_AREA_TOP = '47px';
1117
configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config }) => {
1218
test.describe(title('modal: safe-area handling'), () => {
1319
test.beforeEach(async ({ page }) => {
@@ -123,7 +129,7 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config
123129
expect(height).toBe('calc(var(--height) - var(--ion-safe-area-bottom, 0px))');
124130
});
125131

126-
test('sheet modal at breakpoint 1 should inherit top safe-area', async ({ page }, testInfo) => {
132+
test('sheet modal at breakpoint 1 should keep top safe-area zeroed', async ({ page }, testInfo) => {
127133
testInfo.annotations.push({
128134
type: 'issue',
129135
description: 'https://github.com/ionic-team/ionic-framework/issues/30900',
@@ -149,11 +155,71 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config
149155
});
150156
await ionBreakpointDidChange.next();
151157

152-
// At breakpoint 1, top safe-area should be inherited
158+
// At breakpoint 1, top safe-area should still be 0px because the
159+
// sheet height is frozen with the resolved root value. This prevents
160+
// header content from getting double-offset padding.
153161
safeAreaTop = await modal.evaluate((el: HTMLIonModalElement) => {
154162
return el.style.getPropertyValue('--ion-safe-area-top');
155163
});
164+
expect(safeAreaTop).toBe('0px');
165+
});
166+
167+
test('sheet modal should have --ion-modal-offset-top set with resolved safe-area value', async ({ page }, testInfo) => {
168+
testInfo.annotations.push({
169+
type: 'issue',
170+
description: 'https://github.com/ionic-team/ionic-framework/issues/30900',
171+
});
172+
173+
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
174+
175+
await page.click('#sheet-modal');
176+
await ionModalDidPresent.next();
177+
178+
const modal = page.locator('ion-modal');
179+
180+
// The internal --ion-modal-offset-top property should be set to the
181+
// resolved root --ion-safe-area-top value. The SCSS --height formula
182+
// uses this instead of --ion-safe-area-top directly.
183+
const offsetTop = await modal.evaluate((el: HTMLIonModalElement) => {
184+
return el.style.getPropertyValue('--ion-modal-offset-top');
185+
});
186+
expect(offsetTop).toBe(TEST_SAFE_AREA_TOP);
187+
});
188+
189+
test('fullscreen modal safe-area should update on resize from phone to tablet', async ({ page }, testInfo) => {
190+
testInfo.annotations.push({
191+
type: 'issue',
192+
description: 'https://github.com/ionic-team/ionic-framework/issues/30900',
193+
});
194+
195+
const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
196+
197+
await page.click('#fullscreen-modal');
198+
await ionModalDidPresent.next();
199+
200+
const modal = page.locator('ion-modal');
201+
202+
// On phone viewport, modal should inherit safe-area
203+
let safeAreaTop = await modal.evaluate((el: HTMLIonModalElement) => {
204+
return el.style.getPropertyValue('--ion-safe-area-top');
205+
});
156206
expect(safeAreaTop).toBe('inherit');
207+
208+
// Resize to tablet viewport (centered dialog breakpoint)
209+
await page.setViewportSize(Viewports.tablet.portrait);
210+
211+
// Poll until the debounced resize handler updates safe-area overrides
212+
await expect.poll(async () => {
213+
return modal.evaluate((el: HTMLIonModalElement) =>
214+
el.style.getPropertyValue('--ion-safe-area-top')
215+
);
216+
}).toBe('0px');
217+
218+
const safeAreaBottom = await modal.evaluate((el: HTMLIonModalElement) => {
219+
return el.style.getPropertyValue('--ion-safe-area-bottom');
220+
});
221+
222+
expect(safeAreaBottom).toBe('0px');
157223
});
158224

159225
test('centered dialog should have all safe-area values zeroed on tablet', async ({ page }, testInfo) => {

0 commit comments

Comments
 (0)