Skip to content

Commit 5c59d59

Browse files
committed
fix(modal): prevent safe-area flash for custom-dimension modals on phone viewports
1 parent 05ce997 commit 5c59d59

4 files changed

Lines changed: 88 additions & 25 deletions

File tree

core/src/components/content/content.scss

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@
2828
--keyboard-offset: 0px;
2929
--offset-top: 0px;
3030
--offset-bottom: 0px;
31-
// Internal: set by modal applyFullscreenSafeArea() for modals without footer. Not a public API.
32-
--ion-content-safe-area-padding-bottom: 0px;
3331
--overflow: auto;
3432

3533
display: block;
@@ -64,6 +62,12 @@
6462
background: var(--background);
6563
}
6664

65+
/**
66+
* --ion-content-safe-area-padding-bottom is an internal property set by
67+
* modal.tsx for fullscreen modals without an ion-footer. This decouples
68+
* safe-area-bottom scroll padding from --padding-bottom (which is a
69+
* public property consumers may override).
70+
*/
6771
.inner-scroll {
6872
@include position(calc(var(--offset-top) * -1), 0px,calc(var(--offset-bottom) * -1), 0px);
6973
@include padding(calc(var(--padding-top) + var(--offset-top)), var(--padding-end), calc(var(--padding-bottom) + var(--keyboard-offset) + var(--offset-bottom) + var(--ion-content-safe-area-padding-bottom, 0px)), var(--padding-start));

core/src/components/modal/modal.tsx

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
applySafeAreaOverrides,
5151
clearSafeAreaOverrides,
5252
getRootSafeAreaTop,
53+
hasCustomModalDimensions,
5354
type ModalSafeAreaContext,
5455
} from './safe-area-utils';
5556
import { setCardStatusBarDark, setCardStatusBarDefault } from './utils';
@@ -312,11 +313,8 @@ export class Modal implements ComponentInterface, OverlayInterface {
312313
this.updateSafeAreaOverrides();
313314

314315
// Re-evaluate fullscreen safe-area padding: clear first, then re-apply.
315-
// Single findContentAndFooter() call avoids redundant DOM traversal.
316316
const { contentEl, hasFooter } = this.findContentAndFooter();
317-
if (contentEl) {
318-
contentEl.style.removeProperty('--ion-content-safe-area-padding-bottom');
319-
}
317+
this.clearContentSafeAreaPadding(contentEl);
320318
this.applyFullscreenSafeAreaTo(contentEl, hasFooter);
321319
}
322320
}, 50); // Debounce to avoid excessive calls during active resizing
@@ -1430,6 +1428,11 @@ export class Modal implements ComponentInterface, OverlayInterface {
14301428

14311429
/**
14321430
* Creates the context object for safe-area utilities.
1431+
*
1432+
* `hasCustomDimensions` is only set by `setInitialSafeAreaOverrides()`
1433+
* because it is only read by `getInitialSafeAreaConfig()`. Other callers
1434+
* (resize handler, post-animation update, fullscreen-padding apply) would
1435+
* pay a `getComputedStyle()` cost for a value they never consult.
14331436
*/
14341437
private getSafeAreaContext(): ModalSafeAreaContext {
14351438
return {
@@ -1452,7 +1455,10 @@ export class Modal implements ComponentInterface, OverlayInterface {
14521455
* sheets to prevent header content from getting double-offset padding).
14531456
*/
14541457
private setInitialSafeAreaOverrides(): void {
1455-
const context = this.getSafeAreaContext();
1458+
const context: ModalSafeAreaContext = {
1459+
...this.getSafeAreaContext(),
1460+
hasCustomDimensions: hasCustomModalDimensions(this.el),
1461+
};
14561462
const safeAreaConfig = getInitialSafeAreaConfig(context);
14571463
applySafeAreaOverrides(this.el, safeAreaConfig);
14581464

@@ -1526,14 +1532,13 @@ export class Modal implements ComponentInterface, OverlayInterface {
15261532
}
15271533

15281534
/**
1529-
* Clears --ion-content-safe-area-padding-bottom from ion-content inside
1530-
* this modal.
1535+
* Removes the internal --ion-content-safe-area-padding-bottom property
1536+
* from an already-located ion-content. Callers do their own
1537+
* findContentAndFooter() so they can also read hasFooter if needed.
15311538
*/
1532-
private clearContentSafeAreaPadding(): void {
1533-
const { contentEl } = this.findContentAndFooter();
1534-
if (contentEl) {
1535-
contentEl.style.removeProperty('--ion-content-safe-area-padding-bottom');
1536-
}
1539+
private clearContentSafeAreaPadding(contentEl: HTMLElement | null): void {
1540+
if (!contentEl) return;
1541+
contentEl.style.removeProperty('--ion-content-safe-area-padding-bottom');
15371542
}
15381543

15391544
/**
@@ -1553,7 +1558,6 @@ export class Modal implements ComponentInterface, OverlayInterface {
15531558
for (const child of Array.from(this.el.children)) {
15541559
if (child.tagName === 'ION-CONTENT') contentEl = child as HTMLElement;
15551560
if (child.tagName === 'ION-FOOTER') hasFooter = true;
1556-
// Only search grandchildren for content if we haven't found one yet
15571561
for (const grandchild of Array.from(child.children)) {
15581562
if (grandchild.tagName === 'ION-CONTENT' && !contentEl) contentEl = grandchild as HTMLElement;
15591563
if (grandchild.tagName === 'ION-FOOTER') hasFooter = true;
@@ -1571,14 +1575,8 @@ export class Modal implements ComponentInterface, OverlayInterface {
15711575
// Remove internal sheet offset property
15721576
this.el.style.removeProperty('--ion-modal-offset-top');
15731577

1574-
// Remove legacy wrapper styles (kept for safety in case they were
1575-
// set before a live update applied this fix)
1576-
if (this.wrapperEl) {
1577-
this.wrapperEl.style.removeProperty('height');
1578-
this.wrapperEl.style.removeProperty('padding-bottom');
1579-
}
1580-
1581-
this.clearContentSafeAreaPadding();
1578+
const { contentEl } = this.findContentAndFooter();
1579+
this.clearContentSafeAreaPadding(contentEl);
15821580
}
15831581

15841582
render() {

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

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ export interface ModalSafeAreaContext {
2323
presentingElement?: HTMLElement;
2424
breakpoints?: number[];
2525
currentBreakpoint?: number;
26+
/**
27+
* Only consulted by `getInitialSafeAreaConfig()`. Callers that only use the
28+
* context for non-initial paths can omit this. See `hasCustomModalDimensions()`.
29+
*/
30+
hasCustomDimensions?: boolean;
2631
}
2732

2833
/**
@@ -38,6 +43,13 @@ const MODAL_INSET_MIN_WIDTH = 768;
3843
const MODAL_INSET_MIN_HEIGHT = 600;
3944
const EDGE_THRESHOLD = 5;
4045

46+
/**
47+
* CSS values for `--width` / `--height` that are treated as fullscreen
48+
* (modal touches the corresponding screen edges). Empty string means the
49+
* property was not overridden. See `hasCustomModalDimensions()`.
50+
*/
51+
const FULLSCREEN_SIZE_VALUES = new Set(['', '100%', '100vw', '100vh', '100dvw', '100dvh', '100svw', '100svh']);
52+
4153
/**
4254
* Cache for resolved root safe-area-top value, invalidated once per frame.
4355
*/
@@ -92,6 +104,23 @@ export const getRootSafeAreaTop = (): number => {
92104
return value;
93105
};
94106

107+
/**
108+
* True when the modal host declares BOTH a non-fullscreen `--width` AND a
109+
* non-fullscreen `--height` (i.e. a centered-dialog-like modal that doesn't
110+
* touch any screen edge).
111+
*
112+
* The conservative "both axes" check avoids mis-zeroing safe-area for
113+
* partial-custom modals where the modal still touches top/bottom edges
114+
* (e.g. only `--width` overridden). Partial cases fall through to the
115+
* existing position-based post-animation correction.
116+
*/
117+
export const hasCustomModalDimensions = (hostEl: HTMLElement): boolean => {
118+
const styles = getComputedStyle(hostEl);
119+
const width = styles.getPropertyValue('--width').trim();
120+
const height = styles.getPropertyValue('--height').trim();
121+
return !FULLSCREEN_SIZE_VALUES.has(width) && !FULLSCREEN_SIZE_VALUES.has(height);
122+
};
123+
95124
/**
96125
* Returns the initial safe-area configuration based on modal type.
97126
* This is called before animation starts and uses configuration-based prediction.
@@ -129,8 +158,11 @@ export const getInitialSafeAreaConfig = (context: ModalSafeAreaContext): SafeAre
129158

130159
// On viewports that meet the centered dialog media query breakpoints,
131160
// regular modals render as centered dialogs (not fullscreen), so they
132-
// don't touch any screen edges and don't need safe-area insets.
133-
if (isCenteredDialogViewport()) {
161+
// don't touch any screen edges and don't need safe-area insets. Also
162+
// applies to phone viewports when the modal declares custom --width and
163+
// --height; these don't touch screen edges either, so the initial
164+
// prediction must be zero to avoid a post-animation correction flash.
165+
if (isCenteredDialogViewport() || context.hasCustomDimensions) {
134166
return {
135167
top: '0px',
136168
bottom: '0px',

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,35 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config
322322
expect(safeAreaBottom).toBe('0px');
323323
});
324324

325+
test('centered dialog with custom dimensions on phone should zero safe-area from initial prediction', async ({
326+
page,
327+
}, testInfo) => {
328+
testInfo.annotations.push({
329+
type: 'issue',
330+
description: 'https://github.com/ionic-team/ionic-framework/issues/31015',
331+
});
332+
333+
// Stay on phone viewport. This is the path where the centered-dialog
334+
// media query does NOT match but the modal still doesn't touch screen
335+
// edges because cssClass sets --width/--height. Without the initial
336+
// prediction catching this, safe-area flashes inherited values and
337+
// then snaps to 0px after animation.
338+
const ionModalWillPresent = await page.spyOnEvent('ionModalWillPresent');
339+
await page.click('#centered-dialog');
340+
await ionModalWillPresent.next();
341+
342+
// Read inline style IMMEDIATELY after will-present fires, before the
343+
// animation finishes. This captures the initial prediction value.
344+
const modal = page.locator('ion-modal');
345+
const initial = await modal.evaluate((el: HTMLIonModalElement) => ({
346+
top: el.style.getPropertyValue('--ion-safe-area-top'),
347+
bottom: el.style.getPropertyValue('--ion-safe-area-bottom'),
348+
}));
349+
350+
expect(initial.top).toBe('0px');
351+
expect(initial.bottom).toBe('0px');
352+
});
353+
325354
test('safe-area overrides should be cleared on dismiss', async ({ page }, testInfo) => {
326355
testInfo.annotations.push({
327356
type: 'issue',

0 commit comments

Comments
 (0)