Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions core/scripts/testing/styles.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

96 changes: 95 additions & 1 deletion core/src/components/content/content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ export class Content implements ComponentInterface {
private scrollEl?: HTMLElement;
private backgroundContentEl?: HTMLElement;
private isMainContent = true;
private sizeToContent = false;
private resizeTimeout: ReturnType<typeof setTimeout> | null = null;
private fullscreenResizeObserver?: ResizeObserver;
private sizeToContentObserver?: MutationObserver;
private inheritedAttributes: Attributes = {};

private tabsElement: HTMLElement | null = null;
Expand Down Expand Up @@ -190,6 +192,7 @@ export class Content implements ComponentInterface {

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

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.

Suggested change
this.setupSizeToContentObserver();
this.setupSizeToContentObserver();
this.updateSizeToContent();

This re-arms the observer but nothing recomputes the class, and the only thing that ever writes sizeToContent is the render expression. Stencil doesn't re-render on reconnect, so the old value just sits there.

A modal at the default height is 800px. Detach the content, set --height: fit-content, put it back, and you get 44px with the content at 0, which is the bug this PR is fixing. Any unrelated style write on the modal afterwards snaps it to 244px, so the observer's fine, it's just that nothing evaluates on reconnect. The other direction sticks content-sizing on and leaves a full-height modal with contain: none.

Anything that unmounts and remounts the modal body across a height change hits it, so *ngIf, v-if and friends. That'd cover it, though making sizeToContent a @State would be sturdier.

}

componentDidLoad() {
Expand Down Expand Up @@ -222,6 +225,7 @@ export class Content implements ComponentInterface {
}

this.destroyFullscreenResizeObserver();
this.destroySizeToContentObserver();
}

/**
Expand Down Expand Up @@ -258,6 +262,51 @@ export class Content implements ComponentInterface {
this.fullscreenResizeObserver.observe(this.el);
}

/**
* A modal's `--height` can be changed at runtime with no event to react
* to, either by setting the property directly or by toggling a class that
* changes which rule wins. Both of those mutate an attribute on the modal,
* so watch for that and re-evaluate. Viewport driven changes are already
* covered by the `resize` listener.
*/
private setupSizeToContentObserver() {
Comment thread
thetaPC marked this conversation as resolved.
if (!Build.isBrowser || typeof MutationObserver === 'undefined') {
return;
}

if (this.sizeToContentObserver !== undefined) {
return;
}

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

this.sizeToContentObserver = new MutationObserver(() => this.updateSizeToContent());
this.sizeToContentObserver.observe(modal, { attributes: true, attributeFilter: ['style', 'class'] });

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.

The JSDoc says this covers toggling a class that changes which rule wins, but that only holds when the class is on the modal. Two shapes that both collapse to a 44px header-only modal while getComputedStyle on the modal happily reports --height: fit-content:

  • body.compact ion-modal { --height: fit-content; }, class on body
  • ion-modal { --height: var(--dlg-h); } with --dlg-h toggled on :root, same shape as ion-palette-dark

I don't think any observer on the modal can catch those, since neither one mutates an attribute on it. Would it be easier for ion-modal to push the signal down? It's already re-reading --height on present, breakpoint and resize.

}

private destroySizeToContentObserver() {
if (this.sizeToContentObserver !== undefined) {
this.sizeToContentObserver.disconnect();
this.sizeToContentObserver = undefined;
}
}

/**
* Re-renders when the overlay is no longer sized the way the last render
* assumed. Read in a `readTask` because resolving the custom property forces
* a style recalculation.
*/
private updateSizeToContent() {
readTask(() => {
if (this.shouldSizeToContent() !== this.sizeToContent) {
forceUpdate(this);
}
});
}

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

/**
* Whether to size the component to its content height.
*
* 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 shouldSizeToContent() {
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').trim();

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.

Suggested change
const height = getComputedStyle(modal).getPropertyValue('--height').trim();
const height = getComputedStyle(modal).getPropertyValue('--height').trim().toLowerCase();

Height keywords are case-insensitive, so --height: AUTO is valid CSS but doesn't match here. You get no content-sizing and a 44px header-only modal.


/**
* Compared as a suffix so a value carrying a vendor prefix is still
* recognized, such as `-webkit-fit-content` or the `-moz-fit-content`
* that Firefox needed before 94.
*/
return CONTENT_SIZED_HEIGHTS.some((value) => height.endsWith(value));
}

private resize() {
/**
* Only force update if the component is rendered in a browser context.
Expand All @@ -320,6 +401,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 Down Expand Up @@ -538,7 +626,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 = this.shouldSizeToContent()),

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.

Assigning to this.sizeToContent inside the class object is the only one of its kind in core/src, and anyone tidying it into a plain call later would kill the runtime reactivity without a test to catch them.

The bigger one is getComputedStyle running from render, which Stencil does in the write task, so every ion-content render forces a style recalc. The comment on updateSizeToContent() argues against exactly that and uses a readTask.

Could sizeToContent be a @State computed in componentWillLoad plus the observer and resize paths? That'd get the read out of the write phase and fix the reconnect staleness too.

overscroll: forceOverscroll,
[`content-${rtl}`]: true,
})}
Expand Down Expand Up @@ -579,6 +667,12 @@ export class Content implements ComponentInterface {
}
}

/**
* `ion-modal` `--height` values that size the modal to its contents, leaving
* children an indefinite height to resolve against.
*/
const CONTENT_SIZED_HEIGHTS = ['auto', 'fit-content', 'min-content', 'max-content'];

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.

The hasCustomModalDimensions helper in safe-area-utils already reads --height off the modal the same way, with its own FULLSCREEN_SIZE_VALUES. Two keyword sets in two components that have to stay in sync, and the --max-height change makes them disagree in at least one case.


const getParentElement = (el: any) => {
if (el.parentElement) {
// normal element with a parent element
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.

--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
Loading
Loading