tholman / elevator.js

Finally, a "back to top" button that behaves like a real elevator.
http://tholman.com/elevator.js
6.63k stars 502 forks source link

Slow scrolling in Chrome #138

Open manuelmeurer opened 4 years ago

manuelmeurer commented 4 years ago

Firstly, thanks for this great project! :)

I implemented it on my website (https://uplink.tech/) but noticed the scrolling is a bit weird in Chrome (v83 on macOS): first it scrolls very slowly, and then very quickly. In Firefox and Safari it works fine.

What can I do to debug this further?

tholman commented 4 years ago

Hmmm i see what you mean, strange considering the demo site is working fine on chrome 83. Is there any other code on the site that is trying to edit the scroll position?

How have you implemented the library? (can't really see the code, since its all compiled)

manuelmeurer commented 4 years ago

How have you implemented the library?

Via Webpack(er), but I've tried everything else as well... commenting out all other JS code (one piece was watching the scroll event, so I thought that was the culprit, but nothing changed) and simply loading it on window.onload. The problem persists...

Unless you have any other idea (maybe the structure of the page or some specific CSS?) I'm sorry to say I give up... :(

erichstark commented 4 years ago

Hey @manuelmeurer,

it looks like a normal behaviour if you have in your CSS scroll-behavior: smooth; on html or body tag.

I am not sure why it is working in Firefox, it looks like a bug. Safari and Firefox are working fine on my page https://stark.codes

manuelmeurer commented 4 years ago

@erichstark Ah yeah, that's it! 😄 Now the question is how can I keep scroll-behavior: smooth; but still make Elevator.js work as expected... Maybe it would work to reset the scroll-behavior when the Elevator button is clicked, and add it back when it's done?

erichstark commented 4 years ago

That is actually good idea. It could work. Why do you need smooth scroll? If it is for SPA navigation via #some-id, it could be done by some JavaScript code. And with JavaScript it would work also in Safari.

manuelmeurer commented 4 years ago

I like to use CSS instead of JS wherever possible. 😄 Since scroll-behavior is reasonably well supported in browsers by now, there is no need for JS based smooth scroll code anymore IMHO.

manuelmeurer commented 4 years ago

It works! 🎉 I added these callbacks:

startCallback: function() {
  $('html').attr('style', 'scroll-behavior:auto !important')
},
endCallback: function() {
  $('html').removeAttr('style')
}

Of course they should check if a style attribute is already present on <html> and add/remove the style accordingly...

manuelmeurer commented 4 years ago

Thanks for solving this riddle, @erichstark!

@tholman, do you think it's worth it mentioning this gotcha in the README? Happy to do a PR for that!

tholman commented 4 years ago

Wow! What a journey & amazing catch, haha, wouldn't have got that at all. Definitely worth a readme gotcha at the very least 🙏

tholman commented 4 years ago

Am also guessing that adding the behavior: 'auto' option to the scrollTo events in the library might also do the job here and solve this once and for all.

manuelmeurer commented 4 years ago

I tried to get it to work with the behavior parameter by changing https://github.com/tholman/elevator.js/blob/a8332ecf7fa45cf51a177959359780e573205ede/elevator.js#L91 to window.scrollTo({ top: easedPosition, behavior: 'auto' }); but it doesn't seem to work for me...

erichstark commented 4 years ago

@manuelmeurer btw I think style attr is readonly.

Better approach should be setting directly the property like

let html = document.querySelector('html');
// proper init ...

            startCallback: function() {
              html.style.scrollBehavior = 'auto';
            },

            endCallback: function() {
              html.style.scrollBehavior = '';
            }
manuelmeurer commented 4 years ago

document.querySelector('html').style.scrollBehavior works for me. :)