Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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);
    });
});
share|improve this question
add comment

3 Answers

up vote 7 down vote accepted

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.

share|improve this answer
add comment

maybe this one too ?

// My animation
function myAnimation(cont, h, d){
    $(cont).stop().delay(d).animate({ "height" : h+'px'}, 700);
}

// Handlers
$(function() {
    $('#clientLoginText').mouseover(myAnimation('#clientLoginText', '80', 0));
    $('#clientLoginBox').mouseout(myAnimation('#clientLoginBox', '0', 500));
});
share|improve this answer
    
Nice idea. Note though that the original only had a .delay() in one of the animations. –  nnnnnn Jan 28 '12 at 3:44
    
This is right. Sry for the mistake. –  Kursion Jan 28 '12 at 12:16
add comment

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().

share|improve this answer
    
In the original question there are two separate elements for enter and leave so this doesn't work. –  Paul Spencer Jan 30 '13 at 14:31
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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