Skip to content

fix: handle scaled parent elements #1247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 3, 2021
Merged

fix: handle scaled parent elements #1247

merged 3 commits into from
Aug 3, 2021

Conversation

FezVrasta
Copy link
Member

@FezVrasta FezVrasta commented Feb 20, 2021

fixes #376

@rollingversions
Copy link

rollingversions bot commented Feb 20, 2021

Change Log for @popperjs/core (2.9.2 → 2.9.3)

Bug Fixes

  • Handle scaled parent elements

Edit changelog

Change log has not been updated since latest commit Update Changelog

@github-actions
Copy link
Contributor

github-actions bot commented Feb 20, 2021

Filename Size Change
dist/umd/popper-base.min.js 3.28 kB +183 B (5%) 🔍
dist/umd/popper-lite.min.js 4.23 kB +171 B (4%)
dist/umd/popper.min.js 6.08 kB +182 B (2%)

compressed-size-action

@XhmikosR
Copy link

@FezVrasta https://github.com/twbs/bootstrap/compare/main-xmr-test-popper (ignore the lint failure)

Not sure how to test this since including your branch seems to break building.

@FezVrasta
Copy link
Member Author

Sorry, nevermind, there are some of our own E2E tests that are failing, we'll need to fix them first.

@rmanalan
Copy link

rmanalan commented May 6, 2021

Hi. Just checking to see if there are plans to merge this in once tests pass?

@FezVrasta
Copy link
Member Author

I don't have time to fix the code so that tests pass. Contributions are welcome

@tholmes-rms
Copy link
Contributor

I've written up some logic to only apply the scale in getBoundingClientRect when called in getCompositeRect and the offsetParent is scaled.

I added a few more test cases for some combinations of reference, popper, and parent elements being scaled. I'm not convinced that this isn't going to break anything else or work for all scales scenarios, but all the tests pass (except one functional test in scrolling-fixed.test.js that's also failing for me on master?).

If you want to give me access, I can create a branch off of this branch and make a PR if you want.

@FezVrasta
Copy link
Member Author

Feel free to branch this out, it'd be great to get this fixed

@tholmes-rms
Copy link
Contributor

PR here: #1322 with the above-mentioned changes.

@FezVrasta FezVrasta merged commit f4a6550 into master Aug 3, 2021
@FezVrasta FezVrasta deleted the fix/376 branch August 3, 2021 15:24
@Nantris
Copy link
Contributor

Nantris commented Aug 28, 2021

As far as I can tell, our popper's ancestor elements have no scaling applied, but upgrading from 2.9.2 to 2.9.3 caused our popper locations to change (shifted upwards maybe 16px if I had to guess.)

@FezVrasta
Copy link
Member Author

As far as I can tell, our popper's ancestor elements have no scaling applied, but upgrading from 2.9.2 to 2.9.3 caused our popper locations to change (shifted upwards maybe 16px if I had to guess.)

Please open a new issue

@prasanga-peak
Copy link

The scaleX and scaleY values will be set to Infinity when element.offsetWidth is 0, which results in the popper location change. I'll open a new issue for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Popper position is wrong with transform: scale
6 participants