Skip to content

Commit 1d232c8

Browse files
committed
fix(modal): attempting to fix scenarios where the modal would become fully constrained
1 parent 7584e61 commit 1d232c8

4 files changed

Lines changed: 69 additions & 36 deletions

File tree

core/src/components/popover/animations/ios.enter.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -158,12 +158,17 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
158158
if (bottomValue !== undefined) {
159159
contentEl.style.setProperty('bottom', `calc(${bottomValue})`);
160160
/**
161-
* When both top and bottom are set, we need to override the
162-
* height: var(--height) style to allow the top/bottom constraint
163-
* to determine the height. Setting height to 'auto' with both
164-
* top and bottom defined would cause bottom to be ignored.
161+
* When both top and bottom are explicitly constrained (isFullyConstrained),
162+
* we need to override the height: var(--height) style to allow the
163+
* top/bottom constraint to determine the height.
164+
*
165+
* We only do this when fully constrained because setting height: unset
166+
* when only bottom is set (without explicit top) would result in an
167+
* incorrectly sized popover.
165168
*/
166-
contentEl.style.setProperty('height', 'unset');
169+
if (isFullyConstrained) {
170+
contentEl.style.setProperty('height', 'unset');
171+
}
167172
}
168173

169174
contentEl.style.setProperty('top', `calc(${topValue} + var(--offset-y, 0))`);

core/src/components/popover/animations/md.enter.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
6565
checkSafeAreaBottom,
6666
checkSafeAreaLeft,
6767
checkSafeAreaRight,
68+
isFullyConstrained,
6869
} = calculateWindowAdjustment(
6970
side,
7071
results.top,
@@ -135,12 +136,17 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
135136
if (bottomValue !== undefined) {
136137
contentEl.style.setProperty('bottom', `calc(${bottomValue})`);
137138
/**
138-
* When both top and bottom are set, we need to override the
139-
* height: var(--height) style to allow the top/bottom constraint
140-
* to determine the height. Setting height to 'auto' with both
141-
* top and bottom defined would cause bottom to be ignored.
139+
* When both top and bottom are explicitly constrained (isFullyConstrained),
140+
* we need to override the height: var(--height) style to allow the
141+
* top/bottom constraint to determine the height.
142+
*
143+
* We only do this when fully constrained because setting height: unset
144+
* when only bottom is set (without explicit top) would result in an
145+
* incorrectly sized popover.
142146
*/
143-
contentEl.style.setProperty('height', 'unset');
147+
if (isFullyConstrained) {
148+
contentEl.style.setProperty('height', 'unset');
149+
}
144150
}
145151
})
146152
.fromTo('transform', 'scale(0.8)', 'scale(1)');

core/src/components/popover/test/safe-area/popover.e2e.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,25 @@ configs({ modes: ['ios', 'md'], directions: ['ltr'] }).forEach(({ title, config
2525
description: 'https://github.com/ionic-team/ionic-framework/issues/30900',
2626
});
2727

28-
// Use a smaller viewport to force the popover to be constrained
29-
await page.setViewportSize({ width: 375, height: 500 });
28+
/**
29+
* Use a small viewport to force the popover to be fully constrained.
30+
* The large popover has 15 items (~700px), which will exceed the available
31+
* space in this viewport, causing it to be constrained with both top and
32+
* bottom edges near the safe areas.
33+
*
34+
* A 300px viewport ensures there's not enough space above OR below the
35+
* trigger for the full popover content, triggering the fully constrained path.
36+
*/
37+
await page.setViewportSize({ width: 375, height: 300 });
3038

3139
const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent');
3240

33-
// Click the trigger near the bottom of the screen
34-
await page.click('#bottom-trigger');
41+
// Click the large popover trigger which has enough content to extend toward the bottom
42+
await page.click('#large-popover-trigger');
3543
await ionPopoverDidPresent.next();
3644

37-
// Target the specific popover that was presented (the one with trigger="bottom-trigger")
38-
const popover = page.locator('ion-popover[trigger="bottom-trigger"]');
45+
// Target the specific popover that was presented
46+
const popover = page.locator('ion-popover[trigger="large-popover-trigger"]');
3947
const popoverContent = popover.locator('.popover-content');
4048

4149
// Get the computed bottom style - should include safe-area calc

core/src/components/popover/utils.ts

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,19 @@ export const calculateWindowAdjustment = (
884884
* margins.
885885
*/
886886
if (triggerTop + triggerHeight + contentHeight > bodyHeight && (side === 'top' || side === 'bottom')) {
887-
if (triggerTop - contentHeight > 0) {
887+
/**
888+
* Calculate available space above and below, accounting for safe areas.
889+
* This ensures we flip to whichever side has more usable space.
890+
*/
891+
const spaceAbove = (triggerCoordinates?.top ?? triggerTop) - bodyPadding - safeAreaMargin;
892+
const spaceBelow = bodyHeight - triggerTop - triggerHeight - bodyPadding - safeAreaMargin;
893+
894+
/**
895+
* Flip above if:
896+
* 1. Content fits entirely above the trigger, OR
897+
* 2. There's more usable space above than below (accounting for safe areas)
898+
*/
899+
if (triggerTop - contentHeight > 0 || spaceAbove > spaceBelow) {
888900
/**
889901
* While we strive to align the popover with the trigger
890902
* on smaller screens this is not always possible. As a result,
@@ -910,25 +922,27 @@ export const calculateWindowAdjustment = (
910922
}
911923

912924
/**
913-
* After flipping above, check if popover still extends into bottom safe area.
914-
* This can happen when the popover is taller than the available space between
915-
* the top safe area and the trigger. In this case, constrain with bottom too.
925+
* After flipping above, check if popover will likely overflow the viewport.
926+
* This can happen when the popover is taller than the available space.
916927
*
917-
* We estimate the effective top by adding safeAreaMargin if checkSafeAreaTop
918-
* is true (since CSS will add the actual safe-area-top value).
928+
* When checkSafeAreaTop is true, the CSS will add safe-area-top to the
929+
* top position, pushing the popover down. Since we don't know the exact
930+
* CSS safe-area value, we use a threshold that accounts for likely
931+
* safe-area sizes. This only triggers when:
932+
* 1. We're already applying safe-area-top (checkSafeAreaTop), and
933+
* 2. The popover is close enough to overflowing that any safe-area
934+
* would push it past the viewport
919935
*/
920-
const estimatedTop = checkSafeAreaTop ? top + safeAreaMargin : top;
921-
if (estimatedTop + contentHeight > bodyHeight - safeAreaMargin) {
936+
if (checkSafeAreaTop && top + contentHeight > bodyHeight - safeAreaMargin - bodyPadding) {
922937
bottom = bodyPadding;
923938
checkSafeAreaBottom = true;
924939
isFullyConstrained = true;
925940
}
926941

927942
/**
928-
* If not enough room for popover to appear
929-
* above trigger, constrain to full viewport.
930-
* Pin both top and bottom to maximize visible area
931-
* and let the content scroll within those bounds.
943+
* If not enough room for popover to appear above trigger
944+
* (i.e., content is taller than space above), then constrain
945+
* the popover to fill the entire viewport from top to bottom.
932946
*/
933947
} else {
934948
top = bodyPadding;
@@ -940,19 +954,19 @@ export const calculateWindowAdjustment = (
940954
}
941955

942956
/**
943-
* Final check: If the popover extends into any safe-area region,
944-
* constrain it to avoid overlapping system UI.
945-
* This handles cases where a side-positioned popover (left/right)
946-
* or a bottom-positioned popover extends into the safe area.
957+
* Check if popover is near edges and needs safe-area adjustments.
958+
* When the popover extends into the safe-area zone, set a bottom constraint
959+
* to push it up and out of the unsafe area. This is essential for
960+
* edge-to-edge displays on Android API 36+ and iOS devices with home indicators.
947961
*/
948962
const popoverBottom = bottom !== undefined ? bodyHeight - bottom : top + contentHeight;
949-
if (popoverBottom + safeAreaMargin > bodyHeight && bottom === undefined) {
963+
if (popoverBottom > bodyHeight - safeAreaMargin && bottom === undefined) {
964+
checkSafeAreaBottom = true;
950965
/**
951-
* Popover extends into bottom safe area but isn't already constrained.
952-
* Set bottom to constrain the popover and apply safe-area adjustment.
966+
* Set a bottom constraint to push the popover up out of the safe-area zone.
967+
* The animation will add the safe-area CSS variable to this value.
953968
*/
954969
bottom = bodyPadding;
955-
checkSafeAreaBottom = true;
956970
}
957971
if (top < safeAreaMargin) {
958972
checkSafeAreaTop = true;

0 commit comments

Comments
 (0)