3
\$\begingroup\$

Basically I want some help in reducing the bloat of this code. I am new to JavaScript so this represents an attempt at learning jQuery by doing. All of this code works as I want it to, but it is not efficient or maintainable long term.

I realize there is no context for what is going on, but I figure that someone could immediately see places to create reusable chunks of code at a glance. If someone is willing to help me reduce the length and improve the quality dramatically I would be happy to provide a link to the website that utilizes it. Foregoing that I am mostly looking for someone to provide insight and point me in the right direction.

jQuery(document).ready(function($){

/*RIGHT ANIMATIONS*/
$("body").on('click', '.right-tog', function(event) {
    $('.right').animate({right: 0}),
    $('#content').animate({right: 300}),
    $('header nav, header div, .right-tog, .bkgd-desc').fadeOut(300);
});

$("body").on('click', '#content, .click-thru, a[role=close]', function(event) {
    var divpos = parseInt($('.right').css('right'));

    if (divpos == 0) {
        $('.right').animate({right: '-100%'}),
        $('#content').animate({right: 0}),
        $('header nav, header div, .right-tog, .bkgd-desc').fadeIn(300);
    };
});

/*LINKS ANIMATIONS*/
$("body").on('click', '.events', function(event) {
    $('#events').animate({top: 0}),
    $('#content').animate({top: 100}),
    $('header nav, #right-tog, .bkgd-desc').fadeOut(300);

    var rightpos = parseInt($('.right').css('right'));

    if (rightpos == 0) {
        $('.right').animate({right: -600}),
        $('#content').animate({right: 0}),
        $('#main-menu, #right-tog, .bkgd-desc').fadeIn(300);
    };
});

$("body").on('click', '.links, .links-thru', function(event) {
    $('#footer').animate({bottom: 0}),
    $('#content').animate({bottom: 100}),
    $('header nav, #right-tog, .bkgd-desc').fadeOut(300);

    var rightpos = parseInt($('.right').css('right'));

    if (rightpos == 0) {
        $('.right').animate({right: -600}),
        $('#content').animate({right: 0}),
        $('#main-menu, #right-tog, .bkgd-desc').fadeIn(300);
    };
});

/*LEFT ANIMATIONS*/
$("body").on('click', '.work', function(event) {
    $('.left').animate({left: 0}),
    $('#content').animate({left: 200}),
    $('header nav, #right-tog, .bkgd-desc').fadeOut(300);
});

$("body").on('click', '#content, .work-link, a[role=close]', function(event) {
    var divPosition = parseInt($('.left').css('left'));

    if (divPosition == 0) {
        $('.left').animate({left: -1000}),
        $('#content').animate({left: 0}),
        $('header nav, #right-tog, .bkgd-desc').fadeIn(300);
    };
});

$("body").on('click', 'header navi', function(event) {
    var target = $( event.target );
    var divWidth = parseInt($(this).css('width'));

    if (divWidth < 130 & target.is('header nav') ) {
        $(this).animate({width: 130}, 100),
        $('header h1').animate({left: 160}, 150, function(){
            $(this).delay(60).animate({left: 140}, 100);
        });
    } else {
        $(this).animate({width: 60}, 100),
        $('header h1').animate({left: 70}, 100);
    }
});

/*LINKS ANIMATIONS*/
$("body").on('click', '#content', function(event) {
    var divpos = parseInt($('footer').css('bottom'));
    var eventspos = parseInt($('#events').css('top'));

    if (divpos == 0 || eventspos == 0) {
        $('footer').animate({bottom: '-60%'}),
        $('#events').animate({top: -200}),
        $('#content').animate({bottom: 0, top: 0}),
        $('header nav, #right-tog, .bkgd-desc').fadeIn(300);
    };
});


/*CLICK THRU FUNCTIONS*/
$("body").on('click', '#work-thru', function(event) {
    $('#left').animate({left: 0}),
    $('#main').animate({left: 300}),
    $('#right').animate({right: -600}),
    $('#main').animate({right: 0}),
    $('header nav, #right-tog, .bkgd-desc').fadeOut(300);
});
$("body").on('click', '#about-thru', function(event) {
    $('#right').animate({right: -600}),
    $('#main').animate({right: 0}),
    $('header nav, #right-tog, .bkgd-desc').fadeIn(300);
});

/*CATEGORY FILTER*/
$('#filter li a').click(function() {
    $('#filter li .current').removeClass('current');
    $(this).parent().addClass('current');

    var filterVal = $(this).text().replace(/ /g,'-');

    if(filterVal == 'All') {
        $('.work-grid li.hidden').fadeIn('slow').removeClass('hidden');
    } else {
        $('.work-grid li').each(function() {
            if(!$(this).hasClass(filterVal)) {
                $(this).fadeOut(600).addClass('hidden');
            } else {
                $(this).fadeIn(600).removeClass('hidden');
            }
        });
    }

    return false;
});

});
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

It looks to me like there a lot of differences in each function, it might take just as much code to account for all the differences, and by the time you are done it might be significantly more complicated! One thing that can help performance is caching your nodes by storing them as variables. To illustrate:

 jQuery(document).ready(function($){

var $rightDiv = $('.right');
var $navsOne = $('header nav, #right-tog, .bkgd-desc');
var $navsTwo = $('header nav, header div, .right-tog, .bkgd-desc');
var $content = $('#content'); 

/*RIGHT ANIMATIONS*/
$("body").on('click', '.right-tog', function(event) {
    $rightDiv.animate({right: 0}),
    $content.animate({right: 300}),
    $navsTwo.fadeOut(300);
});

$("body").on('click', '#content, .click-thru, a[role=close]', function(event) {
    var divpos = parseInt(rightDiv.css('right'));

    if (divpos == 0) {
        $rightDiv.animate({right: '-100%'}),
        $content.animate({right: 0}),
        $navsTwo.fadeIn(300);
    };
});

/*LEFT ANIMATIONS*/
$("body").on('click', '.work', function(event) {
    $('.left').animate({left: 0}),
    $content.animate({left: 200}),
    $navsOne.fadeOut(300);
});

$("body").on('click', '#content, .work-link, a[role=close]', function(event) {
    var divPosition = parseInt($('.left').css('left'));

    if (divPosition == 0) {
        $('.left').animate({left: -1000}),
        $content.animate({left: 0}),
        $navsOne.fadeIn(300);
    };
});

etc... here is a little test that illustrates performance differences. That's a lot of animation!!

\$\endgroup\$
1
  • \$\begingroup\$ Yes it is! I am a proponent of pure CSS solutions but in this case the build relies on click events. Also this is mostly a buddy's code that I am adapting. He is roughly at the same skill level with jQuery as I. Thank you for the suggestion! \$\endgroup\$
    – th3lonius
    May 3, 2014 at 2:08
1
\$\begingroup\$

A minor thing that I see right away is that I see a bunch of $(selector).animate({options}); calls with slightly different parameters. What I'd do is write a single function for these:

function animateSelector(selector, options){
    $(selector).animate(options);
}

This way the construct "I select this, then I animate it with these options" is abstracted away to "animate this selector with these options". it doesn't change the performance, but it makes slightly clearer what's going on.

Another problem, slightly bigger, is that in a lot of places, you have an inline function (nothing wrong with that) which uses commas as line endings, which should be semicolons (;).

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