Skip to content
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

Popper is not correctly placed by 1px for some viewport widths in flexbox container #713

Closed
piecyk opened this issue Nov 16, 2018 · 4 comments

Comments

@piecyk
Copy link
Contributor

piecyk commented Nov 16, 2018

CodePen demo

https://codepen.io/anon/pen/WYOzLr

Steps to reproduce the problem

  1. resize the codepen window

screen shot 2018-11-16 at 10 20 47

screen shot 2018-11-16 at 10 21 08

What is the expected behavior?

Popper should be placed pixel perfect to reference element

What went wrong?

For some viewport widths we got in computeStyle popper.left very close to integer value, but Math.flor in this case will break the offset by 1px

popper.left 561.984375
offsets.left 561

https://github.com/FezVrasta/popper.js/blob/master/packages/popper/src/modifiers/computeStyle.js#L44 changing it back to Math.round looks like fixes the problem

Any other comments?

Connected with #277

@atomiks
Copy link
Collaborator

atomiks commented Nov 16, 2018

Maybe centering needs to use floor for left/right, but using shifting like -start and -end should always round. Changing back to round will cause the incorrect centering problem... 🤕

Edit: Actually I think the width of the popper should be computed. If it's an odd number of pixels wide, use floor, otherwise use round.

const roundingFunc = popper.width % 2 === 0 ? Math.round : Math.floor
    
const offsets = {
  left: roundingFunc(popper.left),
  top: Math.round(popper.top),
  bottom: Math.round(popper.bottom),
  right: roundingFunc(popper.right)
};

Play here: https://codepen.io/anon/pen/Pxjdoo

I'll add more tests

@piecyk
Copy link
Contributor Author

piecyk commented Nov 16, 2018

Yep, it could be. In case -start | -end round looks like better fit. Will try to dig more into it 👍

@atomiks
Copy link
Collaborator

atomiks commented Nov 16, 2018

Looks like the odd/even check doesn't fix it. This seems to work best, and preserves proper rounding for centering:

const roundingFunc = data.placement.indexOf('-') > -1 ? Math.round : Math.floor

@piecyk
Copy link
Contributor Author

piecyk commented Nov 16, 2018

It make sens that the width of popper don't apply in this scenario as we position to edge of reference element, when cantering it's a different story, IMHO this should resolve this issue

Thanks for helping with 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

No branches or pull requests

2 participants