Repository navigation
fix(select-modal): zero top safe-area inset for centered dropdown modal in the Ionic theme - #31750
OS-susmitabhowmik wants to merge 4 commits into
Conversation
The centered select modal (Interface Type = Modal) never touches the top edge of the viewport. Ionic predicts fullscreen coverage before the open animation and pads the header with the status-bar safe-area height, then corrects it afterwards, causing the content to visibly jump up on Android devices with a status bar. Setting --ion-safe-area-top: 0px on the select-modal host when inside a default modal prevents the toolbar from inheriting the status-bar padding.
Verifies that a non-zero --ion-safe-area-top does not add extra padding above the toolbar in the centered (default) select modal.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
Very close! Just a couple of things inline, mainly the new rule also catching fullscreen select modals.
| // Centered Dropdown modal (Interface Type = Modal) never touches the top edge. | ||
| // Ionic predicts a fullscreen modal before the open animation and pads the header | ||
| // with the status bar height, then removes it afterwards, so the content jumps up. | ||
| :host(.in-modal-default) { | ||
| --ion-safe-area-top: 0px; | ||
| } |
There was a problem hiding this comment.
| // Centered Dropdown modal (Interface Type = Modal) never touches the top edge. | |
| // Ionic predicts a fullscreen modal before the open animation and pads the header | |
| // with the status bar height, then removes it afterwards, so the content jumps up. | |
| :host(.in-modal-default) { | |
| --ion-safe-area-top: 0px; | |
| } | |
| // The select's centered modal never reaches the top edge unless the viewport | |
| // is too short to fit its 340px min-height, so it does not need the top safe area. | |
| @media (min-height: 351px) { | |
| :host-context(ion-modal.select-modal.modal-default) { | |
| --ion-safe-area-top: 0px; | |
| } | |
| } |
I think this is too broad. The in-modal-default class gets added for any non-sheet ion-modal, but only the modal that ion-select opens has the select-modal class that centers it. If someone puts ion-select-modal in their own ion-modal, like the custom test page does, it's still fullscreen, and with an 80px top inset the toolbar padding goes from 80px on next to 0px here, so the header ends up under the status bar. The select's own modal hits it too on short screens, since at 740x340 in landscape the 340px min-height fills the viewport.
This version only applies the rule when the modal is actually centered. The centered select modal still gets 0px on every frame of the open animation, and both of the cases above keep their 80px. At 351px tall and below, the modal's top is within the 5px edge threshold updateSafeAreaOverrides uses, so that's where it hands the inset back. I also trimmed the comment, because the current one walks through the modal internals and I think it'll go stale.
Base branch:
This PR:
| await page.addStyleTag({ content: ':root { --ion-safe-area-top: 80px; }' }); | ||
| await selectModalPage.setup(config, checkedOptions, false); |
There was a problem hiding this comment.
I don't think this test can catch the bug. Since setup() calls page.setContent, which navigates to a new document, the style tag added here is gone before the modal renders, so the screenshot is taken with a 0px inset. Even with the inset set, the fixture is a plain ion-modal without select-modal, so it's the fullscreen case from my other comment, and the screenshot is taken after the modal is presented, when the end state was already correct on next.
Could you open the modal through ion-select with interface="modal", set the inset after setContent, and check the toolbar's padding-top on ionModalWillPresent? The initial safe-area overrides are already applied by then, so I think it'd be 0px here and 80px on next. A second case that checks a fullscreen ion-modal with ion-select-modal inside keeps its 80px would cover the regression too.
| // Centered Dropdown modal (Interface Type = Modal) never touches the top edge. | ||
| // Ionic predicts a fullscreen modal before the open animation and pads the header | ||
| // with the status bar height, then removes it afterwards, so the content jumps up. | ||
| :host(.in-modal-default) { |
There was a problem hiding this comment.
I think the jump comes from getModalCoveredAxes in safe-area-utils. It counts the modal as fully covered whenever --width and --height are both fullscreen, and they're still 100% here because the centered select modal only sets --max-height, --max-width and --min-height. It's the same early return I ran into on the content-height PR. Any non-sheet modal with a max size does this in every theme, a plain ion-modal with --max-height: 50%; --max-width: 90% jumps on md on next as well.
Something like this gets rid of the jump for both the select modal and the generic one, and the default fullscreen modal keeps the shortcut since its --max-height is 100% and --max-width is auto:
const maxWidth = getOverlaySizeType(styles.getPropertyValue('--max-width'));
const maxHeight = getOverlaySizeType(styles.getPropertyValue('--max-height'));
const isClamped = maxWidth === 'definite' || maxHeight === 'definite';
if (width === 'fullscreen' && height === 'fullscreen' && !isClamped) {
return { vertical: true, horizontal: true };
}If that's out of scope here, could we make a card for it? This rule only covers the select modal in the Ionic theme.
next during the animation:
next after the animation:
| /** | ||
| * Verifies that a non-zero safe-area-top does not add | ||
| * extra padding above the toolbar in the centered select modal. | ||
| */ |
There was a problem hiding this comment.
| /** | |
| * Verifies that a non-zero safe-area-top does not add | |
| * extra padding above the toolbar in the centered select modal. | |
| */ | |
| /** | |
| * The safe-area override only exists in the Ionic theme, | |
| * where the select modal is a centered dialog on phones. | |
| * This behavior does not vary across directions. | |
| */ |
Nit: the other configs() blocks in this file say why they're restricted, so I'd do the same here. Up to you!
Issue number: resolves internal
What is the current behavior?
On Android devices with a visible status bar, the centered dropdown modal (Interface Type = Modal) content starts lower and jumps up after the open animation completes (~0.3s).
This happens because Ionic's
setInitialSafeAreaOverrides()predicts fullscreen coverage before the animation and pads the header toolbar with the status-bar safe-area height (--ion-safe-area-top). After the animation,updateSafeAreaOverrides()detects the modal is centered (not touching the top edge) and corrects the value to0px, causing the visible content shift.The issue was reported on a Samsung Galaxy A33 with Android 13.
What is the new behavior?
ion-select-modal.in-modal-default) now sets--ion-safe-area-top: 0pxon its host element in the Ionic themeDoes this introduce a breaking change?
Other information
How to reproduce in a browser
document.documentElement.style.setProperty('--ion-safe-area-top', '80px')Testing
--ion-safe-area-top: 80pxand verifies no extra padding appears above the toolbar in the centered select modal (ionic-md mode)