2
\$\begingroup\$

I wanted to write a function that when invoked, will scroll the page downwards in a smooth fashion.

Comments and criticism welcome. I don't like, for example that determining whether it is done is not based on the scroll position.

const EASING = BezierEasing(0, .1, .1, 1); // Example values.
const DURATION = 1000; // ms.

document.addEventListener('DOMContentLoaded', 
  () =>  {
    document.querySelector('#foo')
      .addEventListener('click', onClick, false);
});

function onClick(e) {
  scroll(400); // px.
  e.preventDefault();
  return false;
}

function scroll(distance) {
  var start, travelled, html;

  start = performance.now();
  travelled = 0;
  html = document.querySelector('html');

  (function move(now) {
    var fractionDone, dy, toMove;

    fractionDone = (now-start) / DURATION;

    if((1 - fractionDone) <= 0) {
      return; // Done!
    }

    if(window.scrollY + window.innerHeight 
      === html.offsetHeight) {
      return; // At bottom of page.
    }

    dy = ((EASING.get(fractionDone)) * distance);
    toMove = Math.floor((dy - travelled)); // `scrollBy` only accepts integers.

    if(toMove > 0) {
      window.scrollBy(0, toMove);
      travelled += toMove;
    }

    requestAnimationFrame(move);
  }(start));
}
<!DOCTYPE html>
<html>
  <head>
    <script src="https://rawgit.com/gre/bezier-easing/master/build.js"></script>
  </head>

  <body>
    <a href id="foo">Click Me!</a>
    <script>
      var body = document.querySelector('body');
      for(var x = 0; x < 50; x++) {  
        var div = document.createElement("div"); 
        var text = document.createTextNode(x);
        div.appendChild(text);
        body.appendChild(div);
      }
    </script>
  </body> 
</html>

\$\endgroup\$
  • \$\begingroup\$ Note that () => { … } is ECMAScript 6, which may not work in all browsers today. \$\endgroup\$ – 200_success Nov 22 '15 at 18:58
2
\$\begingroup\$

Now assuming you simply just want to use standard APIs and ES6 (because from a cross-browser point of view, this code isn't guaranteed to work)...

  • onClick is unnecessarily pulled out. You can just inline it. Also, I believe return false are unnecessary when you already have preventDefault. You wouldn't want to prevent bubbling as something else might be delegating their handlers.

    document.addEventListener('DOMContentLoaded', () =>  {
        document.querySelector('#foo').addEventListener('click', e => {
          event.preventDefault();
          scroll(400);
        }, false);
    });
    
  • Variable declarations are also unnecessarily separated. It's not like they're defined somewhere in the middle (where you sometimes need to pull them up for visibility).

    var start = performance.now();
    var travelled = 0;
    var html = document.querySelector('html');
    
  • Pull out your move function from your scroll. What happens is that every call to scroll, you're creating move.

  • I also suggest you schedule you call requestAnimationFrame first thing in the callback. That's so your code won't hold up the scheduling of the next frame. Should you want to bail out, you can always use cancelAnimationFrame.

  • Math.floor can be substituted by | 0 (bitwise or). Under the hood, it transforms the number from float to int, dropping the decimals. It's fast one some engines, but your mileage may vary.

  • Your "bail out" conditions can simply be combined:

    if((1 - fractionDone) <= 0 || window.scrollY + window.innerHeight === html.offsetHeight) return
    
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks. Note that scroll is only invoked once per user interaction, making the creation of the function-object a small overhead. move is indirectly recursive, but does not exhibit the same problem. \$\endgroup\$ – Ben Nov 22 '15 at 21:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.