Animating login screens

Question

I know there is probably a better way to have coded this. Can anyone be of help? I am not an expert JavaScript coder.

$(function() {
    $('#clientLoginText').hover(function() {
        $('#clientLoginBox').stop().animate({ "height" : '80px'}, 700);
    }, function() {
        //$('#clientLoginBox').stop().delay(500).animate({ "height" : '0px'}, 700);
    });
});

$(function() {
    $('#clientLoginBox').hover(function() {
    }, function() {
        $('#clientLoginBox').stop().delay(500).animate({ "height" : '0px'}, 700);
    });
});

Answer 1

Well here is a tidied up version that shouldn't change the functionality at all:

$(function() {
    $('#clientLoginText').mouseenter(function() {
        $('#clientLoginBox').stop().animate({ "height" : '80px'}, 700);
    });

    $('#clientLoginBox').mouseleave(function() {
        $(this).stop().delay(500).animate({ "height" : '0px'}, 700);
    });
});

About the changes:

• You only need a single document.ready function: the only good reason I can think of for having two is if you need to put them in separate JS files.

• You had used the .hover() function, which is shorthand for .mouseenter() and .mouseleave(), but in the first use you passed an empty function for the mouseleave part and in the second use you passed an empty function for the mouseenter part, so why not just code the first directly as .mouseenter() and the second as .mouseleave()?

• Within an event handler you can refer to the element that triggered the event as this, or $(this) if you need it as a jQuery object. More efficient than getting jQuery to find it again from its id.

Answer 2

There's no reason to separate the mouseenter and mouseleave jQuery events into different calls. Since it's the same element, just use:

$(function() {
    $('#clientLoginBox').mouseenter(function() {
        $(this).stop().animate({"height": '80px'}, 700);
    }).mouseleave(function() {
        $(this).stop().delay(500).animate({"height": '0px'}, 700);
    });
});

It's not that big of a deal to have them separate, but it is an extra call to select the element. Combining them the way directly above makes use of chaining supported in jQuery.

If you wanted to keep them physically separate but not need two calls, you could use:

$(function() {
    var the_box = $('#clientLoginBox').mouseenter(function() {
        $(this).stop().animate({"height": '80px'}, 700);
    });

    the_box.mouseleave(function() {
        $(this).stop().delay(500).animate({"height": '0px'}, 700);
    });
});

Since you want to handle both events, I think using .hover makes more sense, just combine them better:

$(function() {
    $('#clientLoginBox').hover(function() {
        $(this).stop().animate({"height": '80px'}, 700);
    }, function() {
        $(this).stop().delay(500).animate({"height": '0px'}, 700);
    });
});

.hover() is just a shortcut for binding mouseenter and mouseleave in that order. If you wanted to only bind mouseenter, you could pass a single function to .hover().