Click function running on jquery.click?

Question

I'm trying to create a toggle function between showing and hiding elements. Here is my code:

$(document).ready(function() {
     (...) Code for forms (...)
     $("#hide").click( HideAll() );
}

function HideAll() {
    $("#course").hide();
    $("#students").hide();
    $("#hide").text("Show course");
    $("#hide").unbind().click(ShowAll());
}

function ShowAll() {
    $("#course").show();
    $("#students").show();
    $("#hide").text("Hide course");
    $("#hide").unbind().click(HideAll());
}  

With HTML structured like so:

<div id="hide">Hide courses</div>
<ul id="course">
    <li><div>CRN:</div><input type="text" id="crn" value="" size="5"/></li>
    <li><div>Prefix:</div><input type="text" id="pre" maxlength="4" value="" size="4"/></li>
    <li><div>Number:</div><input type="text" id="num" maxlength="4" value=""size="4" /></li>
    <li><div>Title:</div><input type="text" id="title" value="" /></li>
    <li><div>Section:</div><input type="text" id="sec" maxlength="2" value=""size="2" /></li>
    <li><div>Year:</div><input type="text" id="year" maxlength="4" value="" size="4"/></li>
</ul>

<div id="students">
    <ul class="student">
        <li><div>RIN:</div><input type="text" class="rin" maxlength="9" value="" size="9"/></li>
        <li><div>First Name:</div><input type="text" class="fname" value="" size="12"/></li>
        <li><div>Last Name:</div><input type="text" class="lname" value="" size="12"/></li>
        <li><div>Address line 1:</div><input type="text" class="ad1" value="" /></li>
        <li><div>Address line 2:</div><input type="text" class="ad2" value="" /></li>
        <li><div>City:</div><input type="text" class="city" value="" /></li>
        <li><div>State:</div><input type="text" class="st" maxlength="2" value="" size="2"/></li>
        <li><div>ZIP:</div><input type="text" class="zip" maxlength="5" value="" size="5"/><input type="text" class="zip4" maxlength="4" value="" size="4"/></li>
        <li><div>Grade:</div><input type="text" class="grade" maxlength="3" value="" size="3"/></li>
    </ul>
</div>

With break points in my script, it was revealed to me that my #hide's click function was being called on the line in the $(document).ready function, then it continuous loops without waiting for clicks. Is there either a problem with my implementation or an error with my code? I haven't quite figured it out.

In case it helps, here is a callstack of when #hide's click function is called:

DOMContentLoaded  
$.extend.ready
$.Callbacks.self.fireWith
$.Callbacks.fire
(anonymous function) 

Thanks for your time.

Answer 1

If I might offer a different recommendation:

$(document).ready(function() {
    var $hide = $('#hide'),
        $courseStudents = $('#course,#students'),
        hideShow = {
            show:true,
            apply:function() {
                var self = this;

                if(self.show){
                    self.show = false;
                    $courseStudents.hide();
                    $hide.text("Show course");
                } else {
                    self.show = true;
                    $courseStudents.show();
                    $hide.text("Hide course");
                }
            }
        };

    (...) Code for forms (...)

    $hide.on('click',hideShow.apply());
});

This consolidates your code into an object hideShow with a boolean show which will determine which series of events occur. On click of #hide, it will switch the boolean and apply the appropriate functions. This saves writing nearly identical code twice, and encapsulates everything so there isn't fragmentation of logic.

Answer 2

There's no need to bind/unbind click events each time you show and hide stuff. You can have the onclick function determine whether to show or hide:

$(document).ready(function () {
    /*(...) Code for forms (...)*/
    $("#hide").on('click', function(){
        if($("#course").is("visible")){
            HideAll;
        } else {
            ShowAll;
        }
    });
}    
function HideAll() {
    $("#course,#students").hide();
    $("#hide").text("Show course");
}

function ShowAll() {
    $("#course,#students").show();
    $("#hide").text("Hide course");
}

Answer 3

Use .one()

Attach a handler to an event for the elements. The handler is executed at most once per element per event type.

$(document).ready(function() {
    // (...) Code for forms (...)
     $("#hide").one('click', HideAll());
}

function HideAll() {
    $("#course").hide();
    $("#students").hide();
    $("#hide").text("Show course");
    $("#hide").one.('click',ShowAll());
}

function ShowAll() {
    $("#course").show();
    $("#students").show();
    $("#hide").text("Hide course");
    $("#hide").one('click', HideAll());
}

Answer 4

You need to pass the functions as references:

$(document).ready(function() {
     //(...) Code for forms (...)
     $("#hide").click(HideAll);
}

function HideAll() {
    $("#course").hide();
    $("#students").hide();
    $("#hide").text("Show course");
    $("#hide").unbind().click(ShowAll);
}

function ShowAll() {
    $("#course").show();
    $("#students").show();
    $("#hide").text("Hide course");
    $("#hide").unbind().click(HideAll);
}  

Your current code is immediately invoking each function and assigning the result to the respective click handler, which is ending up in a loop.

Answer 5

You need to pass the function references to click function

$("#hide").click( HideAll);

then

$("#hide").off('click').click(ShowAll);

But a better solution could be to use .one()

$(document).ready(function () {
    $("#hide").one('click', HideAll);
}

function HideAll() {
    $("#course").hide();
    $("#students").hide();
    $("#hide").text("Show course");
    $("#hide").one('click', ShowAll);
}

function ShowAll() {
    $("#course").show();
    $("#students").show();
    $("#hide").text("Hide course");
    $("#hide").one('click', HideAll);
}