fix(popover): ensure popover does not go offscreen when adjusting top…

… position (#25350) resolves #25349
ionic-team · Jun 9, 2022 · 6926538 · 6926538
1 parent efe9e92
commit 6926538
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 1 deletion.
diff --git a/core/src/components/popover/test/adjustment/index.html b/core/src/components/popover/test/adjustment/index.html
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+  <head>
+    <meta charset="UTF-8" />
+    <title>Popover - Adjustment</title>
+    <meta
+      name="viewport"
+      content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no, viewport-fit=cover"
+    />
+    <link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
+    <link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
+    <script src="../../../../../scripts/testing/scripts.js"></script>
+    <script nomodule src="../../../../../dist/ionic/ionic.js"></script>
+    <script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
+    <script type="module">
+      import { popoverController } from '../../../../dist/ionic/index.esm.js';
+      window.popoverController = popoverController;
+    </script>
+  </head>
+  <body>
+    <ion-app>
+      <ion-content>
+        <p style="text-align: center">Click everywhere to open the popover.</p>
+      </ion-content>
+    </ion-app>
+
+    <script>
+      document.querySelector('ion-content').addEventListener('click', handleButtonClick);
+
+      async function handleButtonClick(ev) {
+        const popover = await popoverController.create({
+          component: 'popover-example-page',
+          event: ev,
+          reference: 'event',
+        });
+
+        popover.present();
+      }
+
+      customElements.define(
+        'popover-example-page',
+        class PopoverContent extends HTMLElement {
+          connectedCallback() {
+            this.innerHTML = `
+              <ion-list>
+                <ion-list-header>Ionic</ion-list-header>
+                <ion-item button>Learn Ionic</ion-item>
+                <ion-item button>Documentation</ion-item>
+                <ion-item button>Showcase</ion-item>
+                <ion-item button>GitHub Repo</ion-item>
+                <ion-item button>Another Item</ion-item>
+              </ion-list>
+            `;
+          }
+        }
+      );
+    </script>
+  </body>
+</html>
diff --git a/core/src/components/popover/test/adjustment/popover.e2e.ts b/core/src/components/popover/test/adjustment/popover.e2e.ts
@@ -0,0 +1,32 @@
+import { expect } from '@playwright/test';
+import { test } from '@utils/test/playwright';
+
+test.describe('popover: adjustment', async () => {
+  test('should not render the popover offscreen', async ({ page }) => {
+    await page.goto('/src/components/popover/test/adjustment');
+
+    /**
+     * We need to click in an area where
+     * there is not enough room to show the popover
+     * below the click coordinates but not enough
+     * room above the click coordinates that we
+     * can just move the popover to without it going
+     * offscreen.
+     */
+    await page.setViewportSize({
+      width: 500,
+      height: 400,
+    });
+
+    const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent');
+
+    await page.mouse.click(300, 300);
+
+    await ionPopoverDidPresent.next();
+
+    const popoverContent = page.locator('ion-popover .popover-content');
+    const box = (await popoverContent.boundingBox())!;
+
+    expect(box.y > 0).toBe(true);
+  });
+});
diff --git a/core/src/components/popover/utils.ts b/core/src/components/popover/utils.ts
@@ -863,7 +863,17 @@ export const calculateWindowAdjustment = (
    */
   if (triggerTop + triggerHeight + contentHeight > bodyHeight && (side === 'top' || side === 'bottom')) {
     if (triggerTop - contentHeight > 0) {
-      top = triggerTop - contentHeight - triggerHeight - (arrowHeight - 1);
+      /**
+       * While we strive to align the popover with the trigger
+       * on smaller screens this is not always possible. As a result,
+       * we adjust the popover up so that it does not hang
+       * off the bottom of the screen. However, we do not want to move
+       * the popover up so much that it goes off the top of the screen.
+       *
+       * We chose 12 here so that the popover position looks a bit nicer as
+       * it is not right up against the edge of the screen.
+       */
+      top = Math.max(12, triggerTop - contentHeight - triggerHeight - (arrowHeight - 1));
       arrowTop = top + contentHeight;
       originY = 'bottom';
       addPopoverBottomClass = true;