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>
2 Answers 2
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.
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();
Explore related questions
See similar questions with these tags.