Skip to content

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

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

fix(popover): correct positioning and sizing when html zoom is applied#31047
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.

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.

3 participants