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

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.