scroll-data-hook
scroll-data-hook copied to clipboard
WIP Allow attaching scroll data hook to any element
@dejorrit This is extremely useful and solved some tricky scroll behaviour I was struggling with, so kudos for creating this hook. The one thing is that I have to modify the hook slightly for my use-case because I need to attach to a DOM element instead of the window. From initial quick tests it works with pretty minimal changes to accept a ref instead of assuming the window
. I'd rather submit back to this project than permanently forking if possible so I'm wondering if you're open to supporting this option? My proposal would be to add an optional ref
in the config. If it's not provided than behaviour would remain unchanged but if it is provided then the scroll data would be collected from the provided element instead. This draft doesn't have the config option, it's just a proof of what changes would be required to handle DOM elements instead of the window
.
Hey @sanbornhilland thanks a lot for coming up with this! To be honest, I knew this use case would be needed at some point :)
So feel free to continue your work on this PR to make it support both window and refs. Would be very nice if we could add this functionality!
@dejorrit I have a code sandbox I've been using here if you want to easily play with changes: https://codesandbox.io/s/usescrolldatamod-j5xye?file=/src/App.js. You can swap between attaching the window and attaching to the ul
by commenting/uncommenting the lines with the ul
ref (55, 66, 98).
One thing I noticed is that if use the window but scroll the scroll container, scrolling
and milliseconds
update. That seems like a bug to me. It should probably be checking the target to make sure that it's only capturing scroll events that originated in the right place. What do you think? That issue exists in the latest release so probably it would make sense to do as a separate fix.
Let me know what you think about the direction of this PR. I'll squash everything before this gets merged.