icd2k3/react-scroll-into-view-if-needed

Window doesn't scroll upward when scrollMode is 'if-needed'

petegleeson opened this issue ยท 7 comments

Hello ๐Ÿ‘‹ I am having some difficulty getting the window to scroll upward to a component when the scroll mode is 'if-needed'. If I set the scroll mode to 'always' it seems to work the way I expect.

I have created a codesandbox which demos this. Try clicking on the 'prev' button https://codesandbox.io/s/4w50o96p9w

Does this look like a bug to you? Thanks!

Hey @petegleeson! This does indeed seem to be a bug.

My guess is that there is a problem with the logic in the scroll-into-view-if-needed package that might be incorrectly be determining if a node "needs" to be scrolled into view.

I'll do some investigation in the near future and see if I can narrow it down. Thanks for reporting and setting up that sandbox!

Thanks @icd2k3. I had a bit more of an explore last night and created a non-react version of that example.
https://codepen.io/anon/pen/QxKKwg?editors=1010

This example appears to work fine so maybe this is a problem specific to usage in React?

Thanks @petegleeson for testing! That's exactly what I was going to try when I got home from work, but you beat me to it! ๐Ÿ˜›

That is interesting. This component passes the React ref for the node and options to the scroll-into-view-if-needed module. So, at first glance, I'm not sure what the problem might be. Maybe some discrepancy with the way React handles refs or something... but I'll do some digging soon and see if I can narrow it down.

Thanks again for the help debugging

Hey @petegleeson, after some investigation, this does look to be an issue with scroll-into-voew-if-needed ponyfill.

Your last example is using the native scrollIntoView MDN (ex: elem.scrollIntoView) that some browsers support (to varying degrees). This component uses a polyfill to cover a wider range of browsers.

I set up this demo of the issue using only the raw scroll-into-voew-if-needed method:
https://codesandbox.io/s/38887pvyp6

I'll file an issue on their end soon and link it here. If I end up finding the problem in that codebase I'll submit a PR there as well.


OT: Unrelated to the bug - I could also update this package to use the native browser method for those that don't mind the spotty browser support if that might be valuable :)

Ah crap, sorry for the bum steer mate. I fixed up the example to use the library's scrollIntoView function now.

https://codepen.io/anon/pen/QxKKwg?editors=1011

I am observing the same behaviour now as the React example so looks to me as well like a problem inside the ponyfill library.

๐Ÿ‘† Added an issue to the other module! I'll post back here with updates.

hey @petegleeson the issue should be resolves as of version 2.1.3. Thanks again for reporting this!