Skip to content

Commit 4e8ecc2

Browse files
committed
fix(modal): prevent ion-content collapsing at content-based heights
resolves #31149
1 parent bfff8e1 commit 4e8ecc2

3 files changed

Lines changed: 78 additions & 2 deletions

File tree

core/src/components/content/content.tsx

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,26 @@ export class Content implements ComponentInterface {
310310
return forceOverscroll === undefined ? mode === 'ios' && isPlatform('ios') : forceOverscroll;
311311
}
312312

313+
/**
314+
* Whether this component should size itself to its contents height, which
315+
* is the case inside any popover and inside a modal whose `--height` is a
316+
* content-based value. Those overlays give the content no definite height
317+
* to fill.
318+
*/
319+
private shouldSizeToContent() {
320+
if (hostContext('ion-popover', this.el)) {
321+
return true;
322+
}
323+
324+
const modal = this.el.closest('ion-modal');
325+
if (modal === null) {
326+
return false;
327+
}
328+
329+
const height = getComputedStyle(modal).getPropertyValue('--height').trim();
330+
return CONTENT_SIZED_HEIGHTS.includes(height);
331+
}
332+
313333
private resize() {
314334
/**
315335
* Only force update if the component is rendered in a browser context.
@@ -538,7 +558,7 @@ export class Content implements ComponentInterface {
538558
class={createColorClasses(this.color, {
539559
[mode]: true,
540560
'content-fullscreen': this.fullscreen,
541-
'content-sizing': hostContext('ion-popover', this.el),
561+
'content-sizing': this.shouldSizeToContent(),
542562
overscroll: forceOverscroll,
543563
[`content-${rtl}`]: true,
544564
})}
@@ -579,6 +599,12 @@ export class Content implements ComponentInterface {
579599
}
580600
}
581601

602+
/**
603+
* `ion-modal` `--height` values that size the modal to its contents, leaving
604+
* children an indefinite height to resolve against.
605+
*/
606+
const CONTENT_SIZED_HEIGHTS = ['auto', 'fit-content', 'min-content', 'max-content'];
607+
582608
const getParentElement = (el: any) => {
583609
if (el.parentElement) {
584610
// normal element with a parent element

core/src/components/modal/modal.scss

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,12 @@
2727
--max-width: auto;
2828
--height: 100%;
2929
--min-height: auto;
30-
--max-height: auto;
30+
/**
31+
* Clamps a content-sized `--height` (auto, fit-content, ...) to the
32+
* overlay, giving the wrapper's flex children something to shrink
33+
* toward so `ion-content` scrolls instead of overflowing.
34+
*/
35+
--max-height: 100%;
3136
--overflow: hidden;
3237
--border-radius: 0;
3338
--border-width: 0;
@@ -87,8 +92,16 @@ ion-backdrop {
8792
/**
8893
* The wrapper receives programmatic focus for screen readers but should not
8994
* show a visible focus ring, which is meant only for keyboard navigation.
95+
*
96+
* A flex layout is required for the wrapper to size itself to its content
97+
* when the modal is content-sized (`--height` is auto, fit-content, ...).
98+
* This makes it so that the content can scroll when it overflows the wrapper.
9099
*/
91100
.modal-wrapper {
101+
display: flex;
102+
103+
flex-direction: column;
104+
92105
outline: none;
93106
}
94107

core/src/css/core.scss

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,9 +203,46 @@ ion-modal > .ion-page {
203203

204204
contain: layout style;
205205

206+
/**
207+
* Override the minimum height a flex item gets, which defaults to
208+
* use the height of its own content. Without this, a modal sized
209+
* to its content clips its overflow instead of scrolling it.
210+
*/
211+
min-height: 0;
212+
206213
height: 100%;
207214
}
208215

216+
/**
217+
* Position the `ion-nav` and its page relatively when inside of an
218+
* `ion-content` that is sized to its content. This allows the `ion-nav`
219+
* to take its height from its page and size itself correctly. Without
220+
* this, the modal will not appear as the nav will be 0 height.
221+
*/
222+
ion-content.content-sizing ion-nav,
223+
ion-content.content-sizing ion-nav > .ion-page {
224+
position: relative;
225+
226+
contain: layout style;
227+
228+
height: auto;
229+
}
230+
231+
/**
232+
* Place every page in the same grid cell so they overlap, while still
233+
* letting the nav take its height from the tallest of them. Without
234+
* this, a transition that has two pages in the tree at once would
235+
* render them one below the other.
236+
*/
237+
ion-content.content-sizing ion-nav {
238+
display: grid;
239+
}
240+
241+
ion-content.content-sizing ion-nav > .ion-page {
242+
grid-row: 1;
243+
grid-column: 1;
244+
}
245+
209246
.split-pane-visible > .ion-page.split-pane-main {
210247
position: relative;
211248
}

0 commit comments

Comments
 (0)