-
Notifications
You must be signed in to change notification settings - Fork 76
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
[Feature Request]Add onScrollEnd callback #416
Comments
Note: there is an upcoming proposal to the scrolling spec: The logic in this library would need to account for both browsers that start implementing the new spec, and the majority of browsers who don’t.
|
It's a huge challenge. |
Challenging but also extremely important for accessibility! For accessibility we often have to manage focus, which causes its own scroll events, BUT the native focus has no way of accounting for things like sticky headers or any of the positioning that is possible with this library (and gradually becoming possible through standards). Currently in order to have the necessary scroll modifications (block positioning, manual offset, etc) we need to call the focus function AFTER the scroll function is complete (since it has 'if-needed' built in, if it's called after the scrolling it doesn't change the document scroll position) It seems that if the focus fires in the middle of the smooth-scroll, it cancels the scrolling, but unfortunately it seems to also cancel the native behaviour (it doesn't move the document scroll position to the focused element like would happen natively... but this may be a weird consequence of both using So... it's necessary to know whether the scrolling is complete before firing the focus function. I'm currently computing the scrolling distance to base a setTimeout on if browser supports smooth-scroll, but it is not an ideal solution.. though based on the caveats @stipsan noted above, it may be as good as it gets for now 😔 |
.focus is being worked on to get similar controls as .scrollIntoView. I don’t know if that includes scrollMode:”if-needed”, but it does include behavior, block and inline at least.
Side note: It might be possible to create a ponyfill for it using compute-scroll-into-view since it have pretty much the same needs🤔 we’d still need a good solution for detecting when smooth scrolling stops though.
Until we start seeing browser support for returning promises that resolve when scrolling is done you could use https://github.com/stipsan/smooth-scroll-into-view-if-needed.
Just like the spec I linked to above it returns a promise that is resolved after the scrolling is complete. The benefit of that is that you can replace it with native Element.scrollIntoView calls one day and it likely just works.
The downside is that it does not use any native smooth scrolling at all. It uses requestAnimationFrame to update the scrollTop. Especially on low powered devices and in conditions where the main thread is busy you will see a noticeable performance difference 😓
At work we’ve built a BottomSheet that had a problem with .focus messing with the scrolling as it transitions in from the bottom of the viewport. In the end we made it so that it first renders with opacity: 0 at its final transition position. We call .focus, on the next tick we move it to the animation start position and the animate it in. That way we finally had a stable behavior with the scrolling behavior if the native focus method 🧐
… On 14 May 2019, at 16:48, Damon Muma ***@***.***> wrote:
Challenging but also extremely important for accessibility! For accessibility we often have to manage focus, which causes its own scroll events, BUT the native focus has no way of accounting for things like sticky headers or any of the positioning that is possible with this library (and gradually becoming possible through standards). Currently in order to have the necessary scroll modifications (block positioning, manual offset, etc) we need to call the focus function AFTER the scroll function is complete (since it has 'if-needed' built in, if it's called after the scrolling it doesn't change the document scroll position)
It seems that if the focus fires in the middle of the smooth-scroll, it cancels the scrolling, but unfortunately it seems to also cancel the native behaviour (it doesn't move the document scroll position to the focused element like would happen natively... but this may be a weird consequence of both using scroll-behavior: smooth) 😑
So... it's necessary to know whether the scrolling is complete before firing the focus function. I'm currently computing the scrolling distance to base a setTimeout on if browser supports smooth-scroll, but it is not an ideal solution.. though based on the caveats @stipsan noted above, it may be as good as it gets for now 😔
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
I keep wanting to use standards but they keep letting me down and I end up wishing I was using javascript animation.. but hard to do that without performance. There is another nice thing I found (that may start working better soon in most browsers..) which is that in Chrome you can call I think the approach you describe (immediately cancelling the native scroll behaviour for focus and doing your own -- without the transition complication) is probably the best all around. We don't have many transitions right now but i can definitely anticipate that becoming a problem :| |
I have recently been reading that in a lot of cases JavaScript animations can be more performant than native css transitions so that makes me feel better about maybe just switching to the smooth- version of this lib if it has a callback option! I think the reliability piece is probably worth the potential performance overhead. It makes me a bit batty trying to think of how to provide reusable components that will manage focus properly while also working whether or not a user has added |
Wish there is an 'onScrollEnd' callback it will be used to set some state / trigger some events after the scrolling finished.
The text was updated successfully, but these errors were encountered: