2
\$\begingroup\$

My current code for scrolling an image contained within a div is:

<html>

<head>
  <style type="text/css">
    #scroll {
      position: absolute;
      white-space: nowrap;
      top: 0px;
      left: 200px;
    }
    #oScroll {
      margin: 0px;
      padding: 0px;
      position: relative;
      width: 200px;
      height: 128px;
      overflow: hidden;
    }
  </style>
  <script type="text/javascript">
    function scroll(oid, iid) {
      this.oCont = document.getElementById(oid)
      this.ele = document.getElementById(iid)
      this.width = this.ele.clientWidth;
      this.n = this.oCont.clientWidth;
      this.move = function() {
        this.ele.style.left = this.n + "px"
        this.n--
          if (this.n < (-this.width)) {
            this.n = this.oCont.clientWidth
          }
      }
    }
    var vScroll

    function setup() {
      vScroll = new scroll("oScroll", "scroll");
      setInterval("vScroll.move()", 20)
    }
    onload = function() {
      setup()
    }
  </script>
</head>

<body>
  <div id="oScroll">
    <img id="scroll" src='http://i.stack.imgur.com/P8i8o.png' />
  </div>
</body>

</html>

This was designed some time ago, and I was thinking that there must be a better way of writing it now.

Since my project requires performance over looks, I want to have a high performing page, but still with little animations like this.

Is there a more efficient way of creating something like this?

\$\endgroup\$
1
\$\begingroup\$

Yes indeed! You can move your entire animation into CSS:

#car-container {
    height: 150px;
    width: 200px;
    position: relative;
    overflow: hidden;
}

#car-container > div {
    height: 200px;
    width: 200px;
    background: url(http://i.stack.imgur.com/P8i8o.png);
    background-repeat: no-repeat;
    position: absolute;
    top: 0;
    left: 0;
    height: 100%;
    animation: moveCar 10s linear infinite;
    -webkit-animation: moveCar 10s linear infinite;
    -moz-animation:    moveCar 10s linear infinite;      
}

@-webkit-keyframes moveCar {
    0% { left: 200px; }
    100% { left: -200px; }
}
@-moz-keyframes moveCar {
    0% { left: 200px; }
    100% { left: -200px; }
}
@keyframes moveCar {
    0% { left: 200px; }
    100% { left: -200px; }
}

Here's the example: http://jsfiddle.net/Hyg3C/3676/

\$\endgroup\$
  • 1
    \$\begingroup\$ I love css! (even though i'm not very good t it lol). So thank you for this, will defo look into using this type of desin more often! cheers. \$\endgroup\$ – jbutler483 Oct 23 '14 at 11:23
2
\$\begingroup\$

Here are a couple of recommendations on the JS side if you choose not to use the CSS-only solution:

  • Use private vars with the var statement instead of instance vars with the this statement unless you need to actually expose this properties for public inspection or use by another object.
  • Don't make vScroll into a global - it is only used in the scope of setup so it can be defined inside of setup, and if it isn't global, you can pass vScroll.move by reference to setInterval instead of using setInterval's quote syntax, which requires an extra parsing step.
  • Use a cross-browser compatible document.ready like jQuery's, or in the very least use DOMContentLoaded. The onload event can wait for the entire document to load, including CSS, but DOMContentLoaded occurs when the DOM is available (will increase load speed).

<html>

<head>
  <style type="text/css">
    #scroll {
      position: absolute;
      white-space: nowrap;
      top: 0px;
      left: 200px;
    }
    #oScroll {
      margin: 0px;
      padding: 0px;
      position: relative;
      width: 200px;
      height: 128px;
      overflow: hidden;
    }
  </style>
  <script type="text/javascript">
function scroll(oid, iid) {
  // 1. These should all be private vars instead of instance vars
  var oCont = document.getElementById(oid),
      ele = document.getElementById(iid),
      width = ele.clientWidth,
      n = oCont.clientWidth;

  // Public API, used by another object (app)
  this.move = function() {
    // Missing semi-colons
    ele.style.left = n + "px";
    n--;
    if (n < -width) {
      n = oCont.clientWidth;
    }
  }
}

// 2. No need for vScroll in this scope unles you need it outside of the setup function
// var vScroll

function setup() {
  var vScroll = new scroll("oScroll", "scroll");
  // 3. Pass a reference to setInterval instead of global var in quotes
  setInterval(vScroll.move, 20)
}

// 4. Use DOMContentLoaded as document.ready instead of window.onload
document.addEventListener('DOMContentLoaded', function(){
  setup()
});
  </script>
</head>

<body>
  <div id="oScroll">
    <img id="scroll" src='http://i.stack.imgur.com/P8i8o.png' />
  </div>
</body>

</html>

\$\endgroup\$

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.