Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
4e8ecc2
fix(modal): prevent ion-content collapsing at content-based heights
brandyscarney Aug 28, 2026
e63841c
test(modal): add content-height preview test
brandyscarney Aug 28, 2026
21e6c23
test(modal): reorder modals, don't use ionic buttons
brandyscarney Aug 31, 2026
51e8923
fix(content): watch for changes to --height and respect prefixed values
brandyscarney Aug 31, 2026
54b1596
test(modal): add e2e tests for various --height values
brandyscarney Aug 31, 2026
d5b4e75
style: comment the test config
brandyscarney Sep 2, 2026
7298645
style: update comment to drop TODO
brandyscarney Sep 2, 2026
6738339
style: update comment to specify the limitation on --height
brandyscarney Sep 2, 2026
dff4f83
fix(content): account for height changes when unmounted
brandyscarney Sep 8, 2026
e6da097
test(modal): move styling for red button
brandyscarney Sep 8, 2026
1205cea
fix(content): make --height comparison case-insensitive
brandyscarney Sep 8, 2026
56dc09f
refactor(content): move sizeToContent to State
brandyscarney Sep 9, 2026
710ca95
test(modal): remove unused wrappers
brandyscarney Sep 9, 2026
cf43abf
style: remove trailing whitespace
brandyscarney Sep 9, 2026
8873e48
fix(content): scope the content-sizing styles to modal
brandyscarney Sep 9, 2026
7277c15
test(modal): turn off animations for all modals
brandyscarney Sep 9, 2026
839354f
test(modal): simplify sampling in the nav overlap test
brandyscarney Sep 9, 2026
8bfd7d3
test(modal): cover both modes and add screenshots
brandyscarney Sep 9, 2026
8a9a5e8
refactor(modal): move fullscreen and content logic to overlays
brandyscarney Sep 10, 2026
3d450d2
fix(modal): apply content-sizing when an ancestor updates --height
brandyscarney Sep 10, 2026
b9ed932
test(modal): clean up the reusable modal functions
brandyscarney Sep 10, 2026
d56d71c
fix(modal): account for different content-sized modals and safe area
brandyscarney Sep 11, 2026
af585a6
test(modal): improve height expectations
brandyscarney Sep 11, 2026
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.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ export namespace Components {
*/
"getScrollElement": () => Promise<HTMLElement>;
/**
* Recalculate content dimensions. Called by overlays (e.g., popover) when sibling elements like headers or footers have finished rendering and their heights are available, ensuring accurate offset-top calculations.
* Recalculates the content dimensions and whether it should size itself to its content. Called by overlays when something they own changes, such as a header finishing its render or `--height` being updated.
*/
"recalculateDimensions": () => Promise<void>;
/**
Expand Down
65 changes: 61 additions & 4 deletions core/src/components/content/content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import {
Listen,
Method,
Prop,
State,
Watch,
forceUpdate,
h,
readTask,
} from '@stencil/core';
import { componentOnReady, hasLazyBuild, inheritAriaAttributes } from '@utils/helpers';
import type { Attributes } from '@utils/helpers';
import { getOverlaySizeType } from '@utils/overlays';
import { isPlatform } from '@utils/platform';
import { isRTL } from '@utils/rtl';
import { createColorClasses, hostContext } from '@utils/theme';
Expand Down Expand Up @@ -77,6 +79,11 @@ export class Content implements ComponentInterface {

@Element() el!: HTMLIonContentElement;

/**
* Whether the host is sized to its content.
*/
@State() sizeToContent = false;

/**
* The color to use from your application's color palette.
* Default options are: `"primary"`, `"secondary"`, `"tertiary"`, `"success"`, `"warning"`, `"danger"`, `"light"`, `"medium"`, and `"dark"`.
Expand Down Expand Up @@ -148,6 +155,7 @@ export class Content implements ComponentInterface {

componentWillLoad() {
this.inheritedAttributes = inheritAriaAttributes(this.el);
this.sizeToContent = this.readSizeToContent();
}

connectedCallback() {
Expand Down Expand Up @@ -190,6 +198,7 @@ export class Content implements ComponentInterface {

// Re-observe on reattach, since componentDidLoad only fires once.
this.setupFullscreenResizeObserver();
this.updateSizeToContent();
}

componentDidLoad() {
Expand Down Expand Up @@ -258,6 +267,17 @@ export class Content implements ComponentInterface {
this.fullscreenResizeObserver.observe(this.el);
}

/**
* Picks up an overlay that is no longer sized the way the last render
* assumed, re-rendering only when the answer changes. Read in a `readTask`
* because resolving the custom property forces a style recalculation.
*/
private updateSizeToContent() {
readTask(() => {
this.sizeToContent = this.readSizeToContent();
});
}

private destroyFullscreenResizeObserver() {
if (this.fullscreenResizeObserver !== undefined) {
this.fullscreenResizeObserver.disconnect();
Expand Down Expand Up @@ -310,6 +330,34 @@ export class Content implements ComponentInterface {
return forceOverscroll === undefined ? mode === 'ios' && isPlatform('ios') : forceOverscroll;
}

/**
* Reads whether to size the component to its content height. Forces a style
* recalculation, so it belongs in a read task or before the first render.
*
* This applies inside popovers and modals with a content-based `--height`,
* where the overlay does not provide the content with a definite height
* to fill.
*
* Only `--height` is consulted. Styling the wrapper directly, such as
* `ion-modal::part(content) { height: fit-content; }`, does not change
* `--height` and therefore cannot be observed. `--height` is the only
* supported way to opt into content-based sizing.
*/
private readSizeToContent() {
if (hostContext('ion-popover', this.el)) {
return true;
}

const modal = this.el.closest('ion-modal');
if (modal === null) {
return false;
}

const height = getComputedStyle(modal).getPropertyValue('--height');

return getOverlaySizeType(height) === 'content';
}

private resize() {
/**
* Only force update if the component is rendered in a browser context.
Expand All @@ -320,6 +368,13 @@ export class Content implements ComponentInterface {
* TODO: Remove if STENCIL-834 determines Stencil will account for this.
*/
if (Build.isBrowser) {
/**
* A window resize can cross a media query that changes the modal's
* `--height`. The content's own offsets are unchanged, so neither branch
* below re-renders and the class from the last render would go stale.
*/
this.updateSizeToContent();

if (this.fullscreen) {
readTask(() => this.readDimensions());
} else if (this.cTop !== 0 || this.cBottom !== 0) {
Expand All @@ -330,14 +385,16 @@ export class Content implements ComponentInterface {
}

/**
* Recalculate content dimensions. Called by overlays (e.g., popover) when
* sibling elements like headers or footers have finished rendering and their
* heights are available, ensuring accurate offset-top calculations.
* Recalculates the content dimensions and whether it should size itself to
* its content. Called by overlays when something they own changes, such as
* a header finishing its render or `--height` being updated.
*
* @internal
*/
@Method()
async recalculateDimensions(): Promise<void> {
readTask(() => this.readDimensions());
this.updateSizeToContent();
}

private readDimensions() {
Expand Down Expand Up @@ -538,7 +595,7 @@ export class Content implements ComponentInterface {
class={createColorClasses(this.color, {
[mode]: true,
'content-fullscreen': this.fullscreen,
'content-sizing': hostContext('ion-popover', this.el),
'content-sizing': this.sizeToContent,
overscroll: forceOverscroll,
[`content-${rtl}`]: true,
})}
Expand Down
15 changes: 14 additions & 1 deletion core/src/components/modal/modal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@
--max-width: auto;
--height: 100%;
--min-height: auto;
--max-height: auto;
/**
* Clamps a content-sized `--height` (auto, fit-content, ...) to the
* overlay, giving the wrapper's flex children something to shrink
* toward so `ion-content` scrolls instead of overflowing.
*/
--max-height: 100%;

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.

Is this considered a breaking change since consumers are used to having it as auto?

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.

No, I wouldn't consider this a breaking change because auto was never a valid value for max-height in the first place.

auto isn't listed as a valid value in the docs for max-height. As a result, max-height: auto is invalid and the property fell back to its initial value, none. If you inspect any .modal-wrapper prior to this change you will see the max-height is computed as none:

CleanShot 2026-09-02 at 16 57 53

That means the actual change is none100%.

From there, the cases where the computed value actually changes are all cases that were already broken:

  • --height: 100% (the default) and every built-in variant (calc(100% - 40px), sheet, card, inset heights) are all ≤ 100%, so the clamp has no effect and rendering remains identical.
  • The iOS card modal sets --max-height: 1000px explicitly, so it's unaffected.
  • A --height taller than the overlay (e.g. 800px in a 600px viewport, or a content-based height with tall content) previously overflowed the host. Since :host has contain: strict, that overflow was clipped at both the top and bottom, leaving some of the content unreachable. Clamping the height so ion-content scrolls instead is a fix.

Anyone who explicitly sets --max-height: auto still ends up with none, since their override is just as invalid as the old default was. And setting --max-height to anything else will still take precedence.

Additionally, CSS variable defaults are not tracked in the public API. api.txt records CSS custom property names only, so there are no generated docs or API diff changes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A content-sized modal with overflowing content now fills the screen, but hasCustomModalDimensions still calls it a centered dialog and zeroes the safe-area, since neither --width nor --height is fullscreen. With a 47px top inset the header stays 44px for the whole enter animation then jumps to 91px. That's the flash hasCustomModalDimensions is there to prevent, on a config that couldn't reach it before because the modal used to collapse.

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.

Good catch. I updated hasCustomModalDimensions to measure the modal in the content-sized case to determine whether it spans the viewport and needs the inherited inset. Short dialogs are still treated as custom-sized and get the inset zeroed as before. There are now tests covering both cases: 8a9a5e8

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding this! I think the new branch only runs when --width is narrowed too. The host rule sets --width: 100%, so ion-modal { --height: fit-content } hits the width === 'fullscreen' early return and never measures.

On a 390x760 viewport a short full-width content-sized modal predicts inherit, and then the post-animation pass writes 0px, so the flash is still there on the shape from the issue. Both new tests set --width: 300px, which is why they're green.

Would a per-axis verdict work? A full-width one still needs the left and right insets, so I don't think it's just a reordering.

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.

Always an edge case. 😅 I think I fixed them all: d56d71c

Preview here: Modal: Safe Area

--overflow: hidden;
--border-radius: 0;
--border-width: 0;
Expand Down Expand Up @@ -87,8 +92,16 @@ ion-backdrop {
/**
* The wrapper receives programmatic focus for screen readers but should not
* show a visible focus ring, which is meant only for keyboard navigation.
*
* A flex layout is required for the wrapper to size itself to its content
* when the modal is content-sized (`--height` is auto, fit-content, ...).
* This makes it so that the content can scroll when it overflows the wrapper.
*/
.modal-wrapper {
display: flex;

flex-direction: column;

outline: none;
}

Expand Down
36 changes: 32 additions & 4 deletions core/src/components/modal/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ import {
clearSafeAreaOverrides,
getRootSafeAreaTop,
onRootSafeAreaTopChange,
hasCustomModalDimensions,
getModalCoveredAxes,
type ModalSafeAreaContext,
} from './safe-area-utils';
import { setCardStatusBarDark, setCardStatusBarDefault } from './utils';
import { onModalHeightChange, setCardStatusBarDark, setCardStatusBarDefault } from './utils';

// TODO(FW-2832): types

Expand Down Expand Up @@ -114,6 +114,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
private viewTransitionAnimation?: Animation;
private resizeTimeout?: any;
private unsubscribeRootSafeAreaTop?: () => void;
private unsubscribeHeightChange?: () => void;
// True from the first safe-area write in `present()` until the enter
// animation settles. A position-based read in that window is not the rest position.
private isPresenting = false;
Expand Down Expand Up @@ -1487,7 +1488,7 @@ export class Modal implements ComponentInterface, OverlayInterface {
/**
* Creates the context object for safe-area utilities.
*
* `hasCustomDimensions` is only set by `setInitialSafeAreaOverrides()`
* `coveredAxes` 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.
Expand All @@ -1502,6 +1503,28 @@ export class Modal implements ComponentInterface, OverlayInterface {
};
}

/**
* Keeps the content's sizing in sync with `--height`. The content reads the
* property to determine whether it should size itself to its content, and
* changes to `--height` on an ancestor or the root can change that behavior
* without changing the modal itself.
*/
private watchHeightForContent(): void {
/**
* A sheet's height comes from its breakpoints, so its content never sizes
* itself to `--height`. Watching it would cause the drag to recalculate on
* every frame.
*/
if (this.isSheetModal) {
return;
}

this.unsubscribeHeightChange?.();
this.unsubscribeHeightChange = onModalHeightChange(this.el, () => {
this.el.querySelectorAll('ion-content').forEach((contentEl) => contentEl.recalculateDimensions());
});
}

/**
* Sets initial safe-area overrides before modal animation.
* Called in present() before animation starts.
Expand All @@ -1515,11 +1538,13 @@ export class Modal implements ComponentInterface, OverlayInterface {
private setInitialSafeAreaOverrides(): void {
const context: ModalSafeAreaContext = {
...this.getSafeAreaContext(),
hasCustomDimensions: hasCustomModalDimensions(this.el),
coveredAxes: getModalCoveredAxes(this.el),
};
const safeAreaConfig = getInitialSafeAreaConfig(context);
applySafeAreaOverrides(this.el, safeAreaConfig);

this.watchHeightForContent();

// Set the internal offset property with the resolved root safe-area-top value
if (context.isSheetModal) {
this.updateSheetOffsetTop();
Expand Down Expand Up @@ -1646,6 +1671,9 @@ export class Modal implements ComponentInterface, OverlayInterface {
this.unsubscribeRootSafeAreaTop?.();
this.unsubscribeRootSafeAreaTop = undefined;

this.unsubscribeHeightChange?.();
this.unsubscribeHeightChange = undefined;

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

Expand Down
130 changes: 130 additions & 0 deletions core/src/components/modal/safe-area-utils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import { getModalCoveredAxes } from './safe-area-utils';

/**
* Tests `getModalCoveredAxes()` across fullscreen, fixed-size, and
* content-sized modals. The helper uses computed CSS sizes when both
* dimensions are fullscreen and measures the rendered wrapper otherwise,
* so the tests mock both sources of size information as needed.
*/
describe('modal: getModalCoveredAxes', () => {
const VIEWPORT_WIDTH = window.innerWidth;
const VIEWPORT_HEIGHT = window.innerHeight;

let host: HTMLElement;
let wrapper: HTMLElement;
let hiddenDuringMeasurement: boolean;
let originalGetComputedStyle: PropertyDescriptor | undefined;
let sizes: Record<string, string>;

const setSize = (width: string, height: string) => {
sizes = { '--width': width, '--height': height };
};

const setWrapperBox = (width: number, height: number) => {
wrapper.getBoundingClientRect = () => {
hiddenDuringMeasurement = host.classList.contains('overlay-hidden');
return { width, height } as DOMRect;
};
};

beforeEach(() => {
host = document.createElement('ion-modal');
host.classList.add('overlay-hidden');
document.body.appendChild(host);

wrapper = document.createElement('div');
wrapper.classList.add('modal-wrapper');
host.attachShadow({ mode: 'open' }).appendChild(wrapper);

hiddenDuringMeasurement = true;
setWrapperBox(0, 0);

/**
* Replace the mocked `getComputedStyle` getter so tests can control
* the modal's `--width` and `--height` values.
*/
sizes = {};
originalGetComputedStyle = Object.getOwnPropertyDescriptor(globalThis, 'getComputedStyle');
Object.defineProperty(globalThis, 'getComputedStyle', {
value: () => ({ getPropertyValue: (property: string) => sizes[property] ?? '' }),
configurable: true,
writable: true,
});
});

afterEach(() => {
if (originalGetComputedStyle) {
Object.defineProperty(globalThis, 'getComputedStyle', originalGetComputedStyle);
}
host.remove();
});

it('should cover both axes when the sizes span the viewport', () => {
setSize('100%', '100%');

expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: true });
});

it('should cover neither axis for a dialog that stays clear of the viewport edges', () => {
setSize('300px', '200px');
setWrapperBox(300, 200);

expect(getModalCoveredAxes(host)).toEqual({ vertical: false, horizontal: false });
});

it('should cover only the horizontal axis for a full-width dialog that fits its content', () => {
setSize('100%', 'fit-content');
setWrapperBox(VIEWPORT_WIDTH, 244);

expect(getModalCoveredAxes(host)).toEqual({ vertical: false, horizontal: true });
});

it('should cover only the vertical axis for a narrow modal that fills the viewport', () => {
setSize('300px', 'fit-content');
setWrapperBox(300, VIEWPORT_HEIGHT);

expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: false });
});

it('should cover an axis whose definite size reaches the viewport', () => {
setSize('300px', `${VIEWPORT_HEIGHT}px`);
setWrapperBox(300, VIEWPORT_HEIGHT);

expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: false });
});

it('should allow a few pixels of tolerance when comparing to the viewport', () => {
setSize('300px', 'fit-content');
setWrapperBox(300, VIEWPORT_HEIGHT - 4);

expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: false });
});

it('should temporarily show a hidden modal while measuring and hide it again', () => {
setSize('300px', 'fit-content');
setWrapperBox(300, VIEWPORT_HEIGHT);

getModalCoveredAxes(host);

expect(hiddenDuringMeasurement).toBe(false);
expect(host.classList.contains('overlay-hidden')).toBe(true);
});

it('should not change visibility when measuring an already visible modal', () => {
host.classList.remove('overlay-hidden');
setSize('300px', 'fit-content');
setWrapperBox(300, 244);

getModalCoveredAxes(host);

expect(host.classList.contains('overlay-hidden')).toBe(false);
});

it('should not measure when both sizes span the viewport', () => {
setSize('100%', '100vh');
setWrapperBox(0, 0);

expect(getModalCoveredAxes(host)).toEqual({ vertical: true, horizontal: true });
expect(hiddenDuringMeasurement).toBe(true);
});
});
Loading
Loading