1
\$\begingroup\$

This seems like an awful lot of code and I'm guessing can be condensed and made more efficient somehow. Can anyone suggest this to me at all?

// Add the active class to the first element
$('.indexWorkDetailsInner:first').addClass('active');

// Next Button Click
$('.indexWorkRight').click(function() {

    // Move all except the active element to the left
    $('.indexWorkDetailsInner').each(function() {
        if($(this).hasClass('active')) { }
            else {
                $(this).css({'left':'-450px'});
            }
    });

    // Animate the active element off the screen
    $('.indexWorkDetailsInner.active').stop().animate({'left' : '450px'}, 1000, function () {

        $(this).removeClass('active').css({'left':'-450px'}); // Remove the active class, and move element back to the left
        $(this).next('.indexWorkDetailsInner').animate({'left' : '0px'}, 1000).addClass('active'); // Animate the next element in to view
        $('.indexWorkDetailsInner:last').after($('.indexWorkDetailsInner:first')); // Move the last element to after the first                

    });

});

// Previous Button Click
$('.indexWorkLeft').click(function() {

    // Move all except the active element to the right
    $('.indexWorkDetailsInner').each(function() {
        if($(this).hasClass('active')) { }
            else {
                $(this).css({'left':'450px'});
            }
    });

    // Animate the active element off the screen
    $('.indexWorkDetailsInner.active').stop().animate({'left' : '-450px'}, 1000, function () {

    $(this).removeClass('active').css({'left':'450px'}); // Remove the active class, and move element back to the right
    $('.indexWorkDetailsInner:first').before($('.indexWorkDetailsInner:last')); // Move the first element before the last item
    $(this).prev('.indexWorkDetailsInner').animate({'left' : '0px'}, 1000).addClass('active'); // Animate the previous element in to view

    });

});

// Add the active class to the first element
$('.indexMonitorWork img:first').addClass('active');

// Next Button Click
$('.indexWorkRight').click(function() {

    // Move all except the active element to the left
    $('.indexMonitorWork img').each(function() {
        if($(this).hasClass('active')) { }
            else {
                $(this).css({'left':'-370px'});
            }
    });

    // Animate the active element off the screen
    $('.indexMonitorWork img.active').stop().animate({'left' : '370px'}, 1000, function () {

        $(this).removeClass('active').css({'left':'-370px'}); // Remove the active class, and move element back to the left
        $(this).next('.indexMonitorWork img').animate({'left' : '0px'}, 1000).addClass('active'); // Animate the next element in to view
        $('.indexMonitorWork img:last').after($('.indexMonitorWork img:first')); // Move the last element to after the first                

    });

});

// Previous Button Click
$('.indexWorkLeft').click(function() {

    // Move all except the active element to the right
    $('.indexMonitorWork img').each(function() {
        if($(this).hasClass('active')) { }
            else {
                $(this).css({'left':'370px'});
            }
    });

    // Animate the active element off the screen
    $('.indexMonitorWork img.active').stop().animate({'left' : '-370px'}, 1000, function () {

    $(this).removeClass('active').css({'left':'370px'}); // Remove the active class, and move element back to the right
    $('.indexMonitorWork img:first').before($('.indexMonitorWork img:last')); // Move the first element before the last item
    $(this).prev('.indexMonitorWork img').animate({'left' : '0px'}, 1000).addClass('active'); // Animate the previous element in to view

    });

});

It is code for a work carousel. The first two functions move the text left and right. The bottom function moves the image screenshot.

\$\endgroup\$
4
  • \$\begingroup\$ Instead of looping, you can just select on both classes at the same time: $('.indexWorkDetailsInner.active'). Also, avoid empty blocks (you can negate conditions, you know). Also, looks like you are duplicating the entire thing – do it once, but make it configurable. \$\endgroup\$
    – Ingo Bürk
    May 2 '14 at 18:34
  • \$\begingroup\$ It is duplicated because the top half moves the text, the bottom half moves the image. I know calling $('.indexWorkDetailsInner') all the time is not good and it should be a variable. If I do this, is there a way to then call :first, :last and also do the part .active with that variable? For example: var innerWork = $('.indexWorkDetailsInner'); Is there a way to then do something like (innerWork)+('.active') to be the same as $('.indexWorkDetailsInner.active') and again (innerWork:first) to be the same as $('.indexWorkDetailsInner:first') \$\endgroup\$
    – Tenatious
    May 2 '14 at 18:39
  • 1
    \$\begingroup\$ Yes \$\endgroup\$
    – Ingo Bürk
    May 2 '14 at 19:31
  • \$\begingroup\$ Excellent. Got it working perfectly now :) \$\endgroup\$
    – Tenatious
    May 2 '14 at 19:43
1
\$\begingroup\$

With help from @Ingo I got it down to the following, which I think is better.

var innerWork = $('div.indexWorkDetailsInner');
var innerImg = $('div.indexMonitorWork img');

// Add the active class to the first element
innerWork.first().addClass('active');
innerImg.first().addClass('active');

function workAnimate(direction){

    var signOne, signTwo;

    if(direction === "right" ) { signOne = '-'; signTwo = ''; }
    if(direction === "left" ) { signOne = ''; signTwo = '-'; }

    innerWork.not('.active').css({'left': signOne + '450px'}); // Move the not active text to the left
    innerImg.not('.active').css({'left': signOne + '370px'}); // Move the not active images to the left

    // Animate the active element off the screen
    innerWork.filter('.active').stop().animate({'left' : signTwo + '450px'}, 1000, function () {

        $(this).removeClass('active').css({'left': signOne + '450px'}); // Remove the active class, and move element back to the left
        if(direction === "right" ) {
            $(this).next(innerWork).animate({'left' : '0px'}, 1000).addClass('active'); // Animate the next element in to view
            innerWork.filter(':last').after(innerWork.filter(':first')); // Move the last element to after the first
        } else {
            innerWork.filter(':first').before(innerWork.filter(':last')); // Move the first element before the last item
            $(this).prev(innerWork).animate({'left' : '0px'}, 1000).addClass('active'); // Animate the next element in to view
        }
    });

    // Animate the active element off the screen
    innerImg.filter('.active').stop().animate({'left' : signTwo + '370px'}, 1000, function () {

        $(this).removeClass('active').css({'left': signOne + '370px'}); // Remove the active class, and move element back to the left
        if(direction === "right" ) {
            $(this).next(innerImg).animate({'left' : '0px'}, 1000).addClass('active'); // Animate the next element in to view
            innerImg.filter(':last').after(innerImg.filter(':first')); // Move the last element to after the first
        } else {
            innerImg.filter(':first').before(innerImg.filter(':last')); // Move the first element before the last item
            $(this).prev(innerImg).animate({'left' : '0px'}, 1000).addClass('active'); // Animate the previous element in to view
        }
    });
}

// Next Button Click
$('div#indexWorkRight').on('click', function() {
    workAnimate('right');
});

// Previous Button Click
$('div#indexWorkLeft').on('click', function() {
    workAnimate('left');
});
\$\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.