Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/components/content/content.scss
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@

.inner-scroll {
@include position(calc(var(--offset-top) * -1), 0px,calc(var(--offset-bottom) * -1), 0px);
@include padding(calc(var(--padding-top) + var(--offset-top)), var(--padding-end), calc(var(--padding-bottom) + var(--keyboard-offset) + var(--offset-bottom)), var(--padding-start));
@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));

position: absolute;

Expand Down
106 changes: 67 additions & 39 deletions core/src/components/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
applySafeAreaOverrides,
clearSafeAreaOverrides,
getRootSafeAreaTop,
hasCustomModalDimensions,
type ModalSafeAreaContext,
} from './safe-area-utils';
import { setCardStatusBarDark, setCardStatusBarDefault } from './utils';
Expand Down Expand Up @@ -311,12 +312,10 @@ export class Modal implements ComponentInterface, OverlayInterface {
if (!context.isSheetModal && !context.isCardModal) {
this.updateSafeAreaOverrides();

// Re-evaluate fullscreen safe-area padding: clear first, then re-apply
if (this.wrapperEl) {
this.wrapperEl.style.removeProperty('height');
this.wrapperEl.style.removeProperty('padding-bottom');
}
this.applyFullscreenSafeArea();
// Re-evaluate fullscreen safe-area padding: clear first, then re-apply.
const { contentEl, hasFooter } = this.findContentAndFooter();
this.clearContentSafeAreaPadding(contentEl);
this.applyFullscreenSafeAreaTo(contentEl, hasFooter);
}
}, 50); // Debounce to avoid excessive calls during active resizing
}
Expand Down Expand Up @@ -1429,6 +1428,11 @@ export class Modal implements ComponentInterface, OverlayInterface {

/**
* Creates the context object for safe-area utilities.
*
* `hasCustomDimensions` is only set by `setInitialSafeAreaOverrides()`
* because it is only read by `getInitialSafeAreaConfig()`. Other callers
* (resize handler, post-animation update, fullscreen-padding apply) would
* pay a `getComputedStyle()` cost for a value they never consult.
*/
private getSafeAreaContext(): ModalSafeAreaContext {
return {
Expand All @@ -1451,7 +1455,10 @@ export class Modal implements ComponentInterface, OverlayInterface {
* sheets to prevent header content from getting double-offset padding).
*/
private setInitialSafeAreaOverrides(): void {
const context = this.getSafeAreaContext();
const context: ModalSafeAreaContext = {
...this.getSafeAreaContext(),
hasCustomDimensions: hasCustomModalDimensions(this.el),
};
const safeAreaConfig = getInitialSafeAreaConfig(context);
applySafeAreaOverrides(this.el, safeAreaConfig);

Expand Down Expand Up @@ -1496,59 +1503,80 @@ export class Modal implements ComponentInterface, OverlayInterface {
}

/**
* Applies padding-bottom to fullscreen modal wrapper to prevent
* content from overlapping system navigation bar.
* Applies safe-area-bottom scroll padding to ion-content inside
* fullscreen modals that have no ion-footer. This prevents content
* from being hidden behind the system navigation bar while keeping
* the modal background edge-to-edge (no visible gap).
*/
private applyFullscreenSafeArea(): void {
const { wrapperEl, el } = this;
if (!wrapperEl) return;

const context = this.getSafeAreaContext();
if (context.isSheetModal || context.isCardModal) return;

// Check for standard Ionic layout children (ion-content, ion-footer),
// searching one level deep for wrapped components (e.g.,
// <app-footer><ion-footer>...</ion-footer></app-footer>).
// Note: uses a manual loop instead of querySelector(':scope > ...') because
// Stencil's mock-doc (used in spec tests) does not support :scope.
let hasContent = false;
const { contentEl, hasFooter } = this.findContentAndFooter();
this.applyFullscreenSafeAreaTo(contentEl, hasFooter);
}

/**
* Sets --ion-content-safe-area-padding-bottom on the given ion-content
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong but it seems that --ion-content-safe-area-padding-bottom is a a new variable to the project? If that's the case, can we comment that it's meant for internal usages and when to use it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure! Done: 559103a

* when no footer is present, so ion-content's .inner-scroll includes
* safe-area-bottom in its scroll padding. This keeps the modal background
* edge-to-edge while ensuring content scrolls clear of the system nav bar.
*/
private applyFullscreenSafeAreaTo(contentEl: HTMLElement | null, hasFooter: boolean): void {
// Only apply for standard Ionic layouts (has ion-content but no
// ion-footer). When a footer is present it handles its own safe-area
// padding. Custom modals with raw HTML are developer-controlled.
if (!contentEl || hasFooter) return;

contentEl.style.setProperty('--ion-content-safe-area-padding-bottom', 'var(--ion-safe-area-bottom, 0px)');
}

/**
* Removes the internal --ion-content-safe-area-padding-bottom property
* from an already-located ion-content. Callers do their own
* findContentAndFooter() so they can also read hasFooter if needed.
*/
private clearContentSafeAreaPadding(contentEl: HTMLElement | null): void {
if (!contentEl) return;
contentEl.style.removeProperty('--ion-content-safe-area-padding-bottom');
}

/**
* Finds ion-content and ion-footer among direct children and one level of
* grandchildren (for wrapped components like <app-footer><ion-footer>).
*
* Intentionally does NOT use findIonContent() or querySelector() because
* those search the full subtree and would match ion-content inside nested
* routes/pages. We only want direct slot children (+ one wrapper level).
*
* Uses a manual loop instead of querySelector(':scope > ...') because
* Stencil's mock-doc (used in spec tests) does not support :scope.
*/
private findContentAndFooter(): { contentEl: HTMLElement | null; hasFooter: boolean } {
let contentEl: HTMLElement | null = null;
let hasFooter = false;
for (const child of Array.from(el.children)) {
if (child.tagName === 'ION-CONTENT') hasContent = true;
for (const child of Array.from(this.el.children)) {
if (child.tagName === 'ION-CONTENT') contentEl = child as HTMLElement;
if (child.tagName === 'ION-FOOTER') hasFooter = true;
for (const grandchild of Array.from(child.children)) {
if (grandchild.tagName === 'ION-CONTENT') hasContent = true;
if (grandchild.tagName === 'ION-CONTENT' && !contentEl) contentEl = grandchild as HTMLElement;
if (grandchild.tagName === 'ION-FOOTER') hasFooter = true;
}
}

// Only apply wrapper padding for standard Ionic layouts (has ion-content
// but no ion-footer). Custom modals with raw HTML are fully
// developer-controlled and should not be modified.
if (!hasContent || hasFooter) return;

// Reduce wrapper height by safe-area and add equivalent padding so the
// total visual size stays the same but the flex content area shrinks.
// Using height + padding instead of box-sizing: border-box avoids
// breaking custom modals that set --border-width (border-box would
// include the border inside the height, changing the layout).
wrapperEl.style.setProperty('height', 'calc(var(--height) - var(--ion-safe-area-bottom, 0px))');
wrapperEl.style.setProperty('padding-bottom', 'var(--ion-safe-area-bottom, 0px)');
return { contentEl, hasFooter };
}

/**
* Clears all safe-area overrides and padding from wrapper.
* Clears all safe-area overrides and padding.
*/
private cleanupSafeAreaOverrides(): void {
clearSafeAreaOverrides(this.el);

// Remove internal sheet offset property
this.el.style.removeProperty('--ion-modal-offset-top');

if (this.wrapperEl) {
this.wrapperEl.style.removeProperty('height');
this.wrapperEl.style.removeProperty('padding-bottom');
}
const { contentEl } = this.findContentAndFooter();
this.clearContentSafeAreaPadding(contentEl);
}

render() {
Expand Down
36 changes: 34 additions & 2 deletions core/src/components/modal/safe-area-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ export interface ModalSafeAreaContext {
presentingElement?: HTMLElement;
breakpoints?: number[];
currentBreakpoint?: number;
/**
* Only consulted by `getInitialSafeAreaConfig()`. Callers that only use the
* context for non-initial paths can omit this. See `hasCustomModalDimensions()`.
*/
hasCustomDimensions?: boolean;
}

/**
Expand All @@ -38,6 +43,13 @@ const MODAL_INSET_MIN_WIDTH = 768;
const MODAL_INSET_MIN_HEIGHT = 600;
const EDGE_THRESHOLD = 5;

/**
* CSS values for `--width` / `--height` that are treated as fullscreen
* (modal touches the corresponding screen edges). Empty string means the
* property was not overridden. See `hasCustomModalDimensions()`.
*/
const FULLSCREEN_SIZE_VALUES = new Set(['', '100%', '100vw', '100vh', '100dvw', '100dvh', '100svw', '100svh']);

/**
* Cache for resolved root safe-area-top value, invalidated once per frame.
*/
Expand Down Expand Up @@ -92,6 +104,23 @@ export const getRootSafeAreaTop = (): number => {
return value;
};

/**
* True when the modal host declares BOTH a non-fullscreen `--width` AND a
* non-fullscreen `--height` (i.e. a centered-dialog-like modal that doesn't
* touch any screen edge).
*
* The conservative "both axes" check avoids mis-zeroing safe-area for
* partial-custom modals where the modal still touches top/bottom edges
* (e.g. only `--width` overridden). Partial cases fall through to the
* existing position-based post-animation correction.
*/
export const hasCustomModalDimensions = (hostEl: HTMLElement): boolean => {
const styles = getComputedStyle(hostEl);
const width = styles.getPropertyValue('--width').trim();
const height = styles.getPropertyValue('--height').trim();
return !FULLSCREEN_SIZE_VALUES.has(width) && !FULLSCREEN_SIZE_VALUES.has(height);
};

/**
* Returns the initial safe-area configuration based on modal type.
* This is called before animation starts and uses configuration-based prediction.
Expand Down Expand Up @@ -129,8 +158,11 @@ export const getInitialSafeAreaConfig = (context: ModalSafeAreaContext): SafeAre

// On viewports that meet the centered dialog media query breakpoints,
// regular modals render as centered dialogs (not fullscreen), so they
// don't touch any screen edges and don't need safe-area insets.
if (isCenteredDialogViewport()) {
// don't touch any screen edges and don't need safe-area insets. Also
// applies to phone viewports when the modal declares custom --width and
// --height; these don't touch screen edges either, so the initial
// prediction must be zero to avoid a post-animation correction flash.
if (isCenteredDialogViewport() || context.hasCustomDimensions) {
return {
top: '0px',
bottom: '0px',
Expand Down
122 changes: 110 additions & 12 deletions core/src/components/modal/test/safe-area/modal.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import { configs, test, Viewports } from '@utils/test/playwright';
* The test page (index.html) sets these root safe-area values.
* Keep in sync with the :root block in test/safe-area/index.html.
*/
const TEST_SAFE_AREA_TOP = '47px';
const TEST_SAFE_AREA_TOP = 47;
const TEST_SAFE_AREA_BOTTOM = 34;
/** Default value of --ion-padding (16px), applied via the .ion-padding class on ion-content in the test modal. */
const TEST_ION_PADDING = 16;

configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config }) => {
test.describe(title('modal: safe-area handling'), () => {
test.beforeEach(async ({ page }) => {
Expand Down Expand Up @@ -100,10 +104,12 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config
expect(safeAreaBottom).toBe('inherit');
});

test('fullscreen modal without footer should have wrapper padding-bottom', async ({ page }, testInfo) => {
test('fullscreen modal without footer should set safe-area scroll padding on ion-content', async ({
page,
}, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/30900',
description: 'https://github.com/ionic-team/ionic-framework/issues/31015',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the issue changed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old version of this test asserted on wrapperEl padding-bottom, which was the behavior #30949 added when fixing #30900. This PR moves that compensation into ion-content's scroll padding to fix #31015 (the regression #30949 caused), so the assertions now verify the #31015 fix rather than the original #30900 work. I don't think we can have two annotations at once for the same issue? Or can we? Not sure. We don't typically do that, so I just replaced this one.

});

const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');
Expand All @@ -113,20 +119,83 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config

const modal = page.locator('ion-modal');

// When no footer is present, the wrapper should have reduced height
// and padding-bottom to prevent content from overlapping the system
// navigation bar, without changing box-sizing (which would break
// custom modals with --border-width).
// The wrapper should NOT have reduced height or padding-bottom.
// Safe-area compensation is handled by ion-content's scroll padding.
const wrapper = modal.locator('.modal-wrapper');
const paddingBottom = await wrapper.evaluate((el: HTMLElement) => {
const wrapperPaddingBottom = await wrapper.evaluate((el: HTMLElement) => {
return el.style.getPropertyValue('padding-bottom');
});
const height = await wrapper.evaluate((el: HTMLElement) => {
const wrapperHeight = await wrapper.evaluate((el: HTMLElement) => {
return el.style.getPropertyValue('height');
});

expect(paddingBottom).toBe('var(--ion-safe-area-bottom, 0px)');
expect(height).toBe('calc(var(--height) - var(--ion-safe-area-bottom, 0px))');
expect(wrapperPaddingBottom).toBe('');
expect(wrapperHeight).toBe('');

// ion-content should have --ion-content-safe-area-padding-bottom set so its
// .inner-scroll element includes safe-area in its bottom padding.
const content = modal.locator('ion-content');
const safeAreaPadding = await content.evaluate((el: HTMLElement) => {
return el.style.getPropertyValue('--ion-content-safe-area-padding-bottom');
});
expect(safeAreaPadding).toBe('var(--ion-safe-area-bottom, 0px)');
});

test('fullscreen modal with ion-content and no footer should not reduce wrapper content area', async ({
page,
}, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/31015',
});

const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');

await page.click('#fullscreen-modal-no-footer');
await ionModalDidPresent.next();

const modal = page.locator('ion-modal');
const wrapper = modal.locator('.modal-wrapper');

// The wrapper's content area should equal the full viewport height.
// Safe-area compensation is handled by ion-content's scroll padding,
// not by reducing the wrapper. This prevents the visible white gap
// reported in #31015.
const { contentHeight, paddingBottom } = await wrapper.evaluate((el: HTMLElement) => {
const computed = getComputedStyle(el);
return {
contentHeight: parseFloat(computed.height),
paddingBottom: parseFloat(computed.paddingBottom),
};
});
const viewportHeight = await page.evaluate(() => window.innerHeight);

expect(paddingBottom).toBeCloseTo(0, 0);
expect(contentHeight).toBeCloseTo(viewportHeight, 0);
});

test('fullscreen modal ion-content scroll padding should include safe-area-bottom', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/31015',
});

const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent');

await page.click('#fullscreen-modal-no-footer');
await ionModalDidPresent.next();

const modal = page.locator('ion-modal');
const content = modal.locator('ion-content');

// The .inner-scroll element inside ion-content's shadow DOM should
// have padding-bottom that includes the safe-area-bottom value.
const innerScroll = content.locator('.inner-scroll');
const scrollPaddingBottom = await innerScroll.evaluate((el: Element) => {
return parseFloat(getComputedStyle(el).paddingBottom);
});

expect(scrollPaddingBottom).toBe(TEST_ION_PADDING + TEST_SAFE_AREA_BOTTOM);
});

test('sheet modal at breakpoint 1 should keep top safe-area zeroed', async ({ page }, testInfo) => {
Expand Down Expand Up @@ -185,7 +254,7 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config
const offsetTop = await modal.evaluate((el: HTMLIonModalElement) => {
return el.style.getPropertyValue('--ion-modal-offset-top');
});
expect(offsetTop).toBe(TEST_SAFE_AREA_TOP);
expect(offsetTop).toBe(`${TEST_SAFE_AREA_TOP}px`);
});

test('fullscreen modal safe-area should update on resize from phone to tablet', async ({ page }, testInfo) => {
Expand Down Expand Up @@ -251,6 +320,35 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config
expect(safeAreaBottom).toBe('0px');
});

test('centered dialog with custom dimensions on phone should zero safe-area from initial prediction', async ({
page,
}, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/31015',
});

// Stay on phone viewport. This is the path where the centered-dialog
// media query does NOT match but the modal still doesn't touch screen
// edges because cssClass sets --width/--height. Without the initial
// prediction catching this, safe-area flashes inherited values and
// then snaps to 0px after animation.
const ionModalWillPresent = await page.spyOnEvent('ionModalWillPresent');
await page.click('#centered-dialog');
await ionModalWillPresent.next();

// Read inline style IMMEDIATELY after will-present fires, before the
// animation finishes. This captures the initial prediction value.
const modal = page.locator('ion-modal');
const initial = await modal.evaluate((el: HTMLIonModalElement) => ({
top: el.style.getPropertyValue('--ion-safe-area-top'),
bottom: el.style.getPropertyValue('--ion-safe-area-bottom'),
}));

expect(initial.top).toBe('0px');
expect(initial.bottom).toBe('0px');
});

test('safe-area overrides should be cleared on dismiss', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
Expand Down
Loading