$("#id").click(function(){calculation()});

Question

What is wrong with the line in the header?

The below example is supposed to make a button which will increment a counter each time it is clicked. However, I enforce a delay of 2000 ms between button clicks. The version below works, however, if I use the commented out line instead of

document.getElementById("rollButton").onclick=function(){calculation()};

(both in function afterWaiting())

I get various odd results, for instance that the counter starts incrementing by a lot more than 1, and the waiting time disappears?

<!DOCTYPE html>
<html>
    <head>

    <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.8.3/jquery.min.js"></script>
    <script>

        function afterWaiting()
        {
            $("#rollButton").css("color","black");
            //$("#rollButton").click(function(){calculation()});
            document.getElementById("rollButton").onclick=function(){calculation()};

        }

        var counter=0;
        function calculation()
        {

            ////Enforcing wait:
            document.getElementById("rollButton").style.color="red";
            document.getElementById("rollButton").onclick="";
            window.setTimeout("afterWaiting()",2000);


            counter=counter+1;
            document.getElementById("test").innerHTML=counter;

            }



    </script>

    </head>
<body>

  <button type="button" onclick="calculation()" id="rollButton"> Roll! </button>

<p id="test"> </p>


</body>
</html>

What have I misunderstood?

thanks in advance :)

JSFiddle: http://jsfiddle.net/Bwxb9/

I'm running it through the browser, which doesn't make any complaints? — Kaare, Feb 23 '13 at 13:39

score 3 · Answer 1 · answered Feb 23 '13 at 13:41

3

The difference is that when you apply event handlers through onclick as you do in your original version, you can only bind one handler to the element. And using onclick="" kind of clears it.

When using jQuery .click(handler) you bind a new handler each time you call it (and you can unbind it with unbind('click') (and not with onclick=""). So after a couple of calls to afterWaiting you have applied mulitple click handlers on your element, and on each click the calculation function runs multiple times..

So, one way to correct it is to replace

document.getElementById("rollButton").onclick="";

with

$('#rollButton').unbind('click');

answered Feb 23 '13 at 13:41

Gabriele Petrioli

191,379
34
261
317

`and not with onclick=""` -- are you sure? How is jQuery bound to that element? – John Dvorak Feb 23 '13 at 13:47
1

@JanDvorak it is bound using the `addEventListener` (*or `addEvent` of old IE*) which works in a different way (*read http://stackoverflow.com/questions/6348494/addeventlistener-vs-onclick*) – Gabriele Petrioli Feb 23 '13 at 13:55

score 2 · Answer 2 · answered Feb 23 '13 at 13:40

2

That's generally a bit of an odd and confusing approach. Here's how i'd do it, without mixing jquery and pure js (onclick) too much:

http://jsfiddle.net/LGvKS/

var wait = false;
counter = 0;
$('button').click(function(){
    if(!wait){
        $('span').text(++counter);
        wait=true;
        setTimeout(function(){
            wait=false;
        },2000);
    }
});

answered Feb 23 '13 at 13:40

Andy

29,707
9
41
58

well, I'm doing it this way because I'm learning jQuery, so trying to transition from the js version to a jQuery without breaking stuff. – Kaare Feb 23 '13 at 13:42
1

@Kaare Sure, nothing wrong with that. It sounded more harsh than i intended it to be. – Andy Feb 23 '13 at 13:44
1

I think what @Andy points out here (in his code) is that you don't need to remove the event handler each time. Just set a flag (or you could disable the button) for 2 secs. And that's what I would aslo do. – Onur Yıldırım Feb 23 '13 at 13:51

Arun P Johny · Accepted Answer · 2013-02-23T14:50:24.560

The only code required is

<button type="button" id="rollButton"> Roll! </button>
<p id="test"> </p>


var counter = 0;
var $test = $('#test');
var $rollButton = $('#rollButton');
function increment(){
    $test.html(counter++);
    $rollButton.off('click', increment);
    setTimeout(function(){
        $rollButton.on('click', increment);
    }, 2000);
}
$rollButton.on('click', increment);

Demo: Fiddle

Updated: as suggested by Andy, but I would recommend Andy's answer as it involves no additional event manipulation

var counter = 0;
var $test = $('#test');
var $rollButton = $('#rollButton');
function increment(){
    $test.html(counter++);
    setTimeout(function(){
        $rollButton.one('click', increment);
    }, 2000);
}
$rollButton.one('click', increment);

Demo: Fiddle

i that case you could also use .one() http://api.jquery.com/one/ instead of .on(). this way you don't have to call off(). — Andy, Feb 23 '13 at 14:46

$("#id").click(function(){calculation()});

3 Answers3