`tether-out-of-bounds` set wrongly when only `scrollParent` is `document.body`

[matthewgertner]

I have a tether directly inside a DOM window with no intermediate scrollParent. When I set constraints: { to: 'scrollParent' } then I get incorrect behavior of the tether-out-of-bounds class. If I resize the window so the target and element are not visible, both get the tether-out-of-bounds class as expected. But if I then scroll them into view, the class remains. In other words, it doesn't seem to take the scroll position into account.

It seems to work fine if I use constraints: { to: 'window' } instead, but I want my code to work both when there is an intermediate scrollParent and when there isn't.

[matthewgertner]

I looked into this in more detail and it seems to me like a clear bug. getBoundingRect (https://github.com/HubSpot/tether/blob/master/src/js/constraint.js#L12) is used to get the bounds of the visible area to determine whether an element should get the tether-out-of-bounds class. It handles two cases: constraining to scrollParent or to window. The bug is that in case of scrollParent, the to bounding rect is always the same and the bounds of the element retrieved in getBounds (https://github.com/HubSpot/tether/blob/master/src/js/utils.js#L95) are what change. In the case of window, the to bounding rect changes since it uses pageXOffset and pageYOffset (which change as you scroll).

if you are scrolling the whole window then the scroll parent is body. Now getBounds calls getOrigin (https://github.com/HubSpot/tether/blob/master/src/js/utils.js#L49) which adjusts the bounds of the element based on a div place in the upper left of the window, apparently to fix some issue with jitter (https://github.com/HubSpot/tether/blob/master/src/js/utils.js#L50). Since the elements inside the body move along with that special div, the values returned by getBounds never change. This is correct if to is window since it is the to bounding rect that changes in this case. Not so much in the case of scrollParent since neither the to bounds or the element bounds change as you scroll, which explains the behavior I described in my OP.

IMO the behavior of scrollParent and window should be the same in getBoundingRect (i.e. bounds don't change as you scroll), and the use of pageXOffset and pageYOffset should be moved to getBounds and used as a special case for adjusting the bounds of elements whole direct scroll parent is body (i.e. probably use element.scrollLeft and element.scrollTop) for this. I don't know the code base very well though so I'm not sure which other issues this might cause.

Is this repo actively maintained? Will anyone take a look if I post a PR?

[TrevorBurnham]

I'd certainly review a PR to fix this.

[lukebmay]

I think I may have found an additional case where .tether-out-of-bounds is not set when it should be. In these two JsFiddles if you click the green box you'll see corner pieces show up that are tethered to the green box. In one version, "scrollParent" works fine, and in another (a case with a wrapper div around each element) the "scrollParent" fails to hide the tethers when they go out of bounds of the grandparent. Perhaps there needs to be an additional "constraints" keyword (called "scrollAncestor" for example) which would add the .tether-out-of-bounds class whenever the tether goes out of bounds of any ancestor's scrolled view? Just a thought.

Works Properly: https://jsfiddle.net/lukebmay/m5psrqgz/

Fails to add .tether-out-of-bounds class: https://jsfiddle.net/lukebmay/ueqtnep1/1/