7
\$\begingroup\$

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:

<link href="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/3.3.4/css/bootstrap.css" media="all" rel="stylesheet" type="text/css">
<style type="text/css">
body {
    height: 100%;
    overflow-x: hidden;
}

.animate {
    position: relative;
}

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

HTML:

<div class="container">
<div class="row">
    <div class="col-xs-3">
        <div class="animate bg-info" data-animation="left" data-distance="200" data-speed="1000" data-delay="0" data-offset="80" data-easing="easeInOutBack">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.</div>
    </div>
    <div class="col-xs-3">
        <div class="animate bg-info" data-animation="top" data-distance="200" data-speed="1000" data-delay="500" data-offset="80" data-easing="easeInOutBack">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.</div>
    </div>
    <div class="col-xs-3">
        <div class="animate bg-info" data-animation="bottom" data-distance="200" data-speed="1000" data-delay="1000" data-offset="80" data-easing="easeInOutBack">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.</div>
    </div>
    <div class="col-xs-3">
        <div class="animate bg-info" data-animation="right" data-distance="200" data-speed="1000" data-delay="1500" data-offset="80" data-easing="easeInOutBack">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.</div>
    </div>        
</div>

JavaScript:

<script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/jquery/1.11.3/jquery.js"></script>
<script type="text/javascript" src="https://code.jquery.com/ui/1.11.4/jquery-ui.js" ></script>
<script type="text/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.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);
            }
        });

        $(window).on("scroll load", function() {
            $(".animate").each(function () {
                loadvalues($(this)); 
                var i = parseFloat(settings.offset) / 100;
                var trigger = $(window).height() * i + $(window).scrollTop();
                var position = $(this).offset().top;

                if (settings.animation == 'bottom') {
                    position = position - settings.distance;
                }
                if (settings.animation == 'top') {
                    position = position + settings.distance;
                }

                if (position < trigger) {
                    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);
                    }
                };
            });
        });

        function loadvalues(obj) {
            if (obj.attr('data-animation')) {
                settings.animation = obj.data("animation");
            };

            if (obj.attr('data-offset')) {
                settings.offset = obj.data("offset");
            };

            if (obj.attr('data-distance')) {
                settings.distance = obj.data("distance");
            };

            if (obj.attr('data-easing')) {
                settings.easing = obj.data("easing");
            };

            if (obj.attr('data-speed')) {
                settings.speed = obj.data("speed");
            };

            if (obj.attr('data-delay')) {
                settings.delay = obj.data("delay");
            };
        }
    };
}(jQuery));

$(document).ready(function () {
    $.fn.animateSliders({offset : 200});
});
</script>
\$\endgroup\$
4
\$\begingroup\$

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.

\$\endgroup\$
4
\$\begingroup\$

loadValues() is rather problematic.

It checks for each possible data-attribute and if present overrides the value in settings. These repeated checks are just duplicated code which could be eliminated by just calling .data() without parameters to grab all the data-attributes. But the fact that it's modifying settings is a real problem because the function is used like so:

$(".animate").each(function () {
    loadvalues($(this));
    // ... animate based on settings ...
});

Now consider when there are two .animate elements on page:

<div class="animate" data-speed="200"></div>
<div class="animate" data-delay="1000"></div>

The first one will be animated with speed=200 and default delay, but the second one will be animated with both delay=1000 and speed=200 because each time loadValues() is ran, the values it overrides in settings will stay there.

This jQuery plugin is hard-coded to a concrete selector.

Instead of selecting .animate class directly inside the plugin, you should use the this context element:

this.each(function () {
    ...
});

After that you can use the plugin to animate any element:

$(".animate, .some-other-class").animateSliders();
\$\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.