HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Improving jQuery scrolldown animations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
animationsjqueryimprovingscrolldown

Problem

I'm pretty new to client side scripting and I'm still learning.

I've written this JS plugin which animates blocks of HTML (fade-in from left/top/right/bottom) as you scroll down the page.

Everything seems to be works correctly, but was just wondering if there is anyone who could suggest how I can improve on this?

e.g:

  • Reduce the size of the code.



  • Structure the plugin better.



  • Stop repetitive code.



  • Increase browser compatibility.



  • Make better use of jQuery keywords.



Really, just doing the same thing, but, written better!

CSS:


body {
    height: 100%;
    overflow-x: hidden;
}

.animate {
    position: relative;
}

.bg-info {
    padding: 10px;
    text-align: center;
}


HTML:


    
        Nulla vel varius dolor. In pellentesque mi ac congue pulvinar. Sed cursus tincidunt condimentum. Curabitur vel velit leo. Nulla condimentum dolor dui, nec convallis elit congue eu.
    
    
        Nulla vel varius dolor. In pellentesque mi ac congue pulvinar. Sed cursus tincidunt condimentum. Curabitur vel velit leo. Nulla condimentum dolor dui, nec convallis elit congue eu.
    
    
        Nulla vel varius dolor. In pellentesque mi ac congue pulvinar. Sed cursus tincidunt condimentum. Curabitur vel velit leo. Nulla condimentum dolor dui, nec convallis elit congue eu.
    
    
        Nulla vel varius dolor. In pellentesque mi ac congue pulvinar. Sed cursus tincidunt condimentum. Curabitur vel velit leo. Nulla condimentum dolor dui, nec convallis elit congue eu.
            


JavaScript:

```

(function ($) {
$.fn.animateSliders = function(options) {
var style = { opacity: "0", "-ms-opacity": "0" };
var settings = $.extend({
offset: 80,
distance : 200,
animation: "left",
easing: "easeInOutBack",
speed: 1000,
delay: 0
}, options);

$(".animate").each(function () {

Solution

Just a few quick notes, this code block:

if (settings.animation == 'left') {
    $(this).delay(settings.delay).animate({ 'left': '0px', opacity: 1 }, settings.speed, settings.easing);
}
if (settings.animation == 'top') {
    $(this).delay(settings.delay).animate({ 'top': '0px', opacity: 1 }, settings.speed, settings.easing);
}
if (settings.animation == 'right') {
    $(this).delay(settings.delay).animate({ 'right': '0px', opacity: 1 }, settings.speed, settings.easing);
}
if (settings.animation == 'bottom') {
    $(this).delay(settings.delay).animate({ 'bottom': '0px', opacity: 1 }, settings.speed, settings.easing);
}


Is performing the same action no matter what the value is, so you could reduce it to either:

if (settings.animation) {
    var animationOptions = {
       opacity: 1
    };
    animationOptions[settings.animation] = '0px';
    $(this).delay(settings.delay).animate(animationOptions, settings.speed, settings.easing);
}


or checking that the value is one of 'left', 'right', 'top', 'bottom'

if (~['left', 'right', 'top', 'bottom'].indexOf(settings.animation)) {
    var animationOptions = {
       opacity: 1
    };
    animationOptions[settings.animation] = '0px';
    $(this).delay(settings.delay).animate(animationOptions, settings.speed, settings.easing);
}


You could apply the same reduction to this code block:

$(".animate").each(function () {
    loadvalues($(this));
    if (settings.animation == 'left') {
        $(this).css({ left: -settings.distance + 'px' }).css(style);
    }
    if (settings.animation == 'top') {
        $(this).css({ top: -settings.distance + 'px' }).css(style);
    }
    if (settings.animation == 'right') {
        $(this).css({ right: -settings.distance + 'px' }).css(style);
    }
    if (settings.animation == 'bottom') {
        $(this).css({ bottom: -settings.distance + 'px' }).css(style);
    }
});


Additionally, your loadValues function could be reduced to a couple lines of code if you get all data- attributes and iterate the list:

function loadvalues(obj) {
   var dataKeys = obj.data();
   for(var key in dataKeys) {
     settings[key] = dataKeys[key];
   }
}

Code Snippets

if (settings.animation == 'left') {
    $(this).delay(settings.delay).animate({ 'left': '0px', opacity: 1 }, settings.speed, settings.easing);
}
if (settings.animation == 'top') {
    $(this).delay(settings.delay).animate({ 'top': '0px', opacity: 1 }, settings.speed, settings.easing);
}
if (settings.animation == 'right') {
    $(this).delay(settings.delay).animate({ 'right': '0px', opacity: 1 }, settings.speed, settings.easing);
}
if (settings.animation == 'bottom') {
    $(this).delay(settings.delay).animate({ 'bottom': '0px', opacity: 1 }, settings.speed, settings.easing);
}
if (settings.animation) {
    var animationOptions = {
       opacity: 1
    };
    animationOptions[settings.animation] = '0px';
    $(this).delay(settings.delay).animate(animationOptions, settings.speed, settings.easing);
}
if (~['left', 'right', 'top', 'bottom'].indexOf(settings.animation)) {
    var animationOptions = {
       opacity: 1
    };
    animationOptions[settings.animation] = '0px';
    $(this).delay(settings.delay).animate(animationOptions, settings.speed, settings.easing);
}
$(".animate").each(function () {
    loadvalues($(this));
    if (settings.animation == 'left') {
        $(this).css({ left: -settings.distance + 'px' }).css(style);
    }
    if (settings.animation == 'top') {
        $(this).css({ top: -settings.distance + 'px' }).css(style);
    }
    if (settings.animation == 'right') {
        $(this).css({ right: -settings.distance + 'px' }).css(style);
    }
    if (settings.animation == 'bottom') {
        $(this).css({ bottom: -settings.distance + 'px' }).css(style);
    }
});
function loadvalues(obj) {
   var dataKeys = obj.data();
   for(var key in dataKeys) {
     settings[key] = dataKeys[key];
   }
}

Context

StackExchange Code Review Q#91395, answer score: 3

Revisions (0)

No revisions yet.