-
-
Notifications
You must be signed in to change notification settings - Fork 354
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
Scrolling to element uses wrong parent container if overflow-x: auto #275
Comments
Hi @baohouse, thanks for open the Issue and for the investigation. There was some similar situations where UI libraries use Thanks in adavace, |
@elrumordelaluz Yup, quite easy. Just add overflow-x: hidden to the root element in the demo. |
right! but in that case you aren't using an external UI lib, so the easy solution is to set the |
Ah true, I can use onAfterOpen/onBeforeClose to temporarily override/restore the CSS property on the wrongly selected container. This will have to do for now. |
After stepping through the code line-by-line, there was a situation where reactour wasn't scrolling to the target element further down the page. So it turns out the way scrollparent works is that it checks ancestor elements for the "overflow", "overflow-x", and "overflow-y" CSS properties, looking for "auto". See olahol/scrollparent.js#7
However, there is a funky situation where if one of the ancestors is set with an "overflow-x: hidden", it causes the "overflow-y" to become "auto". https://stackoverflow.com/questions/18135204/setting-overflow-y-causes-overflow-x-to-change-as-well
Thus this container becomes the incorrectly flagged scrollable parent. You can't even force a CSS override for "overflow-y" to be "visible"; every browser will change it to "auto"!
Right now the workaround is for me to override the CSS for the wrongly-detected container and set overflow-y to "hidden" (I can't set it to "visible" because overflow-x MUST be "hidden" because I'm using a third-party UI library, but I'm worried this will just cause unforeseen bugs down the road.
The text was updated successfully, but these errors were encountered: