1
\$\begingroup\$

I feel like I’m violating a lot of DRY rules here but can’t think of a better way to do this. I have two event methods (there are actually four total just showing two) that are doing a lot of similar actions but on different DOM elements. Is there a better way to re-factor this so I’m not repeating myself so much.

    $('.user .expandDetails a').on('tap', function () {
        var expandWrapper = $('.userFooter');
        if ($('.userInfo .details').is(':hidden')) {
            $('.userInfo .details').addClass('active').slideDown();
            $('.userInfo .expandUserDetails').removeClass('down').addClass('up').addClass('active');
        } else {
            $('.userInfo .expandUserDetails').removeClass('up').addClass('down').removeClass('active');
            $('.userInfo .details').removeClass('active').slideUp();
        }
    });

    $('.user .paymentDetails a').on('tap', function () {
        var expandWrapper = $('.paymentFooter');
        if ($('.paymentInfo .details').is(':hidden')) {
            $('.paymentInfo .details').addClass('active').slideDown();
            $('.paymentInfo .expandPaymentDetails').removeClass('down').addClass('up').addClass('active');
        } else {
            $('.paymentInfo .expandPaymentDetails').removeClass('up').addClass('down').removeClass('active');
            $('.paymentInfo .details').removeClass('active').slideUp();                                      
        }
    });

Please include code samples where applicable somewhat new to jquery.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

You're right, you are violating some DRY rules, so lets clean it up. First, I would cache some of the selectors you're calling.

var $info = $('.userInfo');
var $details = $info.find('.details');
var $detailsExpand = $info.find('.expandUserDetails');

Then I would take a look at some of the toggle options that jQuery offers, mainly:

This will save you the trouble of having to use .addClass, .removeClass, slideUp, and slideDown.

For example, you have:

if ($('.paymentInfo .details').is(':hidden')) {
    $('.paymentInfo .details').addClass('active').slideDown();
    $('.paymentInfo .expandPaymentDetails').removeClass('down').addClass('up').addClass('active');
} else {
    $('.paymentInfo .expandPaymentDetails').removeClass('up').addClass('down').removeClass('active');
    $('.paymentInfo .details').removeClass('active').slideUp();
}

We can remove that else statement, and just let our new toggle methods handle the adding/removing.

if ($details.is(':hidden')) {
    $details
        .toggleClass('active', true) // true means the class will be added
        .slideToggle();

    $detailsExpand
        .toggleClass('down', false)  // false means the class will be removed
        .toggleClass('up', true)
        .toggleClass('active', true);
}

Lastly, and this might be overkill, you could try creating a jQuery plugin:

$.fn.toggleDetails = function (options) {
    var settings = $.extend({
        infoClass: null,
        expandClass: null
    }, options);

    var $info = $(settings.infoClass);

    var $details = $info.find('.details');
    var $detailsExpand = $info.find(settings.expandClass);

    if ($details.is(':hidden')) {
        $details
            .toggleClass('active', true)
            .slideToggle();

        $detailsExpand
            .toggleClass('down', false)
            .toggleClass('up', true)
            .toggleClass('active', true);
    }
};

And then applying your new plugin would be as easy as calling the .toggleDetails method on your selected elements, and providing both an infoClass and expandClass.

$('.user .expandDetails a').toggleDetails({
    infoClass: '.userInfo',
    expandClass: '.expandUserDetails'
});

$('.user .paymentDetails a').toggleDetails({
    infoClass: '.paymentInfo',
    expandClass: '.expandPaymentDetails'
});

Caveats:

  • This code hasn't been tested.
  • I'm not sure how to assign a down or up to slideToggle. I would have expected a similar true/false parameter similar to toggleClass.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.