From 6d14cabb797c86926ec52414e48fed8015378c6b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 11:48:13 +0000 Subject: [PATCH 1/3] fix(popover): account for css zoom in positioning When a CSS `zoom` other than 1 applies to the popover, geometry APIs like `getBoundingClientRect()` and pointer `clientX`/`clientY` report values in the zoomed coordinate space, while the inline `top`/`left`/`--width` styles the popover sets are interpreted in the unzoomed layout space and re-scaled by the browser. Applying the zoom factor twice placed the popover in the wrong location and, with `size="cover"`, gave it the wrong width. Read the effective zoom from the popover's own context via `currentCSSZoom`, so a zoom applied anywhere above it is picked up and accumulated zoom across ancestors is handled, falling back to the ratio between the bounding rect and `offsetWidth` where that property is unavailable. Normalize every rect-derived measurement by it: the trigger and content rects, the arrow dimensions, the `size="cover"` width, and the pointer coordinates used by `reference="event"`. `innerWidth`/`innerHeight` are not affected by CSS `zoom`, so scale them into the same space as well. Otherwise the offscreen adjustment clamps against a viewport larger than the space actually available and the popover can render past the edge of the screen. closes #30919 Co-authored-by: KanhaiyaPandey Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014em3LPxMPQRPufMRz5i7so --- .../popover/animations/ios.enter.ts | 29 ++- .../components/popover/animations/md.enter.ts | 34 ++- core/src/components/popover/test/util.spec.ts | 87 +++++++- .../components/popover/test/zoom/index.html | 76 +++++++ .../popover/test/zoom/popover.e2e.ts | 193 ++++++++++++++++++ core/src/components/popover/utils.ts | 82 ++++++-- 6 files changed, 475 insertions(+), 26 deletions(-) create mode 100644 core/src/components/popover/test/zoom/index.html create mode 100644 core/src/components/popover/test/zoom/popover.e2e.ts diff --git a/core/src/components/popover/animations/ios.enter.ts b/core/src/components/popover/animations/ios.enter.ts index 02a078a2f71..4273074f6bb 100644 --- a/core/src/components/popover/animations/ios.enter.ts +++ b/core/src/components/popover/animations/ios.enter.ts @@ -5,6 +5,7 @@ import type { Animation } from '../../../interface'; import { calculateWindowAdjustment, getArrowDimensions, + getElementCSSZoom, getPopoverDimensions, getPopoverPosition, getSafeAreaInsets, @@ -31,16 +32,31 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation => const { event: ev, size, trigger, reference, side, align } = opts; const doc = baseEl.ownerDocument as any; const isRTL = doc.dir === 'rtl'; - const bodyWidth = doc.defaultView.innerWidth; - const bodyHeight = doc.defaultView.innerHeight; - const root = getElementRoot(baseEl); const contentEl = root.querySelector('.popover-content') as HTMLElement; const arrowEl = root.querySelector('.popover-arrow') as HTMLElement | null; + /** + * A CSS `zoom` other than 1 on an ancestor (e.g. the `html` element) causes + * geometry APIs like `getBoundingClientRect()` to report zoomed values while + * inline `top`/`left`/`--width` styles are interpreted in the unzoomed layout + * space. Normalize all rect-derived measurements by this factor so the + * popover is positioned and sized correctly. + */ + const zoom = getElementCSSZoom(contentEl); + + /** + * `innerWidth`/`innerHeight` are not affected by CSS `zoom`, so they must be + * scaled down to the same layout space as the normalized measurements above. + * Otherwise the popover would be clamped against a viewport that is larger + * than the space actually available to it. + */ + const bodyWidth = doc.defaultView.innerWidth / zoom; + const bodyHeight = doc.defaultView.innerHeight / zoom; + const referenceSizeEl = trigger || ev?.detail?.ionShadowTarget || ev?.target; - const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl); - const { arrowWidth, arrowHeight } = getArrowDimensions(arrowEl); + const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl, zoom); + const { arrowWidth, arrowHeight } = getArrowDimensions(arrowEl, zoom); const defaultPosition = { top: bodyHeight / 2 - contentHeight / 2, @@ -60,7 +76,8 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation => align, defaultPosition, trigger, - ev + ev, + zoom ); const padding = size === 'cover' ? 0 : POPOVER_IOS_BODY_PADDING; diff --git a/core/src/components/popover/animations/md.enter.ts b/core/src/components/popover/animations/md.enter.ts index 8de9976e86c..6d37474ceb2 100644 --- a/core/src/components/popover/animations/md.enter.ts +++ b/core/src/components/popover/animations/md.enter.ts @@ -2,7 +2,13 @@ import { createAnimation } from '@utils/animation/animation'; import { getElementRoot } from '@utils/helpers'; import type { Animation } from '../../../interface'; -import { calculateWindowAdjustment, getPopoverDimensions, getPopoverPosition, getSafeAreaInsets } from '../utils'; +import { + calculateWindowAdjustment, + getElementCSSZoom, + getPopoverDimensions, + getPopoverPosition, + getSafeAreaInsets, +} from '../utils'; const POPOVER_MD_BODY_PADDING = 12; @@ -15,14 +21,29 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation => const doc = baseEl.ownerDocument as any; const isRTL = doc.dir === 'rtl'; - const bodyWidth = doc.defaultView.innerWidth; - const bodyHeight = doc.defaultView.innerHeight; - const root = getElementRoot(baseEl); const contentEl = root.querySelector('.popover-content') as HTMLElement; + /** + * A CSS `zoom` other than 1 on an ancestor (e.g. the `html` element) causes + * geometry APIs like `getBoundingClientRect()` to report zoomed values while + * inline `top`/`left`/`--width` styles are interpreted in the unzoomed layout + * space. Normalize all rect-derived measurements by this factor so the + * popover is positioned and sized correctly. + */ + const zoom = getElementCSSZoom(contentEl); + + /** + * `innerWidth`/`innerHeight` are not affected by CSS `zoom`, so they must be + * scaled down to the same layout space as the normalized measurements above. + * Otherwise the popover would be clamped against a viewport that is larger + * than the space actually available to it. + */ + const bodyWidth = doc.defaultView.innerWidth / zoom; + const bodyHeight = doc.defaultView.innerHeight / zoom; + const referenceSizeEl = trigger || ev?.detail?.ionShadowTarget || ev?.target; - const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl); + const { contentWidth, contentHeight } = getPopoverDimensions(size, contentEl, referenceSizeEl, zoom); const defaultPosition = { top: bodyHeight / 2 - contentHeight / 2, @@ -42,7 +63,8 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation => align, defaultPosition, trigger, - ev + ev, + zoom ); const padding = size === 'cover' ? 0 : POPOVER_MD_BODY_PADDING; diff --git a/core/src/components/popover/test/util.spec.ts b/core/src/components/popover/test/util.spec.ts index a383209f96c..cda094f39c6 100644 --- a/core/src/components/popover/test/util.spec.ts +++ b/core/src/components/popover/test/util.spec.ts @@ -1,4 +1,89 @@ -import { isTriggerElement, getIndexOfItem, getNextItem, getPrevItem } from '../utils'; +import { + isTriggerElement, + getIndexOfItem, + getNextItem, + getPrevItem, + getElementCSSZoom, + getPopoverDimensions, + getArrowDimensions, +} from '../utils'; + +describe('getElementCSSZoom', () => { + it('should return 1 when no element is provided', () => { + expect(getElementCSSZoom(null)).toEqual(1); + }); + + it('should use currentCSSZoom when available', () => { + const el = document.createElement('div'); + Object.defineProperty(el, 'currentCSSZoom', { value: 1.5, configurable: true }); + + expect(getElementCSSZoom(el)).toEqual(1.5); + }); + + it('should fall back to the ratio between the client rect and offsetWidth', () => { + const el = document.createElement('div'); + // No currentCSSZoom support in this environment. + el.getBoundingClientRect = () => ({ width: 300, height: 0, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + Object.defineProperty(el, 'offsetWidth', { value: 200, configurable: true }); + + expect(getElementCSSZoom(el)).toEqual(1.5); + }); + + it('should treat sub-pixel rounding in the fallback as no zoom', () => { + const el = document.createElement('div'); + // offsetWidth is rounded to an integer, the bounding rect is not. + el.getBoundingClientRect = () => ({ width: 250.4, height: 0, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + Object.defineProperty(el, 'offsetWidth', { value: 250, configurable: true }); + + expect(getElementCSSZoom(el)).toEqual(1); + }); + + it('should return 1 when the fallback measurements are unavailable', () => { + const el = document.createElement('div'); + el.getBoundingClientRect = () => ({ width: 0, height: 0, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + Object.defineProperty(el, 'offsetWidth', { value: 0, configurable: true }); + + expect(getElementCSSZoom(el)).toEqual(1); + }); +}); + +describe('getPopoverDimensions', () => { + it('should normalize the content dimensions by the zoom factor', () => { + const contentEl = document.createElement('div'); + contentEl.getBoundingClientRect = () => + ({ width: 300, height: 450, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + + const { contentWidth, contentHeight } = getPopoverDimensions('auto', contentEl, undefined, 1.5); + + expect(contentWidth).toEqual(200); + expect(contentHeight).toEqual(300); + }); + + it('should normalize the trigger width by the zoom factor when size is cover', () => { + const contentEl = document.createElement('div'); + contentEl.getBoundingClientRect = () => + ({ width: 300, height: 450, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + const triggerEl = document.createElement('div'); + triggerEl.getBoundingClientRect = () => + ({ width: 150, height: 60, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + + const { contentWidth } = getPopoverDimensions('cover', contentEl, triggerEl, 1.5); + + expect(contentWidth).toEqual(100); + }); +}); + +describe('getArrowDimensions', () => { + it('should normalize the arrow dimensions by the zoom factor', () => { + const arrowEl = document.createElement('div'); + arrowEl.getBoundingClientRect = () => ({ width: 15, height: 15, top: 0, left: 0, bottom: 0, right: 0 } as DOMRect); + + const { arrowWidth, arrowHeight } = getArrowDimensions(arrowEl, 1.5); + + expect(arrowWidth).toEqual(10); + expect(arrowHeight).toEqual(10); + }); +}); describe('isTriggerElement', () => { it('should return true is element is a trigger', () => { diff --git a/core/src/components/popover/test/zoom/index.html b/core/src/components/popover/test/zoom/index.html new file mode 100644 index 00000000000..96a7488768d --- /dev/null +++ b/core/src/components/popover/test/zoom/index.html @@ -0,0 +1,76 @@ + + + + + Popover - Zoom + + + + + + + + + + + + + + Popover - Zoom + + + + + + + Auto + + + + + Cover + + + + + Edge + + + + + diff --git a/core/src/components/popover/test/zoom/popover.e2e.ts b/core/src/components/popover/test/zoom/popover.e2e.ts new file mode 100644 index 00000000000..dc73039e31a --- /dev/null +++ b/core/src/components/popover/test/zoom/popover.e2e.ts @@ -0,0 +1,193 @@ +import { expect } from '@playwright/test'; +import type { E2EPage } from '@utils/test/playwright'; +import { configs, test } from '@utils/test/playwright'; + +import { openPopover } from '../test.utils'; + +/** + * A CSS `zoom` causes geometry APIs such as `getBoundingClientRect()` and + * pointer `clientX`/`clientY` to report values in the zoomed coordinate space, + * while the inline `top`/`left`/`--width` styles the popover sets are + * interpreted in the unzoomed layout space. The popover needs to account for + * this so it stays anchored to its trigger. + * + * These are functional assertions rather than screenshots because what is being + * verified is the popover's geometry relative to its trigger, not its + * appearance. Both boxes are read in the same coordinate space, so the + * relationship between them holds at any zoom level. + */ + +/** + * Maximum difference, in pixels, between two positions still considered + * aligned. Generous enough for sub-pixel rounding across browsers, far tighter + * than the error a missing zoom adjustment produces (tens of pixels). + */ +const TOLERANCE = 2; + +const expectAligned = (actual: number, expected: number) => { + expect(Math.abs(actual - expected)).toBeLessThanOrEqual(TOLERANCE); +}; + +/** + * Builds a page with a trigger and a popover, with `zoomStyles` controlling + * where in the tree the zoom is applied. The trigger is kept near the top left + * so the popover is never pushed onto the screen by the offscreen adjustment, + * which would mask a positioning error. + */ +const zoomedPage = (zoomStyles: string) => ` + + + + + Content + +`; + +const expectAnchoredToTrigger = async (page: E2EPage) => { + const triggerBox = (await page.locator('#trigger').boundingBox())!; + const contentBox = (await page.locator('ion-popover').locator('.popover-content').boundingBox())!; + + expectAligned(contentBox.x, triggerBox.x); + expectAligned(contentBox.y, triggerBox.y + triggerBox.height); +}; + +/** + * This behavior does not vary across directions. MD mode is used because it has + * no arrow offsetting the content and defaults to `start` alignment, which + * makes the expected relationship to the trigger unambiguous. + */ +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('popover: zoom'), () => { + test.beforeEach(() => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30919', + }); + }); + + test.describe('zoom on the html element', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/src/components/popover/test/zoom', config); + }); + + test('should align the popover with its trigger', async ({ page }) => { + await openPopover(page, 'auto-trigger'); + + const triggerBox = (await page.locator('#auto-trigger').boundingBox())!; + const contentBox = (await page.locator('ion-popover.auto-popover').locator('.popover-content').boundingBox())!; + + expectAligned(contentBox.x, triggerBox.x); + expectAligned(contentBox.y, triggerBox.y + triggerBox.height); + }); + + test('should not render the popover offscreen', async ({ page }) => { + await openPopover(page, 'edge-trigger'); + + const viewport = page.viewportSize()!; + const contentBox = (await page.locator('ion-popover.edge-popover').locator('.popover-content').boundingBox())!; + + expect(contentBox.x).toBeGreaterThanOrEqual(0); + expect(contentBox.x + contentBox.width).toBeLessThanOrEqual(viewport.width); + }); + + test('should match the trigger width when size is cover', async ({ page }) => { + await openPopover(page, 'cover-trigger'); + + const triggerBox = (await page.locator('#cover-trigger').boundingBox())!; + const contentBox = (await page.locator('ion-popover.cover-popover').locator('.popover-content').boundingBox())!; + + expectAligned(contentBox.width, triggerBox.width); + }); + }); + + /** + * The zoom must be read from the popover's own context rather than from + * `document.documentElement`, otherwise a zoom applied lower in the tree is + * missed entirely. + */ + test.describe('zoom applied at other levels of the tree', () => { + test('should align the popover when zoom is on the body', async ({ page }) => { + await page.setContent(zoomedPage('body { zoom: 1.5; }'), config); + await openPopover(page, 'trigger'); + + await expectAnchoredToTrigger(page); + }); + + test('should align the popover when zoom accumulates across ancestors', async ({ page }) => { + await page.setContent(zoomedPage('html { zoom: 1.2; } body { zoom: 1.25; }'), config); + await openPopover(page, 'trigger'); + + await expectAnchoredToTrigger(page); + }); + + test('should align the popover when the page is zoomed out', async ({ page }) => { + await page.setContent(zoomedPage('html { zoom: 0.8; }'), config); + await openPopover(page, 'trigger'); + + await expectAnchoredToTrigger(page); + }); + }); + + /** + * `reference="event"` positions the popover from the pointer coordinates of + * the event, which are reported in the zoomed coordinate space too. + */ + test.describe('pointer coordinates', () => { + test('should position the popover at the pointer when reference is event', async ({ page }) => { + await page.setContent( + zoomedPage('html { zoom: 1.5; }').replace('trigger="trigger"', 'trigger="trigger" reference="event"'), + config + ); + + const triggerBox = (await page.locator('#trigger').boundingBox())!; + await openPopover(page, 'trigger'); + + const contentBox = (await page.locator('ion-popover').locator('.popover-content').boundingBox())!; + + /** + * Playwright clicks the centre of the trigger, which is where the + * popover should be anchored. + */ + expectAligned(contentBox.x, triggerBox.x + triggerBox.width / 2); + expectAligned(contentBox.y, triggerBox.y + triggerBox.height / 2); + }); + }); + }); +}); + +/** + * The arrow only exists in ios mode. + */ +configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('popover: zoom'), () => { + test('should centre the arrow on the trigger when a zoom is applied', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30919', + }); + + await page.setContent(zoomedPage('html { zoom: 1.5; }'), config); + await openPopover(page, 'trigger'); + + const triggerBox = (await page.locator('#trigger').boundingBox())!; + const arrowBox = (await page.locator('ion-popover').locator('.popover-arrow').boundingBox())!; + + expectAligned(arrowBox.x + arrowBox.width / 2, triggerBox.x + triggerBox.width / 2); + }); + }); +}); diff --git a/core/src/components/popover/utils.ts b/core/src/components/popover/utils.ts index 0d11a4dfeef..8a257c4809d 100644 --- a/core/src/components/popover/utils.ts +++ b/core/src/components/popover/utils.ts @@ -105,18 +105,73 @@ export const getSafeAreaInsets = (doc: Document): SafeAreaInsets => { return insets; }; +/** + * Largest difference from 1 that the `offsetWidth` based zoom detection below + * attributes to integer rounding rather than to an actual CSS `zoom`. The + * rounding error is at most half a pixel over the width of the popover, which + * is well under this threshold for any realistic popover size. + */ +const ZOOM_ROUNDING_TOLERANCE = 0.01; + +/** + * Returns the cumulative CSS `zoom` factor applied to an element. + * + * When a CSS `zoom` other than 1 is set on an ancestor (e.g. the `html` + * element, as recommended by the docs for dynamic font scaling on Chrome for + * Android), `getBoundingClientRect()`, `clientX`/`clientY` and other geometry + * APIs report values in the *zoomed* (visual) coordinate space, while inline + * `top`/`left`/`--width` styles we set are interpreted in the *unzoomed* + * (layout) space and re-scaled by the browser. Dividing the rect-derived + * values by this factor converts them back to layout space so the popover is + * positioned and sized correctly. Returns 1 when no zoom is applied. + */ +export const getElementCSSZoom = (el: HTMLElement | null): number => { + if (!el) { + return 1; + } + + /** + * `currentCSSZoom` exposes the exact effective zoom of an element + * (Chromium 126+). When available we use it directly. + */ + const currentCSSZoom = (el as unknown as { currentCSSZoom?: number }).currentCSSZoom; + if (typeof currentCSSZoom === 'number' && currentCSSZoom > 0) { + return currentCSSZoom; + } + + /** + * Fallback for browsers without `currentCSSZoom`: compare the rendered + * (zoomed) width from `getBoundingClientRect()` against the layout width + * from `offsetWidth`, which is not affected by CSS `zoom`. + */ + const { width } = el.getBoundingClientRect(); + const { offsetWidth } = el; + if (offsetWidth > 0 && width > 0) { + const ratio = width / offsetWidth; + /** + * `offsetWidth` is rounded to an integer while the bounding rect is not, + * so the ratio is rarely exactly 1 even when no zoom is applied. Treat + * sub-pixel differences as "no zoom" so that unzoomed popovers are not + * shifted by the rounding error. A real zoom deviates far more than this. + */ + return Math.abs(ratio - 1) < ZOOM_ROUNDING_TOLERANCE ? 1 : ratio; + } + + return 1; +}; + /** * Returns the dimensions of the popover * arrow on `ios` mode. If arrow is disabled * returns (0, 0). */ -export const getArrowDimensions = (arrowEl: HTMLElement | null) => { +export const getArrowDimensions = (arrowEl: HTMLElement | null, zoom = 1) => { if (!arrowEl) { return { arrowWidth: 0, arrowHeight: 0 }; } const { width, height } = arrowEl.getBoundingClientRect(); - return { arrowWidth: width, arrowHeight: height }; + return { arrowWidth: width / zoom, arrowHeight: height / zoom }; }; /** @@ -124,14 +179,14 @@ export const getArrowDimensions = (arrowEl: HTMLElement | null) => { * that takes into account whether or not the width * should match the trigger width. */ -export const getPopoverDimensions = (size: PopoverSize, contentEl: HTMLElement, triggerEl?: HTMLElement) => { +export const getPopoverDimensions = (size: PopoverSize, contentEl: HTMLElement, triggerEl?: HTMLElement, zoom = 1) => { const contentDimentions = contentEl.getBoundingClientRect(); - const contentHeight = contentDimentions.height; - let contentWidth = contentDimentions.width; + const contentHeight = contentDimentions.height / zoom; + let contentWidth = contentDimentions.width / zoom; if (size === 'cover' && triggerEl) { const triggerDimensions = triggerEl.getBoundingClientRect(); - contentWidth = triggerDimensions.width; + contentWidth = triggerDimensions.width / zoom; } return { @@ -526,7 +581,8 @@ export const getPopoverPosition = ( align: PositionAlign, defaultPosition: PopoverPosition, triggerEl?: HTMLElement, - event?: MouseEvent | CustomEvent + event?: MouseEvent | CustomEvent, + zoom = 1 ): PopoverPosition => { let referenceCoordinates = { top: 0, @@ -549,8 +605,8 @@ export const getPopoverPosition = ( const mouseEv = event as MouseEvent; referenceCoordinates = { - top: mouseEv.clientY, - left: mouseEv.clientX, + top: mouseEv.clientY / zoom, + left: mouseEv.clientX / zoom, width: 1, height: 1, }; @@ -585,10 +641,10 @@ export const getPopoverPosition = ( } const triggerBoundingBox = actualTriggerEl.getBoundingClientRect(); referenceCoordinates = { - top: triggerBoundingBox.top, - left: triggerBoundingBox.left, - width: triggerBoundingBox.width, - height: triggerBoundingBox.height, + top: triggerBoundingBox.top / zoom, + left: triggerBoundingBox.left / zoom, + width: triggerBoundingBox.width / zoom, + height: triggerBoundingBox.height / zoom, }; break; From 1fe79438c569dd7cb5adb94a76b59a649ef245db Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 19:29:20 +0000 Subject: [PATCH 2/3] fix(popover): address css zoom review feedback - Correct the `currentCSSZoom` availability note: the property landed in Chromium 128, not 126. - Note in the fallback comment that the detected factor stays approximate under a real zoom. The rounding tolerance only snaps the unzoomed case to exactly 1, so nothing bounds the error once a zoom is present. - Drop the cast on `currentCSSZoom`, which `lib.dom.d.ts` already declares on `Element`. The `typeof` guard stays: WebKit has not shipped the property and returns `undefined` at runtime despite the type. - Add a case where the zoom wraps only the trigger, leaving the popover outside the zoomed subtree. That is the split `popoverController.create()` produces by default, and it holds because the factor is read from the popover rather than from `document.documentElement`. - Cover every side in the arrow tests instead of only the default `bottom`, asserting horizontal centring on `top`/`bottom` and vertical centring on `left`/`right`, where the arrow is rotated. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014em3LPxMPQRPufMRz5i7so --- .../popover/test/zoom/popover.e2e.ts | 115 ++++++++++++++++-- core/src/components/popover/utils.ts | 7 +- 2 files changed, 110 insertions(+), 12 deletions(-) diff --git a/core/src/components/popover/test/zoom/popover.e2e.ts b/core/src/components/popover/test/zoom/popover.e2e.ts index dc73039e31a..c772777fd85 100644 --- a/core/src/components/popover/test/zoom/popover.e2e.ts +++ b/core/src/components/popover/test/zoom/popover.e2e.ts @@ -58,6 +58,70 @@ const zoomedPage = (zoomStyles: string) => ` `; +/** + * Markup where the zoom wraps only the trigger, leaving the popover outside the + * zoomed subtree. This is the split `popoverController.create()` produces by + * default, since the overlay is appended to `ion-app`. + */ +const triggerOnlyZoomPage = ` + + +
+ +
+ + Content + +`; + +/** + * Builds a page with the popover on a given side, under a zoom. The trigger sits + * in the middle so the popover fits on every side without the offscreen + * adjustment moving it, which would mask an arrow positioning error. + */ +const zoomedSidePage = (side: string) => ` + + + + + Content + +`; + const expectAnchoredToTrigger = async (page: E2EPage) => { const triggerBox = (await page.locator('#trigger').boundingBox())!; const contentBox = (await page.locator('ion-popover').locator('.popover-content').boundingBox())!; @@ -135,6 +199,13 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { await expectAnchoredToTrigger(page); }); + test('should align the popover when the zoom wraps only the trigger', async ({ page }) => { + await page.setContent(triggerOnlyZoomPage, config); + await openPopover(page, 'trigger'); + + await expectAnchoredToTrigger(page); + }); + test('should align the popover when the page is zoomed out', async ({ page }) => { await page.setContent(zoomedPage('html { zoom: 0.8; }'), config); await openPopover(page, 'trigger'); @@ -160,7 +231,7 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { const contentBox = (await page.locator('ion-popover').locator('.popover-content').boundingBox())!; /** - * Playwright clicks the centre of the trigger, which is where the + * Playwright clicks the center of the trigger, which is where the * popover should be anchored. */ expectAligned(contentBox.x, triggerBox.x + triggerBox.width / 2); @@ -171,23 +242,49 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { }); /** - * The arrow only exists in ios mode. + * The arrow only exists in ios mode. `calculateArrowPosition` branches per side + * and every branch now runs on zoom-normalized dimensions, so each side needs + * its own coverage. */ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { test.describe(title('popover: zoom'), () => { - test('should centre the arrow on the trigger when a zoom is applied', async ({ page }) => { + test.beforeEach(() => { test.info().annotations.push({ type: 'issue', description: 'https://github.com/ionic-team/ionic-framework/issues/30919', }); + }); - await page.setContent(zoomedPage('html { zoom: 1.5; }'), config); - await openPopover(page, 'trigger'); + /** + * On the vertical sides the arrow sits above or below the content and is + * centered horizontally on the trigger. + */ + for (const side of ['top', 'bottom']) { + test(`should center the arrow on the trigger when side is ${side}`, async ({ page }) => { + await page.setContent(zoomedSidePage(side), config); + await openPopover(page, 'trigger'); - const triggerBox = (await page.locator('#trigger').boundingBox())!; - const arrowBox = (await page.locator('ion-popover').locator('.popover-arrow').boundingBox())!; + const triggerBox = (await page.locator('#trigger').boundingBox())!; + const arrowBox = (await page.locator('ion-popover').locator('.popover-arrow').boundingBox())!; - expectAligned(arrowBox.x + arrowBox.width / 2, triggerBox.x + triggerBox.width / 2); - }); + expectAligned(arrowBox.x + arrowBox.width / 2, triggerBox.x + triggerBox.width / 2); + }); + } + + /** + * On the horizontal sides the arrow is rotated to point sideways and is + * centered vertically on the trigger instead. + */ + for (const side of ['left', 'right']) { + test(`should center the arrow on the trigger when side is ${side}`, async ({ page }) => { + await page.setContent(zoomedSidePage(side), config); + await openPopover(page, 'trigger'); + + const triggerBox = (await page.locator('#trigger').boundingBox())!; + const arrowBox = (await page.locator('ion-popover').locator('.popover-arrow').boundingBox())!; + + expectAligned(arrowBox.y + arrowBox.height / 2, triggerBox.y + triggerBox.height / 2); + }); + } }); }); diff --git a/core/src/components/popover/utils.ts b/core/src/components/popover/utils.ts index 8a257c4809d..5c52d7d9216 100644 --- a/core/src/components/popover/utils.ts +++ b/core/src/components/popover/utils.ts @@ -132,9 +132,9 @@ export const getElementCSSZoom = (el: HTMLElement | null): number => { /** * `currentCSSZoom` exposes the exact effective zoom of an element - * (Chromium 126+). When available we use it directly. + * (Chromium 128+). When available we use it directly. */ - const currentCSSZoom = (el as unknown as { currentCSSZoom?: number }).currentCSSZoom; + const currentCSSZoom = el.currentCSSZoom; if (typeof currentCSSZoom === 'number' && currentCSSZoom > 0) { return currentCSSZoom; } @@ -152,7 +152,8 @@ export const getElementCSSZoom = (el: HTMLElement | null): number => { * `offsetWidth` is rounded to an integer while the bounding rect is not, * so the ratio is rarely exactly 1 even when no zoom is applied. Treat * sub-pixel differences as "no zoom" so that unzoomed popovers are not - * shifted by the rounding error. A real zoom deviates far more than this. + * shifted by the rounding error. A real zoom deviates far more, though + * the same rounding leaves the detected factor approximate. */ return Math.abs(ratio - 1) < ZOOM_ROUNDING_TOLERANCE ? 1 : ratio; } From b5f167b0bc4b371ef2f4615945854521fdfc6cae Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 13:48:01 +0000 Subject: [PATCH 3/3] test(popover): cover arrow height normalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The arrow tests asserted only the axis the centring runs along, so for `bottom` nothing checked where the arrow sits vertically — the one place `arrowHeight` feeds into the rendered result. Removing the normalization left the e2e suite green. Assert that the arrow sits flush against the content edge it points away from, alongside the existing centring check, and raise the zoom on the vertical sides so the gap a missing normalization opens clears the tolerance: 7.5px against a 2px bound, where 1.25 gave 3.1px. The horizontal sides stay at the lower zoom. At a larger one the popover no longer fits beside the trigger and the offscreen adjustment moves it, which would mask the arrow position. The vertical sides cannot go much higher either: at 2 there is too little layout height below the trigger and the popover flips above it, changing which edge the arrow sits against. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014em3LPxMPQRPufMRz5i7so --- .../popover/test/zoom/popover.e2e.ts | 39 +++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/core/src/components/popover/test/zoom/popover.e2e.ts b/core/src/components/popover/test/zoom/popover.e2e.ts index c772777fd85..beed5faa776 100644 --- a/core/src/components/popover/test/zoom/popover.e2e.ts +++ b/core/src/components/popover/test/zoom/popover.e2e.ts @@ -91,15 +91,31 @@ const triggerOnlyZoomPage = ` `; +/** + * The vertical sides take a larger zoom. The gap a missing `arrowHeight` + * normalization opens between the arrow and the content edge scales with it, + * so a larger factor keeps that gap comfortably clear of the tolerance. Going + * much beyond this leaves too little layout height below the trigger and the + * popover flips above it, which changes which edge the arrow sits against. + */ +const VERTICAL_SIDE_ZOOM = 1.5; + +/** + * The horizontal sides need a smaller one: at a larger zoom the popover no + * longer fits beside the trigger, and the offscreen adjustment would move it + * and mask the arrow position under test. + */ +const HORIZONTAL_SIDE_ZOOM = 1.25; + /** * Builds a page with the popover on a given side, under a zoom. The trigger sits * in the middle so the popover fits on every side without the offscreen * adjustment moving it, which would mask an arrow positioning error. */ -const zoomedSidePage = (side: string) => ` +const zoomedSidePage = (side: string, zoom: number) => `