From 5ea24bcd92949d33dbafecf48e950bd9405b7ba5 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 25 May 2022 15:28:22 +0000 Subject: [PATCH 1/6] test(popover): add failing test --- .../popover/test/adjustment/index.html | 140 ++++++++++++++++++ .../popover/test/adjustment/popover.e2e.ts | 32 ++++ 2 files changed, 172 insertions(+) create mode 100644 core/src/components/popover/test/adjustment/index.html create mode 100644 core/src/components/popover/test/adjustment/popover.e2e.ts diff --git a/core/src/components/popover/test/adjustment/index.html b/core/src/components/popover/test/adjustment/index.html new file mode 100644 index 00000000000..e9e3c0f7d26 --- /dev/null +++ b/core/src/components/popover/test/adjustment/index.html @@ -0,0 +1,140 @@ + + + + + Popover - Arrow + + + + + + + + + + + + +

+ Click everywhere to open the popover. +

+ + + + + + + diff --git a/core/src/components/popover/test/adjustment/popover.e2e.ts b/core/src/components/popover/test/adjustment/popover.e2e.ts new file mode 100644 index 00000000000..45a46c4cb50 --- /dev/null +++ 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); + }); +}); From a3b4a72b0d1f2232ac015bb313c0655acfc18327 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 25 May 2022 15:28:43 +0000 Subject: [PATCH 2/6] fix(popover): ensure popover does not go offscreen when adjusting top position --- core/src/components/popover/utils.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/core/src/components/popover/utils.ts b/core/src/components/popover/utils.ts index 3457379591c..564c28f0d6e 100644 --- a/core/src/components/popover/utils.ts +++ b/core/src/components/popover/utils.ts @@ -863,7 +863,18 @@ 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; From 46a6b464dc1dd10c797ccbfa46f62da58d40b44c Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 25 May 2022 15:29:45 +0000 Subject: [PATCH 3/6] chore(): lint --- .../popover/test/adjustment/index.html | 18 +++++++----------- .../popover/test/adjustment/popover.e2e.ts | 2 +- core/src/components/popover/utils.ts | 1 - 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/core/src/components/popover/test/adjustment/index.html b/core/src/components/popover/test/adjustment/index.html index e9e3c0f7d26..06bb0ff45db 100644 --- a/core/src/components/popover/test/adjustment/index.html +++ b/core/src/components/popover/test/adjustment/index.html @@ -2,7 +2,7 @@ - Popover - Arrow + Popover - Adjustment -

- Click everywhere to open the popover. -

+

Click everywhere to open the popover.

 - diff --git a/core/src/components/popover/test/adjustment/popover.e2e.ts b/core/src/components/popover/test/adjustment/popover.e2e.ts index 45a46c4cb50..77a0386cb32 100644 --- a/core/src/components/popover/test/adjustment/popover.e2e.ts +++ b/core/src/components/popover/test/adjustment/popover.e2e.ts @@ -15,7 +15,7 @@ test.describe('popover: adjustment', async () => { */ await page.setViewportSize({ width: 500, - height: 400 + height: 400, }); const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent'); diff --git a/core/src/components/popover/utils.ts b/core/src/components/popover/utils.ts index 564c28f0d6e..363db25c520 100644 --- a/core/src/components/popover/utils.ts +++ b/core/src/components/popover/utils.ts @@ -863,7 +863,6 @@ export const calculateWindowAdjustment = ( */ if (triggerTop + triggerHeight + contentHeight > bodyHeight && (side === 'top' || side === 'bottom')) { if (triggerTop - contentHeight > 0) { - /** * While we strive to align the popover with the trigger * on smaller screens this is not always possible. As a result, From 92ac0e4ba82d957f368183a8bf7e93fc90309dc5 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Thu, 26 May 2022 16:20:30 +0000 Subject: [PATCH 4/6] chore(): clean up --- .../popover/test/adjustment/index.html | 102 +++--------------- 1 file changed, 12 insertions(+), 90 deletions(-) diff --git a/core/src/components/popover/test/adjustment/index.html b/core/src/components/popover/test/adjustment/index.html index 06bb0ff45db..8367dfe89b7 100644 --- a/core/src/components/popover/test/adjustment/index.html +++ b/core/src/components/popover/test/adjustment/index.html @@ -16,52 +16,6 @@ import { popoverController } from '../../../../dist/ionic/index.esm.js'; window.popoverController = popoverController; - @@ -71,63 +25,31 @@