fix(modal): prevent ion-content collapsing at content-based heights by brandyscarney · Pull Request #31413 · ionic-team/ionic-framework

brandyscarney · 2026-08-28T20:00:55Z

Issue number: resolves #31149

What is the current behavior?

Setting --height to any of the following on an ion-modal containing an ion-content results in the ion-content collapsing to 0 height: auto, fit-content, min-content, or max-content.

What is the new behavior?

Checks if modal --height is set to auto, fit-content, min-content, or max-content and styles the ion-content, ion-nav and .ion-page appropriately if so
Watches for changes to --height on ion-modal to dynamically add and remove the class that sizes the ion-content to its content
Updates modal --max-height to 100% which allows the content to scroll instead of overflowing and being clipped
Adds a preview test for different scenarios where ion-modal has --height set
Adds e2e tests for the various --height scenarios

Does this introduce a breaking change?

Yes
No

Other information

Dev build: 9.0.1-dev.11788212611.154b1596

Preview: Modal: Content Height

Docs PR: ionic-team/ionic-docs#4689

resolves #31149

vercel · 2026-08-28T20:01:01Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
ionic-framework	Ready	Preview	Sep 10, 2026 10:37pm UTC

brandyscarney · 2026-08-31T21:42:42Z

+    }
+
+    const height = getComputedStyle(modal).getPropertyValue('--height').trim();
+    return CONTENT_SIZED_HEIGHTS.includes(height);


Yes this PR is still a work in progress! 🙂

thetaPC

Mainly waiting on the breaking change question to be resolved.

thetaPC · 2026-09-02T17:37:17Z

+   * overlay, giving the wrapper's flex children something to shrink 
+   * toward so `ion-content` scrolls instead of overflowing.
+   */
+  --max-height: 100%;


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

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:

That means the actual change is none → 100%.

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.

thetaPC

LGTM

ShaneK

Awesome work! Looks really good overall, and the test page covers a lot of ground. I did find a few cases where the content still collapses though. Setting --height from an ancestor rule, using --height: AUTO, and reconnecting across a height change all miss the class and leave a header-only modal, so the original bug is still reachable. Details in the comments, and let me know if I'm missing something!

ShaneK · 2026-09-04T20:24:16Z

+   * Override the minimum height a flex item gets, which defaults to 
+   * use the height of its own content. Without this, a modal sized 


Suggested change

* Override the minimum height a flex item gets, which defaults to

* use the height of its own content. Without this, a modal sized

* Override the minimum height a flex item gets, which defaults to

* the height of its own content. Without this, a modal sized

That fixes these two lines and the dropped word. There are 19 in total though, all in the new comment blocks, spread across this file, the new test page, modal.scss and the testing stylesheet. The root .editorconfig sets trim_trailing_whitespace, and nothing catches it since prettier's globs skip scss and css.

cf43abf

Just FYI I found several other files with trailing whitespace by running npx editorconfig-checker -disable-indentation -disable-indent-size -disable-insert-final-newline -disable-end-of-line -disable-max-line-length -exclude components.d.ts src scripts. If this is something we want to enforce against we should have it covered by the linter.

Makes sense to cover it by a linter if you want since the .editorconfig declares it. Ideally our linters would be aligned

ShaneK

Thanks for working through all of those! Just a few things left (one of them is a reply in a thread, that's the blocker)

ShaneK · 2026-09-11T16:47:26Z

+
+        // The default --max-height keeps a content-sized modal inside the
+        // overlay. Rounded up by one, since the clamp lands on a sub-pixel.
+        expect(await getWrapperHeight(page)).toBeLessThanOrEqual(viewport.height + 1);


This one and the --max-height: 50% test below it only cap the height, they never floor it. A modal that collapsed back to a 44px header satisfies toBeLessThanOrEqual(viewport + 1), and scrollHeight > clientHeight is true at clientHeight: 0 as well, so the original bug passes both. Could they get a lower bound?

ShaneK

Oops, meant to send the last one as request changes. Really it's just that one blocking change and the others are 🤷‍♂️

fix(modal): prevent ion-content collapsing at content-based heights

4e8ecc2

resolves #31149

github-actions Bot added the package: core @ionic/core package label Aug 28, 2026

vercel Bot deployed to Preview August 28, 2026 20:05 View deployment

test(modal): add content-height preview test

e63841c

vercel Bot deployed to Preview August 28, 2026 20:49 View deployment

brandyscarney added 3 commits August 31, 2026 15:37

test(modal): reorder modals, don't use ionic buttons

21e6c23

fix(content): watch for changes to --height and respect prefixed values

51e8923

test(modal): add e2e tests for various --height values

54b1596

vercel Bot deployed to Preview August 31, 2026 21:37 View deployment

brandyscarney commented Aug 31, 2026

View reviewed changes

brandyscarney mentioned this pull request Sep 1, 2026

docs(modal): reorganize Styling and add Sizing to Content section ionic-team/ionic-docs#4689

Open

brandyscarney marked this pull request as ready for review September 2, 2026 17:26

brandyscarney requested a review from a team as a code owner September 2, 2026 17:26

brandyscarney requested a review from ShaneK September 2, 2026 17:26

thetaPC reviewed Sep 2, 2026

View reviewed changes

brandyscarney added 2 commits September 2, 2026 16:47

style: comment the test config

d5b4e75

style: update comment to drop TODO

7298645

vercel Bot deployed to Preview September 2, 2026 20:55 View deployment

style: update comment to specify the limitation on --height

6738339

brandyscarney requested a review from thetaPC September 2, 2026 21:16

vercel Bot deployed to Preview September 2, 2026 21:21 View deployment

thetaPC approved these changes Sep 2, 2026

View reviewed changes

ShaneK requested changes Sep 4, 2026

View reviewed changes

brandyscarney added 2 commits September 8, 2026 16:10

fix(content): account for height changes when unmounted

dff4f83

test(modal): move styling for red button

e6da097

vercel Bot deployed to Preview September 8, 2026 20:24 View deployment

fix(content): make --height comparison case-insensitive

1205cea

vercel Bot deployed to Preview September 8, 2026 20:48 View deployment

refactor(content): move sizeToContent to State

56dc09f

test(modal): remove unused wrappers

710ca95

vercel Bot deployed to Preview September 9, 2026 15:12 View deployment

vercel Bot deployed to Preview September 9, 2026 15:21 View deployment

brandyscarney force-pushed the FW-7731 branch from 4da3287 to 710ca95 Compare September 9, 2026 15:28

style: remove trailing whitespace

cf43abf

vercel Bot deployed to Preview September 9, 2026 15:45 View deployment

fix(content): scope the content-sizing styles to modal

8873e48

vercel Bot deployed to Preview September 9, 2026 17:34 View deployment

test(modal): turn off animations for all modals

7277c15

vercel Bot deployed to Preview September 9, 2026 19:11 View deployment

test(modal): simplify sampling in the nav overlap test

839354f

vercel Bot deployed to Preview September 9, 2026 19:51 View deployment

test(modal): cover both modes and add screenshots

8bfd7d3

vercel Bot deployed to Preview September 9, 2026 21:27 View deployment

refactor(modal): move fullscreen and content logic to overlays

8a9a5e8

vercel Bot deployed to Preview September 10, 2026 20:11 View deployment

fix(modal): apply content-sizing when an ancestor updates --height

3d450d2

vercel Bot deployed to Preview September 10, 2026 22:24 View deployment

test(modal): clean up the reusable modal functions

b9ed932

vercel Bot deployed to Preview September 10, 2026 22:37 View deployment

brandyscarney requested a review from ShaneK September 10, 2026 22:50

ShaneK reviewed Sep 11, 2026

View reviewed changes

ShaneK requested changes Sep 11, 2026

View reviewed changes

		* Override the minimum height a flex item gets, which defaults to
		* use the height of its own content. Without this, a modal sized

Uh oh!

Conversation

brandyscarney commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the current behavior?

What is the new behavior?

Does this introduce a breaking change?

Other information

Uh oh!

vercel Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brandyscarney Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

thetaPC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thetaPC Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

brandyscarney Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thetaPC left a comment

Choose a reason for hiding this comment

Uh oh!

ShaneK left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShaneK Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

brandyscarney Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

ShaneK Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShaneK left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneK Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

ShaneK left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

brandyscarney commented Aug 28, 2026 •

edited

Loading

vercel Bot commented Aug 28, 2026 •

edited

Loading

ShaneK left a comment •

edited

Loading