Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bug: popover positioning does not properly account for safe area #28411

Open
3 tasks done
NLueg opened this issue Oct 25, 2023 · 5 comments · May be fixed by #29068
Open
3 tasks done

bug: popover positioning does not properly account for safe area #28411

NLueg opened this issue Oct 25, 2023 · 5 comments · May be fixed by #29068
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@NLueg
Copy link

NLueg commented Oct 25, 2023 •
edited
Loading

Prerequisites

Describe the Feature Request

The positioning of popovers should include the safe area of the device. At least it should be possible to add it manually w

Describe the Use Case

On devices with safe areas, it can appear, that the select popover overflows the content of the safe area.
This is an unwanted behavior and it at least should be possible to determine whether the popover opens to the bottom or top to add the safe area margin manually.

In the screenshot you can see what happens when I add a large safe area:
image

The button below the popover has the safe-area while the popover overflows the safe area.

Describe Preferred Solution

Add a CSS class based on originY to add specific margins for the safe area based on the current position.

Describe Alternatives

Handle the safe area directly inside the framework like the left property.
@ionitron-bot ionitron-bot bot added the triage label Oct 25, 2023
@liamdebeasi liamdebeasi added the ionitron: needs reproduction a code reproduction is needed from the issue author label Oct 25, 2023
@ionitron-bot
Copy link

ionitron-bot bot commented Oct 25, 2023

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

@ionitron-bot ionitron-bot bot removed the triage label Oct 25, 2023
@NLueg
Copy link
Author

NLueg commented Oct 26, 2023

Hey, I just created an example application: https://github.com/NLueg/ionic-popover-safe-area-issue
Bildschirm­foto 2023-10-26 um 04 59 01

@averyjohnston averyjohnston added triage and removed ionitron: needs reproduction a code reproduction is needed from the issue author labels Oct 26, 2023
@averyjohnston averyjohnston self-assigned this Oct 26, 2023
@averyjohnston
Copy link
Contributor

Thanks! This is a bug we should fix. It looks like it applies to all popovers, not just ones from ion-select.

The utility that adjusts the popover's position to not overflow the window does not seem to account for top or bottom safe area:

ionic-framework/core/src/components/popover/utils.ts

Line 865 in dc94ae0

if (triggerTop + triggerHeight + contentHeight > bodyHeight && (side === 'top' || side === 'bottom')) {

The calculation for left or right safe area seems incorrect as well. While the safe area is accounted for there...

ionic-framework/core/src/components/popover/utils.ts

Line 844 in dc94ae0

if (left < bodyPadding + safeAreaMargin) {

ionic-framework/core/src/components/popover/utils.ts

Line 852 in dc94ae0

} else if (contentWidth + bodyPadding + left + safeAreaMargin > bodyWidth) {

... the safeAreaMargin parameter looks to be used incorrectly. MD always passes in a value of 0:

ionic-framework/core/src/components/popover/animations/md.enter.ts

Line 59 in dc94ae0

0,

And iOS uses a similar static value:

ionic-framework/core/src/components/popover/animations/ios.enter.ts

Line 56 in dc94ae0

const margin = size === 'cover' ? 0 : 25;

This is less impactful than with top/bottom safe area because the popover's trigger will likely be positioned within the safe area, and the contents of the popover itself (such as an ion-list) may also receive padding to account for it.

@averyjohnston averyjohnston changed the title feat: make it possible to add a safe-area for select-popovers bug: popover positioning does not properly account for safe area Oct 26, 2023
@averyjohnston averyjohnston added package: core @ionic/core package type: bug a confirmed bug report labels Oct 26, 2023
@averyjohnston averyjohnston removed their assignment Oct 26, 2023
@ionitron-bot ionitron-bot bot removed the triage label Oct 26, 2023
@btsiders

This comment was marked as off-topic.

Sign in to view
@averyjohnston averyjohnston linked a pull request Feb 16, 2024 that will close this issue
Draft
2 tasks
@emmaus-zam
Copy link

from my understanding immersive mode for the bottom navbar is now the expected default in Android apps, so most apps will have a big bottom safe area. kindly mentioning that since this will affect a lot of use cases and can completely break interacting with the App.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(popover): adjust position to account for approximate safe area ionic-team/ionic-framework
5 participants
@btsiders @liamdebeasi @NLueg @averyjohnston @emmaus-zam