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.

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

bug: inline popover positioned wrong with auto width #24716

Closed
4 of 6 tasks
EinfachHans opened this issue Feb 4, 2022 · 14 comments
Closed
4 of 6 tasks

bug: inline popover positioned wrong with auto width #24716

EinfachHans opened this issue Feb 4, 2022 · 14 comments
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@EinfachHans
Copy link
Contributor

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x

Current Behavior

I have an inline Popover with either --width: fit-content or --width: auto. This causes the positioning to be wrong and the popover is out of the screen.

Expected Behavior

The Positioning should be the same if inline or not. Also the arrow is not displayed 8for both), why is that?

Steps to Reproduce

See linked Repo. The Popovers have the same content but only the inline one is positioned wrong

Code Reproduction URL

https://github.com/EinfachHans/ionic-popover-issue

Ionic Info

Ionic:

Ionic CLI : 6.18.1 (/Users/hanskrywalsky/.nvm/versions/node/v14.17.0/lib/node_modules/@ionic/cli)
Ionic Framework : @ionic/angular 6.0.5
@angular-devkit/build-angular : 13.0.4
@angular-devkit/schematics : 13.0.4
@angular/cli : 13.0.4
@ionic/angular-toolkit : 5.0.3

Capacitor:

Capacitor CLI : 3.4.0
@capacitor/android : not installed
@capacitor/core : 3.4.0
@capacitor/ios : not installed

Utility:

cordova-res (update available: 0.15.4) : 0.15.1
native-run : 1.5.0

System:

NodeJS : v14.17.0 (/Users/hanskrywalsky/.nvm/versions/node/v14.17.0/bin/node)
npm : 6.14.13
OS : macOS Monterey

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Feb 4, 2022
@sean-perkins sean-perkins self-assigned this Feb 4, 2022
@sean-perkins
Copy link
Contributor

@EinfachHans thanks for reporting this issue!

I'm seeing the same issues as you, in your example. The ion-popover with inline usage, doesn't accurately calculate the positioning. It's possible that the width for fit-content cannot be evaluated until after the DOM node is rendered, so we may need to adjust how/when we parse that information. This affects both the left positioning, but also the transform-origin position for auto placement.

The arrow appears in the DOM, but is not visual. Manually modifying the positioning, I can get the arrow to become visible. In the interim, we'll track these bugs together, as the arrow positioning may be affected by the main issue.

@sean-perkins sean-perkins changed the title bug: inline popover positioned wrong with auto with bug: inline popover positioned wrong with auto width Feb 4, 2022
@sean-perkins sean-perkins added package: core @ionic/core package type: bug a confirmed bug report labels Feb 4, 2022
@ionitron-bot ionitron-bot bot removed the triage label Feb 4, 2022
@sean-perkins sean-perkins removed their assignment Feb 4, 2022
@EinfachHans
Copy link
Contributor Author

This is really annoying in our app atm^^ Is this included in one of your next sprints @sean-perkins ?

@sean-perkins
Copy link
Contributor

sean-perkins commented Sep 30, 2022

As a workaround, applying [keepContentsMounted]="true" to the inline popover avoids the issue.

The issue here appears to be that without that option enabled, when the inline overlay is presented in Angular, the present logic calculates the positioning of the popover before the inner contents actually render.

e.g.: ion-popover has a template of:

<ng-container [ngTemplateOutlet]="template" *ngIf="isCmpOpen || keepContentsMounted"></ng-container>

The template does not actually render until isCmpOpen is set to true, which happens when the willPresent event is emitted from the popover (as a result of the trigger being clicked).

I am going to experiment with wrapping the present logic in the web component with a requestAnimationFrame to see if that resolves the style calculations (only when presenting from a trigger). This wouldn't work because of the same reasoning of the event emission timing vs. render.

@sean-perkins
Copy link
Contributor

@EinfachHans can you test with this dev-build and let me know if you run into any issues?

npm install @ionic/angular@6.2.10-dev.11664565192.13ca046d

@EinfachHans
Copy link
Contributor Author

@sean-perkins looks good 😃👍🏼

@EinfachHans
Copy link
Contributor Author

@sean-perkins Can you include you fix into the next version?

@sean-perkins
Copy link
Contributor

This issue has been resolved and will be available in the nightly build as well as the next patch release (6.3.7) of Ionic. Thanks!

@EinfachHans
Copy link
Contributor Author

@sean-perkins it's not really working with the nightly build as is did with the dev build:

6.2.10-dev.11664565192.13ca046d 6.3.7-nightly.20221111
Bildschirm­foto 2022-11-11 um 11 01 31 Bildschirm­foto 2022-11-11 um 11 02 37

@sean-perkins sean-perkins self-assigned this Nov 11, 2022
@sean-perkins sean-perkins reopened this Nov 11, 2022
@sean-perkins
Copy link
Contributor

@EinfachHans can you update your reproduction app (or create a new branch/reproduction) that can reproduce what you are experiencing?

The merged fix is different than the proposed dev-build, to avoid changes to a public API (would push the resolution out to 6.4.0). We opted for a different approach that would allow the fix to ship in a patch release. With the nightly (6.3.7-nightly.20221111) target, the reproduction app is positioning as expected.

If you can get me a sample of where it fails, I can take a look!

@sean-perkins sean-perkins added the needs: reply the issue needs a response from the user label Nov 11, 2022
@sean-perkins sean-perkins removed their assignment Nov 11, 2022
@EinfachHans
Copy link
Contributor Author

Hey @sean-perkins , check out this repo: https://github.com/EinfachHans/ionic-popover-arrow-bug

I opened it for #25142 which was closed as an duplicate of this issue. There i don't use an inline popover but an popover via controller which should be correctly positioned as well of course

Controller Inline
Bildschirm­foto 2022-11-11 um 19 33 05 Bildschirm­foto 2022-11-11 um 19 33 09

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Nov 11, 2022
@sean-perkins
Copy link
Contributor

Interesting 🤔 that is another difference between the dev-build vs. the proposed fix. The dev-build targeted code that applied to all presentation types (controller, inline), where as the proposed fix strictly targets inline. This is my bad, I had assumed based on the reproduction that this was isolated to inline & tried to isolate the affected code to as narrow of a scenario as possible.

Let me get a dev-build of that check removed and test it against the other reproduction. If it works, I'll additionally post it here for you to test with.

@sean-perkins sean-perkins self-assigned this Nov 11, 2022
@sean-perkins
Copy link
Contributor

sean-perkins commented Nov 11, 2022

Ok, looks like removing that extra check works for both controller and inline popovers.

Can you test with dev-build:

npm install @ionic/angular@6.3.7-dev.11668193398.1b5ffdd3

Let me know if you observe the same.

@sean-perkins sean-perkins added needs: reply the issue needs a response from the user and removed triage labels Nov 11, 2022
@EinfachHans
Copy link
Contributor Author

Yes i do, looks good 👍🏼

@ionitron-bot
Copy link

ionitron-bot bot commented Dec 14, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Dec 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests

2 participants