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

Animating a block of HTML on scroll

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

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...

For instance:

  • Reduce the size of the code.



  • Structure the plugin better.



  • Stop repetitious 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 () {
loadvalues($(this));
if (settings.anim

Solution

When you have multiple conditions on the same value, then you should chain the conditions using else if. For example in most in most of the conditions on settings. animation.

Also, since your checks on settings.animation are highly repetitive for left, right, top, bottom, it would be better to rewrite with a loop.

Instead of this:

position = position + settings.distance;


It's more compact to write like this:

position += settings.distance;


Other than these minor issues, the code seems fine.

Code Snippets

position = position + settings.distance;
position += settings.distance;

Context

StackExchange Code Review Q#91439, answer score: 4

Revisions (0)

No revisions yet.