fisshy / react-scroll

React scroll component
https://github.com/fisshy/react-scroll/blob/master/README.md
MIT License
4.36k stars 437 forks source link

scrollToBottom and scrollToTop with duration is not consistent. #384

Closed eHallberg closed 5 years ago

eHallberg commented 5 years ago

I was about to implement a scroll that is ran on interval (overview page) so that it would scroll to bottom and then scroll to top with pretty long duration (15s). I quickly found that the scroll speed was not the same between them, in fact scrollToBottom was running faster then scrollToTop.

What i found was that the calculations for scrollToTop is correct, it can't really fail since it is y=0. scrollToBottom is using a function to calculate the distance to scroll with this function (I have placed comments where i changed the code):

var scrollContainerHeight = function scrollContainerHeight(options) {
  var containerElement = options.data.containerElement;
  if (containerElement && containerElement !== document && containerElement !== document.body) {
    // This is what i was able to test out and it worked on my end, but im not sure if this could have some sort of implications.
    return containerElement.scrollHeight - containerElement.offsetHeight;
    // this was the original row
    // return Math.max(containerElement.scrollHeight, containerElement.offsetHeight, containerElement.clientHeight);
  } else {
    var body = document.body;
    var html = document.documentElement;

    return Math.max(body.scrollHeight, body.offsetHeight, html.clientHeight, html.scrollHeight, html.offsetHeight);
  }
};

This is how i am using this plugin:

let down = true;
        setInterval(() => {
            if (down) {
                down = false;
                // scroll.scrollToTop();
                scroll.scrollToBottom({
                    containerId: "scrollContainer",
                    duration: 15000,
                    delay: 0,
                    smooth: "linear"
                });
            } else {
                down = true;
                // scroll.scrollToTop();
                scroll.scrollToTop({
                    containerId: "scrollContainer",
                    duration: 15000,
                    delay: 0,
                    smooth: "linear"
                });
            }
        }, 20000);
fisshy commented 5 years ago

Good catch, make a PR and check if the tests passes. It's obviously something wack when calculating the distance.

eHallberg commented 5 years ago

@fisshy Done

jpagand commented 4 years ago

It is still the case. scrollToBottom is way faster than scrollToTop given the same duration