[Solved] How do i condense this jQuery down so it’s more efficient?

by


The big thing you can do is avoid running the same query twice, if you know the results haven’t changed. Chain or cache the results of your jQuery calls! Instead of:

$('#button-active').hide();
$('#button-active').delay(30).fadeIn(1000);

you can use the chainability of jQuery objects. In fact, you’re already doing it in the second line–why not take the extra step?

$('#button-active').hide().delay(30).fadeIn(1000);

For readability:

$('#button-active')
    .hide()
    .delay(30)
    .fadeIn(1000);

But that’s not the whole answer. Chaining is concise and great, if you need to do a bunch of stuff to one object or collection in sequence. But if you need to access one object, like button-active, at several different points during execution, you should store the query as a variable.

var $activeButton = $('#button-active'); // a common convention is to use the $ prefix on variables storing jQuery objects, but this is arguable & optional

All together, with a few other efficiency tricks:

;$(document).ready(function() {
    var   inactiveClass     = "inactive"
        , buttonClass       = "button"
        , fadeDuration      = 1000
        , animateDuration   = 500
        , shortDelay        = 30
        , longDelay         = 800

        , loginInactiveButtonAnimateTarget = {
            marginTop: -40
        }
        , inactiveButtonAnimateTarget = {
            opacity: 0
        }

        , loginButtonClickHandler = function() {
            document.forms["LoginForm"].submit() // not modifying this, but if this is the "TopLoginForm" then you could do this simpler
        }

        , $activeButton     = $('#button-active').hide() /* the .hide() method chains, so it returns the button-active object */
        , $loginButton      = $('#LoginButton')
        , $topLoginForm     = $('#TopLoginForm')
        , $loginWelcome     = $('#LoginWelcome')
        , $inactiveButton   = $('#button-inactive')

    if ($loginButton.hasClass(inactiveClass)) {
        $topLoginForm.hide()
    } else {
        $topLoginForm.show()
    }
    $loginButton.click(function() {
        if ($loginButton.hasClass(inactiveClass)) {
            $loginButton.animate(loginnactiveButtonAnimateTarget, animateDuration)
            $inactiveButton.animate(inactiveButtonAnimateTarget, animateDuration).remove()
            $activeButton.hide().delay(shortDelay).fadeIn(fadeDuration)
            $loginWelcome.delay(0).fadeOut(fadeDuration)
            $loginForm.delay(longDelay).fadeIn(fadeDuration)
            $loginButton.addClass(buttonClass).click(loginButtonClickHandler)
        }
    })
});

0

solved How do i condense this jQuery down so it’s more efficient?