ionic-team · averyjohnston · Feb 16, 2024 · Feb 16, 2024 · Feb 16, 2024 · Feb 16, 2024
@@ -53,20 +53,8 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
   );
 
   const padding = size === 'cover' ? 0 : POPOVER_IOS_BODY_PADDING;
-  const margin = size === 'cover' ? 0 : 25;
-
-  const {
-    originX,
-    originY,
-    top,
-    left,
-    bottom,
-    checkSafeAreaLeft,
-    checkSafeAreaRight,
-    arrowTop,
-    arrowLeft,
-    addPopoverBottomClass,
-  } = calculateWindowAdjustment(
+
+  const { originX, originY, top, left, bottom, arrowTop, arrowLeft, addPopoverBottomClass } = calculateWindowAdjustment(
     side,
     results.top,
     results.left,
@@ -75,7 +63,6 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
     bodyHeight,
     contentWidth,
     contentHeight,
-    margin,
     results.originX,
     results.originY,
     results.referenceCoordinates,
@@ -122,20 +109,8 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
         contentEl.style.setProperty('bottom', `${bottom}px`);
       }
 
-      const safeAreaLeft = ' + var(--ion-safe-area-left, 0)';
-      const safeAreaRight = ' - var(--ion-safe-area-right, 0)';
-
-      let leftValue = `${left}px`;
-
-      if (checkSafeAreaLeft) {
-        leftValue = `${left}px${safeAreaLeft}`;
-      }
-      if (checkSafeAreaRight) {
-        leftValue = `${left}px${safeAreaRight}`;
-      }
-
       contentEl.style.setProperty('top', `calc(${top}px + var(--offset-y, 0))`);
-      contentEl.style.setProperty('left', `calc(${leftValue} + var(--offset-x, 0))`);
+      contentEl.style.setProperty('left', `calc(${left}px + var(--offset-x, 0))`);
       contentEl.style.setProperty('transform-origin', `${originY} ${originX}`);
 
       if (arrowEl !== null) {

@@ -56,7 +56,6 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
     bodyHeight,
     contentWidth,
     contentHeight,
-    0,
     results.originX,
     results.originY,
     results.referenceCoordinates

@@ -16,18 +16,43 @@
       import { popoverController } from '../../../../dist/ionic/index.esm.js';
       window.popoverController = popoverController;
     </script>
+
+    <style>
+      .safe-area-cover {
+        background-color: white;
+        position: absolute;
+
+        top: 5%;
+        bottom: 5%;
+        left: 0;
+        right: 0;
+
+        @media (orientation: landscape) {
+          left: 5%;
+          right: 5%;
+        }
+      }
+    </style>
   </head>
   <body>
     <ion-app>
       <ion-content>
-        <p style="text-align: center">Click everywhere to open the popover.</p>
+        <div style="text-align: center">
+          <p>Click everywhere to open the popover.</p>
+          <ion-checkbox id="safe-area-cb">Show Safe Area Approximation</ion-checkbox>
+        </div>
+
+        <div class="safe-area-cover"></div>
       </ion-content>
     </ion-app>
 
     <script>
       document.querySelector('ion-content').addEventListener('click', handleButtonClick);
+      document.querySelector('#safe-area-cb').addEventListener('ionChange', toggleSafeArea);
 
       async function handleButtonClick(ev) {
+        if (ev.target.tagName === 'ION-CHECKBOX') return;
+
         const popover = await popoverController.create({
           component: 'popover-example-page',
           event: ev,
@@ -37,6 +62,16 @@
         popover.present();
       }
 
+      function toggleSafeArea(ev) {
+        const content = document.querySelector('ion-content');
+
+        if (ev.detail.checked) {
+          content.style.setProperty('--background', 'lightblue');
+        } else {
+          content.style.removeProperty('--background');
+        }
+      }
+
       customElements.define(
         'popover-example-page',
         class PopoverContent extends HTMLElement {

@@ -1,5 +1,5 @@
 import { expect } from '@playwright/test';
-import { configs, test } from '@utils/test/playwright';
+import { configs, test, Viewports } from '@utils/test/playwright';
 
 /**
  * This behavior does not vary across modes/directions.
@@ -33,5 +33,45 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
 
       expect(box.y > 0).toBe(true);
     });
+
+    test('should account for vertical safe area approximation in portrait mode', async ({ page }) => {
+      await page.goto('/src/components/popover/test/adjustment', config);
+      const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent');
+
+      await page.mouse.click(0, 0);
+      await ionPopoverDidPresent.next();
+
+      const popoverContent = page.locator('ion-popover .popover-content');
+      const box = (await popoverContent.boundingBox())!;
+
+      /**
+       * The safe area approximation should move the y position by 5%
+       * of the screen height. We use 10px as the threshold to give
+       * wiggle room and help prevent flakiness.
+       */
+      expect(box.x < 10).toBe(true);
+      expect(box.y > 10).toBe(true);
+    });
+
+    test('should account for vertical and horizontal safe area approximation in landscape mode', async ({ page }) => {
+      await page.goto('/src/components/popover/test/adjustment', config);
+      const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent');
+
+      await page.setViewportSize(Viewports.tablet.landscape);
+
+      await page.mouse.click(0, 0);
+      await ionPopoverDidPresent.next();
+
+      const popoverContent = page.locator('ion-popover .popover-content');
+      const box = (await popoverContent.boundingBox())!;
+
+      /**
+       * The safe area approximation should move the y position by 5%
+       * of the screen height. We use 10px as the threshold to give
+       * wiggle room and help prevent flakiness.
+       */
+      expect(box.x > 10).toBe(true);
+      expect(box.y > 10).toBe(true);
+    });
   });
 });
@@ -1,3 +1,4 @@
+import { win } from '@utils/browser';
 import { getElementRoot, raf } from '@utils/helpers';
 
 import type { PopoverSize, PositionAlign, PositionReference, PositionSide, TriggerAction } from './popover-interface';
@@ -814,7 +815,6 @@ export const calculateWindowAdjustment = (
   bodyHeight: number,
   contentWidth: number,
   contentHeight: number,
-  safeAreaMargin: number,
   contentOriginX: string,
   contentOriginY: string,
   triggerCoordinates?: ReferenceCoordinates,
@@ -837,32 +837,72 @@ export const calculateWindowAdjustment = (
   const triggerHeight = triggerCoordinates ? triggerCoordinates.height : 0;
   let addPopoverBottomClass = false;
 
+  /**
+   * Approximate the safe area margins. Getting exact values would necessitate
+   * using window.getComputedStyle(), which is very expensive, so we use "close
+   * enough" values for now.
+   *
+   * 5% is derived from the iPhone 14 top safe area margin (47pt / 844 pt ~= 0.05).
+   * Source: https://useyourloaf.com/blog/iphone-14-screen-sizes/
+   *
+   * TODO(FW-5982): Investigate a more robust solution that uses the actual
+   * safe area margins through alternate means.
+   */
+  let horizontalSafeAreaApprox = 0;
+  let verticalSafeAreaApprox = 0;
+  if (win?.matchMedia !== undefined) {
+    verticalSafeAreaApprox = win.innerHeight * 0.05;
+
+    /**
+     * We only want to check horizontal safe area on landscape.
+     * Most devices do not have horizontal safe area margins in
+     * portrait mode, so enforcing it would lead to popovers
+     * being misaligned with the trigger when we don't want them
+     * to move.
+     */
+    if (win.matchMedia('(orientation: landscape)').matches) {
+      horizontalSafeAreaApprox = win.innerWidth * 0.05;
+    }
+  }
+
   /**
    * Adjust popover so it does not
    * go off the left of the screen.
    */
-  if (left < bodyPadding + safeAreaMargin) {
-    left = bodyPadding;
+  if (left < bodyPadding + horizontalSafeAreaApprox) {
+    left = bodyPadding + horizontalSafeAreaApprox;
     checkSafeAreaLeft = true;
     originX = 'left';
     /**
      * Adjust popover so it does not
      * go off the right of the screen.
      */
-  } else if (contentWidth + bodyPadding + left + safeAreaMargin > bodyWidth) {
+  } else if (contentWidth + bodyPadding + left + horizontalSafeAreaApprox > bodyWidth) {
     checkSafeAreaRight = true;
-    left = bodyWidth - contentWidth - bodyPadding;
+    left = bodyWidth - contentWidth - bodyPadding - horizontalSafeAreaApprox;
     originX = 'right';
   }
 
+  /**
+   * Ensure the popover doesn't sit above the safe area approxmation.
+   * If popover is on the left or right of the trigger, we should not
+   * adjust top margins.
+   */
+  if (side === 'top' || side === 'bottom') {
+    top = Math.max(top, verticalSafeAreaApprox + bodyPadding);
+  }
+
   /**
    * Adjust popover so it does not
    * go off the top of the screen.
    * If popover is on the left or the right of
    * the trigger, then we should not adjust top
    * margins.
    */
-  if (triggerTop + triggerHeight + contentHeight > bodyHeight && (side === 'top' || side === 'bottom')) {
+  if (
+    triggerTop + triggerHeight + contentHeight + verticalSafeAreaApprox > bodyHeight &&
+    (side === 'top' || side === 'bottom')
+  ) {
     if (triggerTop - contentHeight > 0) {
       /**
        * While we strive to align the popover with the trigger
@@ -875,6 +915,12 @@ export const calculateWindowAdjustment = (
        * it is not right up against the edge of the screen.
        */
       top = Math.max(12, triggerTop - contentHeight - triggerHeight - (arrowHeight - 1));
+
+      /**
+       * Ensure the popover doesn't sit below the safe area approxmation.
+       */
+      top = Math.min(top, bodyHeight - verticalSafeAreaApprox - contentHeight - bodyPadding);
+
       arrowTop = top + contentHeight;
       originY = 'bottom';
       addPopoverBottomClass = true;