keepTogether doesn't avoid popper from float away from its reference element

[lukebatchelor]

CodePen demo

No codepen needed, problem reproducible on https://popper.js.org/

Steps to reproduce the problem

1. Go to https://popper.js.org/
2. Scroll down the page

What is the expected behavior?

The "Popper on bottom" popper to flow smoothly

What went wrong?

It has some weird jankyness

Any other comments?

Not sure if its a huge issue or not but thought I'd bring it to your attention.

Happening in Chrome Version 52.0.2743.116 (64-bit) on Mac OSX 10.11.4 scrolling with touch gestures on a Mac Magic Trackpad.

Give me a yell if you need any help reproducing. (Love this library btw, looking into the v1 docs now!)

[FezVrasta]

Thanks, something is definitely wrong. Gonna fix it ASAP

[FezVrasta]

Ok it was a problem with the CDN, the gh-pages was asking for the latest version of popper.js, and so the demo page switched to the v1.0.0-apha.2 version, but the demo page is not yet updated to use this version. Now I forced the page to use the v0.*

Update: This is a real bug in v1!!!

[FezVrasta]

Reopening since this is an actual bug with the version 1.

We modified keepTogether to work only on the main axis, but doing so, when we set as boundariesElement viewport, the popper will float away from its reference element when the reference element is not inside the viewport.

Two possible solutions:

1. Make keepTogether read the offset from the offset modifier and make it allow only the specified offset.

2. Modify the boundariesElement behavior:

   right now we use boundariesElement to calculate the offsets and to make the flip modifier work.
   boundariesElement will be renamed to scrollParent and will have as default value scrollParent. Users may want to replace the value with viewport or window when the popper is fixed or any other special case.
   flip modifier will have a new boundariesElement option that will be used to make the popper flip.

Solution 1 is much easier, but probably 2 is the way to go on long term.

@AndreaScn @nadiam84 thoughts?

[FezVrasta]

@AndreaScn assigning it to you since #59 have been closed in favor of this one. If you can find some time to work on this it would be really awesome, this is a critical high priority issue to be able to release v1.

[adevnadia]

For me it feels like the fix should be in keepTogether modifier, since it's responsible for the 'keep together' behavior. Logically. So I vote for the solution 1. :)

[FezVrasta]

Solution 1 is more a patch than a solution. We'll still have to implement the scrollParent option for boundariesElement that will then be required in almost all the cases (making viewport and window useless). 😕

[FezVrasta]

I'm working on it, almost done.

The flip modifier now detects the flip needs checking the boundaries (smart method) instead of relying on preventOverflow.

[FezVrasta]

Fixed on v1-dev.