fix(modal, popover): wait for contents to mount (#27344)

Issue number: resolves #27343

---------

<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->

<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

In
30e3a1485d
I removed the `deepWait` call from popover/modal in custom element
bundle environments (React and Vue as of writing). This had an
unintended side effect where WebKit/iOS would not play the modal enter
animation correctly because the inner contents are mounted
mid-animation. This does not impact other mobile platforms.

This only impacted the modal because popover had a patch in
be9a399eee
which causes it to wait for the JS Framework to finish mounting before
proceeding with the transition.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Modal now emits `ionMount` event and waits 2 frames before proceeding
with the animation.

Note 1: The JS Framework overlay components were already updated to
support this `ionMount` event in
be9a399eee.

I also updated the modal Angular component to listen for `ionMount`. It
is not needed right now because Angular does not use the custom elements
bundle and therefore does not call `ionMount` (it runs the `deepReady`
function though). However, if we move Angular to support the custom
elements bundle in the future this may become an issue. This behavior
currently exists in the popover component for Angular too.

Note 2: This does appear to be a WebKit bug since it does not happen on
Android. However, this patch seems fairly safe which is why I've opted
to try and fix it internally instead of waiting for a patch from Apple.


| before | after |
| - | - |
| <video
src="https://user-images.githubusercontent.com/2721089/235495325-2f258526-0c43-422b-84c3-ac4f5e228bbd.MP4"></video>
| <video
src="https://user-images.githubusercontent.com/2721089/235495362-9b3bb35d-782c-4a8f-ac13-8aaa8f17729b.MP4"></video>
|


## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

angular/src/directives/overlays/modal.ts

@@ -116,7 +116,7 @@ export class IonModal {
   constructor(c: ChangeDetectorRef, r: ElementRef, protected z: NgZone) {
     this.el = r.nativeElement;
 
-    this.el.addEventListener('willPresent', () => {
+    this.el.addEventListener('ionMount', () => {
       this.isCmpOpen = true;
       c.detectChanges();
     });
@@ -124,7 +124,6 @@ export class IonModal {
       this.isCmpOpen = false;
       c.detectChanges();
     });
-
     proxyOutputs(this, this.el, [
       'ionModalDidPresent',
       'ionModalWillPresent',

core/src/components.d.ts

@@ -5830,6 +5830,10 @@ declare namespace LocalJSX {
           * Emitted before the modal has presented.
          */
         "onIonModalWillPresent"?: (event: IonModalCustomEvent<void>) => void;
+        /**
+          * Emitted before the modal has presented, but after the component has been mounted in the DOM. This event exists so iOS can run the entering transition properly
+         */
+        "onIonMount"?: (event: IonModalCustomEvent<void>) => void;
         /**
           * Emitted before the modal has dismissed. Shorthand for ionModalWillDismiss.
          */

core/src/components/modal/modal.tsx

@@ -31,7 +31,7 @@ import {
 } from '../../utils/overlays';
 import type { OverlayEventDetail } from '../../utils/overlays-interface';
 import { getClassMap } from '../../utils/theme';
-import { deepReady } from '../../utils/transition';
+import { deepReady, waitForMount } from '../../utils/transition';
 
 import { iosEnterAnimation } from './animations/ios.enter';
 import { iosLeaveAnimation } from './animations/ios.leave';
@@ -316,6 +316,16 @@ export class Modal implements ComponentInterface, OverlayInterface {
    */
   @Event({ eventName: 'didDismiss' }) didDismissShorthand!: EventEmitter<OverlayEventDetail>;
 
+  /**
+   * Emitted before the modal has presented, but after the component
+   * has been mounted in the DOM.
+   * This event exists so iOS can run the entering
+   * transition properly
+   *
+   * @internal
+   */
+  @Event() ionMount!: EventEmitter<void>;
+
   breakpointsChanged(breakpoints: number[] | undefined) {
     if (breakpoints !== undefined) {
       this.sortedBreakpoints = breakpoints.sort((a, b) => a - b);
@@ -443,7 +453,30 @@ export class Modal implements ComponentInterface, OverlayInterface {
 
     const { inline, delegate } = this.getDelegate(true);
     this.usersElement = await attachComponent(delegate, el, this.component, ['ion-page'], this.componentProps, inline);
-    hasLazyBuild(el) && (await deepReady(this.usersElement));
+
+    this.ionMount.emit();
+
+    /**
+     * When using the lazy loaded build of Stencil, we need to wait
+     * for every Stencil component instance to be ready before presenting
+     * otherwise there can be a flash of unstyled content. With the
+     * custom elements bundle we need to wait for the JS framework
+     * mount the inner contents of the overlay otherwise WebKit may
+     * get the transition incorrect.
+     */
+    if (hasLazyBuild(el)) {
+      await deepReady(this.usersElement);
+      /**
+       * If keepContentsMounted="true" then the
+       * JS Framework has already mounted the inner
+       * contents so there is no need to wait.
+       * Otherwise, we need to wait for the JS
+       * Framework to mount the inner contents
+       * of this component.
+       */
+    } else if (!this.keepContentsMounted) {
+      await waitForMount();
+    }
 
     writeTask(() => this.el.classList.add('show-modal'));
 

core/src/components/popover/popover.tsx

@@ -10,7 +10,7 @@ import { BACKDROP, dismiss, eventMethod, focusFirstDescendant, prepareOverlay, p
 import type { OverlayEventDetail } from '../../utils/overlays-interface';
 import { isPlatform } from '../../utils/platform';
 import { getClassMap } from '../../utils/theme';
-import { deepReady } from '../../utils/transition';
+import { deepReady, waitForMount } from '../../utils/transition';
 
 import { iosEnterAnimation } from './animations/ios.enter';
 import { iosLeaveAnimation } from './animations/ios.leave';
@@ -455,7 +455,6 @@ export class Popover implements ComponentInterface, PopoverInterface {
       this.componentProps,
       inline
     );
-    hasLazyBuild(el) && (await deepReady(this.usersElement));
 
     if (!this.keyboardEvents) {
       this.configureKeyboardInteraction();
@@ -464,52 +463,50 @@ export class Popover implements ComponentInterface, PopoverInterface {
 
     this.ionMount.emit();
 
-    return new Promise((resolve) => {
+    /**
+     * When using the lazy loaded build of Stencil, we need to wait
+     * for every Stencil component instance to be ready before presenting
+     * otherwise there can be a flash of unstyled content. With the
+     * custom elements bundle we need to wait for the JS framework
+     * mount the inner contents of the overlay otherwise WebKit may
+     * get the transition incorrect.
+     */
+    if (hasLazyBuild(el)) {
+      await deepReady(this.usersElement);
       /**
-       * Wait two request animation frame loops before presenting the popover.
+       * If keepContentsMounted="true" then the
-       * This allows the framework implementations enough time to mount
+       * JS Framework has already mounted the inner
-       * the popover contents, so the bounding box is set when the popover
+       * contents so there is no need to wait.
-       * transition starts.
+       * Otherwise, we need to wait for the JS
-       *
+       * Framework to mount the inner contents
-       * On Angular and React, a single raf is enough time, but for Vue
+       * of this component.
-       * we need to wait two rafs. As a result we are using two rafs for
-       * all frameworks to ensure the popover is presented correctly.
        */
-      raf(() => {
+    } else if (!this.keepContentsMounted) {
-        raf(async () => {
+      await waitForMount();
-          this.currentTransition = present<PopoverPresentOptions>(
+    }
-            this,
-            'popoverEnter',
-            iosEnterAnimation,
-            mdEnterAnimation,
-            {
-              event: event || this.event,
-              size: this.size,
-              trigger: this.triggerEl,
-              reference: this.reference,
-              side: this.side,
-              align: this.alignment,
-            }
-          );
 
-          await this.currentTransition;
+    this.currentTransition = present<PopoverPresentOptions>(this, 'popoverEnter', iosEnterAnimation, mdEnterAnimation, {
-
+      event: event || this.event,
-          this.currentTransition = undefined;
+      size: this.size,
-
+      trigger: this.triggerEl,
-          /**
+      reference: this.reference,
-           * If popover is nested and was
+      side: this.side,
-           * presented using the "Right" arrow key,
+      align: this.alignment,
-           * we need to move focus to the first
-           * descendant inside of the popover.
-           */
-          if (this.focusDescendantOnPresent) {
-            focusFirstDescendant(this.el, this.el);
-          }
-
-          resolve();
-        });
-      });
     });
+
+    await this.currentTransition;
+
+    this.currentTransition = undefined;
+
+    /**
+     * If popover is nested and was
+     * presented using the "Right" arrow key,
+     * we need to move focus to the first
+     * descendant inside of the popover.
+     */
+    if (this.focusDescendantOnPresent) {
+      focusFirstDescendant(this.el, this.el);
+    }
   }
 
   /**

core/src/utils/transition/index.ts

@@ -198,6 +198,23 @@ export const lifecycle = (el: HTMLElement | undefined, eventName: string) => {
   }
 };
 
+/**
+ * Wait two request animation frame loops.
+ * This allows the framework implementations enough time to mount
+ * the user-defined contents. This is often needed when using inline
+ * modals and popovers that accept user components. For popover,
+ * the contents must be mounted for the popover to be sized correctly.
+ * For modals, the contents must be mounted for iOS to run the
+ * transition correctly.
+ *
+ * On Angular and React, a single raf is enough time, but for Vue
+ * we need to wait two rafs. As a result we are using two rafs for
+ * all frameworks to ensure contents are mounted.
+ */
+export const waitForMount = (): Promise<void> => {
+  return new Promise((resolve) => raf(() => raf(() => resolve())));
+};
+
 export const deepReady = async (el: any | undefined): Promise<void> => {
   const element = el as any;
   if (element) {