Animating a div element

I would like to see how would you write this code for best practices, it will help me to learn from you.

This is my jQuery code I use on scroll event to addClass and animate div element.

$(document).on("scroll", function() {

    var about = $(".media-about"),
        info = $(".media-info"),
        history = $(".media-history"),
        dimension = $(".media-dimension");

    var scroll = $(this).scrollTop();

    if (scroll >= 803) {
        about.closest(".animated").addClass("tada");    
    } if (scroll >= 1723) {
        info.closest(".animated").addClass("tada"); 
    } if (scroll >= 2096) {
        history.closest(".animated").addClass("tada");
    }
    if (scroll >= 2548) {
        dimension.closest(".animated").addClass("flipInY");
    }  

    // console.log(typeof scroll);
    console.log(scroll);
});

Structurally, it looks ok to me. I'd be worried about the hard-coded numbers, though. If your page changes, you'll have to update all those.

I'm betting the numbers match the offsets of some elements, so it'd be more flexible to determine the numbers at runtime, using jQuery's .offset() function.

I would perhaps do something like this (assuming the offsets you want are for the elements you're currently selecting):

// set up your "scroll triggers" (using "klass" since "class" is a reserved word)
var scrollTriggers = [
  { selector: ".media-about", klass: "tada" },
  { selector: ".media-info", klass: "tada" },
  { selector: ".media-history", klass: "tada" },
  { selector: ".media-dimension", klass: "flipInY" }
];

// on load, get the elements and their offsets
$(function () {
  scrollTriggers.forEach(function (trigger) {
    trigger.element = $(trigger.selector);
    trigger.offset = trigger.element.offset().top;
  });

  // sort according to offset
  scrollTriggers.sort(function (a, b) { return a.offset - b.offset });
});

// on scroll, loop through the triggers
$(document).on("scroll", function () {
  var scrollTop = $(this).scrollTop(),
      trigger, i, l;
  for(i = 0 , l = scrollTriggers.length ; i++) {
    trigger = scrollTriggers[i];
    if(trigger.offset <= scrollTop) {
      trigger.element.closest(".animated").addClass(trigger.klass);
    } else {
      break; // no need to check the others; their offsets are all higher
    }
  };
});

It's still doing a some unnecessary work (finding closest(".animated") for elements you've already scrolled past and handled), but it shouldn't be a huge drain on things.

Point is that this way, it's easier to keep the JS in sync with the actual markup (no hard-coded numbers), and we've reduced duplication.

An even more automated way to do this would be to give all the relevant elements a special class, e.g. scroll-trigger, and give them a data-* attribute, e.g. data-triggered-class="tada" to hold the class name that should be added when triggered.

With that, the code becomes

var scrollTriggers = [];    

// on load, get the elements and their offsets
$(function () {
  scrollTriggers = $(".scroll-trigger")
    .map(function () {
      var element = $(this);
      return {
        element: element,
        offset: element.offset().top,
        klass: element.data("triggered-class")
      };
    })
    .get()
    .sort(function (a, b) { return a.offset - b.offset });
});

// (same on-scroll handling as above)

Now, there's no need to add elements to the JS at all; just declare triggers in the HTML.

StackExchange Code Review Q#63252, answer score: 3