Skip to content

fix(popover): correct positioning and sizing when html zoom is applied - #31047

Closed
KanhaiyaPandey wants to merge 1 commit into
ionic-team:mainfrom
KanhaiyaPandey:fix/popover-zoom-positioning
Closed

KanhaiyaPandey wants to merge 1 commit into
ionic-team:mainfrom
KanhaiyaPandey:fix/popover-zoom-positioning

Conversation

@KanhaiyaPandey

Copy link
Copy Markdown
Contributor

🐛 Issue #30919

When CSS zoom is applied on the html element (e.g. zoom: 1.5), the popover is rendered in an incorrect position.
Additionally, size="cover" results in incorrect sizing.


✅ Expected Behavior

Popover should be correctly positioned and sized regardless of the document zoom level.


🔧 Fix

  • Normalized DOMRect values and pointer coordinates based on the document zoom factor.
  • Ensures consistent calculations for positioning and sizing across zoom levels.

Files updated

  • core/src/components/popover/utils.ts
  • md.enter.ts
  • ios.enter.ts

🧪 Tests

  • Added E2E regression test for:
    • Popover positioning
    • size="cover" behavior under html { zoom: 1.5 }
  • Test file:
    • core/src/components/popover/test/zoom/popover.e2e.ts
  • Firefox is skipped since CSS zoom is not supported there.

▶️ How to verify

cd core
PLAYWRIGHT_TEST_BASE_URL=http://localhost:3333 \
npx playwright test src/components/popover/test/zoom/popover.e2e.ts --project="Mobile Chrome"

@KanhaiyaPandey
KanhaiyaPandey requested a review from a team as a code owner March 30, 2026 13:28
@KanhaiyaPandey
KanhaiyaPandey requested a review from ShaneK March 30, 2026 13:28
@vercel

vercel Bot commented Mar 30, 2026

Copy link
Copy Markdown

@KanhaiyaPandey is attempting to deploy a commit to the Ionic Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added the package: core @ionic/core package label Mar 30, 2026
@NyaomiDEV

Copy link
Copy Markdown

Actually, Firefox now supports CSS zoom.

@thetaPC thetaPC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! This is an important fix. I've researched how other positioning libraries handle CSS zoom and I have some questions about the current approach.

Current Approach - Good Start ✅

You're correctly:

  • Reading the zoom CSS property via getComputedStyle()
  • Including a fallback for older browsers
  • Applying the zoom factor to positioning

Questions/Gaps to Address

1. What if zoom is applied at different DOM levels?

Your implementation checks document.documentElement.zoom. But what if a developer applies zoom at a parent level instead? How does the fix handle accumulated zoom across multiple ancestors? Have you tested zoom at different levels in the ancestor chain?

2. Where are you reading the zoom from?

Are you getting zoom from documentElement, or from the popover element's own context? These could be different. Which one is correct for positioning the popover?

3. Does size="cover" work?

The issue specifically mentions size="cover" breaks with zoom. Did you verify that the sizing calculations (not just positioning) account for zoom? How does the width/height calculation change with zoom applied?

4. Are pointer coordinates handled?

If the popover uses pointer events (touch/mouse), are those coordinate adjustments also accounted for? Or only DOMRect positioning?

5. Arrow positioning

Does the popover have a separate arrow element? If so, is its positioning also adjusted for zoom?


Edge Cases to Test

Before marking this ready, please verify:

  • Zoom at documentElement level
  • Zoom at intermediate parent level
  • Zoom at multiple levels (accumulated)
  • Popover with size="cover" + zoom
  • Arrow alignment with zoom
  • Pointer-based interactions with zoom

Research Reference

I'd suggest looking at how Floating UI solved this (PR #3492) for comparison. They handle zoom differently in some key ways that might be relevant.

@caspinos

caspinos commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Hi — I'm the reporter of #30919, thanks for picking this up.

I've been testing this area against main (v9.0.1) and ran into an edge case that I think is worth flagging, because the two zoom tests here wouldn't catch it.

The offscreen adjustment isn't zoom-aware

In md.enter.ts (and likewise ios.enter.ts), everything handed to calculateWindowAdjustment() is zoom-normalized — results.top/results.left, contentWidth/contentHeight, safeArea — except bodyWidth/bodyHeight, which come straight from innerWidth/innerHeight. So the clamp compares normalized coordinates against a viewport measured in a different space: it fires later than it should, and when it does fire it clamps to the wrong edge.

I think the premise behind it is the comment in utils.ts saying innerWidth/innerHeight "remain in the unscaled coordinate space". From what I can measure it's the other way round: at a 393px viewport with zoom: 1.5, innerWidth stays 393 — identical to zoom 1 — and lines up with getBoundingClientRect(), while the actual unscaled layout width is 262 (getComputedStyle(html).width, body.clientWidth). So dividing the rects by the zoom factor moves them out of innerWidth's space rather than into it.

Repro: MD mode, Pixel 5 viewport, html { zoom: 1.5 }, popover --width: 120px, trigger near the right edge of the 262px layout viewport. The popover's right edge lands at 450px on a 393px viewport — 57px offscreen. (iOS mode: 405px.) Dividing innerWidth/innerHeight by the zoom factor clamps it back on screen.

What makes this easy to miss: alignment-to-trigger and size="cover" width both still pass in that state, so a suite covering those two stays green while this is broken. That's exactly what happened in my own first attempt at this fix.

For transparency: the code path above is from this PR's head, but I confirmed the behaviour on an equivalent local implementation rather than by running this branch.

On the review feedback

While testing I ended up with working code for three of @thetaPC's points:

  • Zoom at different DOM levels — using currentCSSZoom, which returns the cumulative effective zoom for the element, rather than reading zoom off documentElement (that only reports the value set on that one element, so it misses zoom on an intermediate ancestor — with nested 1.5 → 2 → 1.25, currentCSSZoom gives 3.75 where getComputedStyle().zoom gives 1.25).
  • Pointer coordinatesclientX/clientY normalized for reference="event".
  • Arrow positioning — arrow dimensions normalized alongside the rest.

Plus unit tests for the zoom detection and an E2E case covering the offscreen behaviour above.

@KanhaiyaPandey happy to hand any of that over if it's useful — a PR against your branch, or just the diff in a comment, whichever you prefer. Not trying to duplicate your work, I'd just like to see this fixed.

@thetaPC

thetaPC commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@caspinos thanks for digging into this. The offscreen clamp finding is a good catch, and the items you listed (cumulative zoom, pointer normalization for reference="event", arrow positioning, tests) are the right scope.

@KanhaiyaPandey hasn't been active here in a while, so rather than wait longer: please open a new PR with your work and we'll review it there.

Two asks for the new PR:

If a PR isn't up by September 8, I'll open one on our side so this doesn't sit any longer.

We'll close this one out once a replacement is open.

@KanhaiyaPandey

Copy link
Copy Markdown
Contributor Author

Thanks @thetaPC and @caspinos for digging into this and for the detailed reproduction. The coordinate-space issue with innerWidth/innerHeight makes sense, and I agree the offscreen case needs to be handled separately.

pull Bot pushed a commit to tangzixuan/ionic-framework that referenced this pull request Sep 18, 2026
…eam#31426)

Issue number: resolves ionic-team#30919

---------

Supersedes ionic-team#31047, which this builds on. @KanhaiyaPandey is credited as
co-author on the commit.

## What is the current behavior?

When a CSS `zoom` other than `1` applies to the popover, `ion-popover`
renders incorrectly: it is positioned away from its trigger, and with
`size="cover"` it is given the wrong width. This affects a documented
workflow — adjusting the `html` zoom is the approach Ionic's
documentation recommends for dynamic font scaling on Chrome for Android.

The zoom factor is effectively applied twice. Geometry APIs
(`getBoundingClientRect()` on the trigger, content and arrow, plus
`clientX`/`clientY` for `reference="event"`) report values in the zoomed
coordinate space. Those values are written straight into the inline
`top`/`left`/`--width` styles on `.popover-content`, which are
interpreted in the unzoomed layout space and then re-scaled by the
browser.

## What is the new behavior?

- The effective zoom is read from the **popover's own context** via
`currentCSSZoom`, not from `document.documentElement`. This picks up a
zoom applied anywhere above the popover and accounts for zoom
accumulated across several ancestors. Where the property is unavailable,
it falls back to the ratio between the element's bounding rect and its
`offsetWidth`; differences below a small tolerance are treated as no
zoom, since `offsetWidth` is integer-rounded and would otherwise report
a phantom zoom.
- Every rect-derived measurement is normalized by that factor: trigger
and content rects, arrow dimensions, the `size="cover"` width, and the
pointer coordinates used by `reference="event"`.
- `innerWidth`/`innerHeight` are scaled into the same space. They are
not affected by CSS `zoom`, so leaving them alone made the offscreen
adjustment clamp against a viewport larger than the space actually
available, letting the popover render past the edge of the screen.
- Behavior is unchanged when no zoom is applied: the detected factor is
exactly `1` and every normalization is a division by `1`.

This mirrors how Floating UI addressed the same problem in
floating-ui/floating-ui#3492 — `Element.currentCSSZoom` as both the
value and the feature detector, with a default of `1` on engines that
lack it. Their fix also had to scale the overflow bounds inside
`detectOverflow()`, which is the same class of issue as the
`innerWidth`/`innerHeight` point above.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

The new `zoom` parameters on the popover positioning helpers are
optional and default to `1`. Those helpers are internal to the component
and are not part of the public API.

## Other information

**Tests**

Eight E2E tests in `core/src/components/popover/test/zoom/`, covering
the review points raised on ionic-team#31047:

| Scenario | Covers |
|---|---|
| Zoom on `body`; accumulated zoom (`html` 1.2 × `body` 1.25) | zoom
applied at levels other than `documentElement` |
| `size="cover"` width matches the trigger | sizing, not just
positioning |
| `reference="event"` anchors to the pointer | pointer coordinates |
| Arrow centred on the trigger (ios) | arrow positioning |
| Popover stays within the viewport | offscreen adjustment |
| Zoomed out (`0.8`) and zoomed in (`1.5`) | factors either side of 1 |

All eight fail against `main` and pass with this change, so each one
covers the regression rather than merely passing.

These are functional assertions rather than screenshots: what is being
verified is the popover's geometry relative to its trigger, and both
boxes are read in the same coordinate space, so the relationship holds
at any zoom level. No screenshot baselines are added.

Unit tests in `core/src/components/popover/test/util.spec.ts` cover the
zoom detection itself — the `currentCSSZoom` path, the `offsetWidth`
fallback, the rounding tolerance — and the normalization of content,
trigger and arrow measurements.

**Verification**

The spec suite passes in full: 82 files, 714 tests, no failures.

The zoom tests are not skipped for any browser and pass on all three
browser projects — Chromium, Firefox and WebKit — in both `ios` and `md`
modes. Assertions use a 2px tolerance to absorb sub-pixel differences
between engines.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: KanhaiyaPandey <kanhaiyapandey2232@gmail.com>
@thetaPC

thetaPC commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Thank you for submitting the PR! I'm going to close it in favor of #31426. It's been merged and will be available in an upcoming release of Ionic.

@thetaPC thetaPC closed this Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants