JQuery onclick firing constantly before the element is clicked

Question

I've looked at more than a dozen questions about using onclick, click, bind("click", ...), on("click", ...), etc. and have yet to find the issue that I'm having.

Basically it's an expandable div that hides some content when not expanded. I'm using the Twitter Bootstrap collapse class with a data-toggle button to expand/collapse the content itself, but I also need to modify the CSS of the container div to increase the height so that visually, the box the content is in will stretch to contain it.

Here's my script code:

$(document).ready(function() {
    $("#expand-button").bind('click', expandClick($));
    document.getElementById("#expand-button").bind("click", expandClick($));
});

function expandClick($) {
    $("#outer-container").animate({ "height": "350" }, 500);
    $("#expand-button").html("^");
    $("#expand-button").bind("click", collapseClick($));
};

function collapseClick($) {
    $("#outer-container").animate({ "height": "50" }, 500);
    $("#expand-button").html("V");
    $("#expand-button").bind("click", expandClick($));
}

The idea is simply that the handler rotates in and out depending on the state of the button. What's actually happening is that as soon as I load the page, the the expandClick function is immediately executed, which sets off an infinite loop of my container bouncing up and down despite nothing being clicked.

Any ideas?

Also, I don't think it should be relevant, but the HTML looks like:

    <div id="outer-container" class="container-fluid subsession-collapsed">
        <div class="row-fluid" style="height: 50px">
            <!-- OTHER STUFF... -->
            <div class="span1" id="4">
                <button id="expand-button" class="btn-success" data-toggle="collapse" data-target="#expandable">V</button>
            </div>
        </div>
        <br /><br />
        <div id="expandable" class="row-fluid collapse">
            <div class="span12" style="padding: 0 20px">
                <!-- CONTENT -->
            </div>
        </div>
    </div>

Edit:

One SO topic I've used to try and find a solution is this one, but all of the responses have given the same result.

Ruan Mendes · Accepted Answer · 2012-10-30T18:59:31.453

This statement is assigning the result of expandClick as the handler, that is

$("#expand-button").bind('click', expandClick($));

should be

$("#expand-button").bind('click', function() { expandClick($) });

The other problem is that you're adding more click handlers from expandClick and collapseClick but never removing them

This is what I would rewrite your code to be, I don't know why you are passing $ around

$(document).ready(function() {
    // Cache your variables instead of looking them up every time
    var expandButton =  $("#expand-button"),
        outerContainer =  $("#outer-container");

    function expandClick() {
        outerContainer.animate({ "height": "350" }, 500);
        expandButton.html("^");
        // Remove the previous handler
        expandButton.off('click', expandClick );
        // Bind the new handler
        expandButton.bind("click", collapseClick);
    };

    function collapseClick() {
       outerContainer.animate({ "height": "50" }, 500);
       expandButton.html("V");
        // Remove the previous handler
       expandButton.off('click', collapseClick);
        // Bind the new handler
       expandButton.bind("click", expandClick);
    }

    expandButton.bind('click', expandClick);
    // What is this????
    //document.getElementById("#expand-button").bind("click", expandClick($));
});

or simply `$("#expand-button").bind('click', expandClick);` based on `$(document)` showing that `$` is `jQuery` globally, and should mention that this change needs to happen to every .bind in the OP code. — Kevin B, Oct 30 '12 at 18:53
@KevinB I didn't even realize `$` was jQuery, let me suggest an improvement for that too, great catch — Ruan Mendes, Oct 30 '12 at 18:55
This and @Nelson's answer combined did the trick! Thanks guys! — mshubert12, Oct 30 '12 at 19:00
Oh, well Nelson deleted his. Ok then, thanks to @JuanMendes and @KevinB! — mshubert12, Oct 30 '12 at 19:01

VicoMan · Answer 2 · 2012-10-30T20:31:13.627

-1

Intead of using:

$(document).ready(function() {
    $("#expand-button").bind('click', expandClick($));
    document.getElementById("#expand-button").bind("click", expandClick());
});

Try

<button id="expand-button" class="btn-success"
data-toggle="collapse" data-target="#expandable" 
onclick="expandClick($)">V</button>

also

function expandClick($) {
    $("#outer-container").animate({ "height": "350" }, 500);
    $("#expand-button").html("^");
    $("#expand-button").unbind("click", expandClick());
    $("#expand-button").bind("click", collapseClick());
};

function collapseClick($) {
    $("#outer-container").animate({ "height": "50" }, 500);
    $("#expand-button").html("V");
    $("#expand-button").unbind("click", collapseClick());
    $("#expand-button").bind("click", expandClick());
}

edited Oct 30 '12 at 20:31

answered Oct 30 '12 at 18:52

VicoMan

289
2
13

1

Why would you suggest JS embedded in the HTML? – Ruan Mendes Oct 30 '12 at 18:52
1

Yes it will have the same bad behavior... Still gets my downvote, you're taking us back to the dark ages of web programming, there are plenty of nice ways to fix the problem. By the way, your suggestion doesn't work, the OP wants to button to expand and collapse – Ruan Mendes Oct 30 '12 at 19:03
Now it does ... maybe... or maybe not, it's still calling `collapseClick($)` instead of assigning is as the handler. Considering there's already an accepted answer, unless you have a better answer, I would consider deleting yours since when you do get it working it will be almost exactly like mine. – Ruan Mendes Oct 30 '12 at 20:25

JQuery onclick firing constantly before the element is clicked

2 Answers2