fix(popover): inline popover positioning with fit-content or auto width

[sean-perkins]

Pull request checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Some docs updates need to be made in the ionic-docs repo, in a separate PR. See the contributing guide for details.
• Build (npm run build) was run locally and any changes were pushed
• Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

What is the current behavior?

Inline popovers with a width of auto or fit-content are incorrectly positioned when the popover is presented.

This bug would apply to any enter animation for overlays that relies on the dimensions of the content inside of the overlay, to position the overlay during the transition.

Issue URL: #24716

What is the new behavior?

• Inline popovers with a width of auto or fit-content are positioned correctly when presented.
• AnimationBuilder can be resolved asynchronously, to handle animations that require additional async operations

Does this introduce a breaking change?

• Yes
• No

Other information

[liamdebeasi]

I tested this on a Samsung Galaxy S3 and the changes worked correctly in the sample application. I did not note any major performance degredation.

However, this is an API change so we need to target a feature branch. Alternatively, is there a way we can pull this logic into ion-popover and ion-modal?

Something like the following:

async present() {

  /**
   * Check to see if the .ion-page element has not been added
   * You could also check to see if this component is inline
   */
  if (doesNotHaveIonPageElement) {
    /**
     * Fire the ionMount event
     * The framework wrappers will need to listen for ionMount instead of willPresent
     */
    this.ionMount.emit();
    
    /**
     * Wait one raf
     * Alternatively, can use the delegateController to detect when
     * the JS Framework has mounted the inner content? If not, then the raf
     * is probably fine.
     */
    await waitOneFrame();
  }

  // regular present logic
  present();
}

This should have the same functionality as your current approach but without any feature changes to the animation library, letting us get this fix into a patch instead of a minor. This code also has a fast track for controller and keepContentsMounted popovers which lets it skip the raf.

[sean-perkins]

With moving this logic to the component-layer, it would introduce an additional requestAnimationFrame for custom animations (unless we imposed extra checks to determine if the transition is the built-in one or custom).

Personally, I would rather target this change for 6.4.0, knowing that it helps solve animation limitations when the transition is dependent on reading the dimensions of the animating element. There is also a viable workaround in the interim, of using keepContentsMounted to avoid this bug, so I do not think we need to fast track a fix.

Thoughts?

[liamdebeasi]

I'm fine with either option, but I think it's strange to change the public API of all overlay components in addition to page transitions to fix a bug only currently found in ion-popover.

If developers are creating a custom popover animation they are still going to need to read the dimensions of the component to position the popover correctly (unless the custom popovers always have a fixed dimension, though that is a risk for both implementations), and the alternative implementation fixes the issue for custom animations too. With the original fix, developers would need to update their transition to make use of a requestAnimationFrame and a Promise.