wintercounter / Protip

A new generation jQuery Tooltip plugin
http://protip.rocks
MIT License
359 stars 38 forks source link

Overflow-x: hidden on html and body causes protip not to show up when scrolled #21

Closed zenichanin closed 5 years ago

zenichanin commented 8 years ago

I think this is a bug. If I add overflow-x: hidden to my CSS and I have protips that go beyond the top fold of the page, the bottom protips show up but in their original non-scrolled location and thus can't even be seen on the screen.

Here's an example and if you scroll down in render area and hover over any of the Tip words near the bottom of page, you won't see a protip: https://jsfiddle.net/ouamhaw9/4/

And here it works when I comment out overflow-x: hidden: https://jsfiddle.net/ouamhaw9/5/

wintercounter commented 8 years ago

Seems like this is a Chrome bug. If overflow-x is applied both to html and body, Chrome will report document.body.scrollTop to be 0. It's a known bug. If you remove overflow-x only from the html element, it'll work just fine. Te be honest I don't know how should I fix this without having proper metrics, it even breaks jQuery also :( I leave this issue open so anyone with the same issue can see this, but no fix will be done by me.

https://jsfiddle.net/4frs821y/ https://github.com/jquery/jquery/issues/2215#issuecomment-94332101

zenichanin commented 8 years ago

Ahh ok. I'll just use it on the body element and hope Chrome will get fixed over time. Thanks for the help.

Tofandel commented 5 years ago

Looking at the jsfiddle you sent the document.body.scrollTop is correctly reported yet the bug is still present.. :/

Tofandel commented 5 years ago

So I had a look and... in the _getPosition function the document.body.scrollTop is actually not included in the calculation while it should be..

wintercounter commented 5 years ago

This is a weird situation. When you apply overflow-x: hidden both on html and body, the scroll element becomes the body. In a normal situation however the scroll element is window. You can test by adding both listeners $(window), $('body'), only 1 will fire in each case. On the other side when window is being used, body.scrollTop should report 0 which should make it safe to add everywhere.

Tofandel commented 5 years ago
case C.POSITION_TOP:
    this._offset.top += (globalOffset + arrowOffset.height) * -1 + document.body.scrollTop;

This gives me the correct result in the fiddle I don't know if it would work with other use cases though

wintercounter commented 5 years ago

I'll add it and make a few tests.

Tofandel commented 5 years ago

Thanks, it would be the same with the other top offsets as well and a + document.body.scrollLeft for the left offsets

wintercounter commented 5 years ago

Unfortuanatelly it ruins the calculation when overflow-x is not being added (which is the proper way). I need to find a proper solution to determ if it's needed or not, because document.body.scrollTop is reporting the same in both cases which is not entirely true.

wintercounter commented 5 years ago

Found the solution, publishing the fix soon.

wintercounter commented 5 years ago

Fixed and released in 1.4.18 https://github.com/wintercounter/Protip/commit/c11eef1306e5200ad3e611428a9153ffab01fa2e

Tofandel commented 5 years ago

Great, thanks! PS: The travis build failed because of wrong git credentials ;)

wintercounter commented 5 years ago

Yeah, I know :) I'm removing Travis completely. I prefer to do it manually with this environment. Maybe I'll update in the future.

On Wed, Jan 16, 2019, 17:31 Adrien Foulon <notifications@github.com wrote:

Great, thanks! PS: The travis build failed because of wrong git credentials ;)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/wintercounter/Protip/issues/21#issuecomment-454845756, or mute the thread https://github.com/notifications/unsubscribe-auth/AA60wFR5KV9M5T6NkjMev5EpyCIYtb17ks5vD1P-gaJpZM4ICf64 .