AJAX jQuery .click Element running function on load instead of click

Question

Maybe i'm just a jQuery noob, but I don't understand why when I set an elements .click event it runs when the element loads.

Code:

//Deal Cards button
$(".draw-cards").click(function(){
    var playerId = 1;
    var gameId = 20;
    $.ajax({
        type: 'GET',
        url: './system/actions/draw.php',
        data: "playerId=" + playerId,
        success: function(data) {
            //Post to Player Card container
            $('#player-cards').html(data);
            //Resize Fonts
            fontSize();
            //For each loaded check if usable
            $( ".card" ).each(function() {
                var cardId = $(this).attr('id');
                comparePlayerCard(cardId, function(data) {
                    console.log(data);
                    if (data == 1){
                        $("#"+cardId+".card").css('box-shadow', '0px 0px 12px 6px #00ff40');
                        $("#"+cardId+".card").click(addCardToInventory(playerId, gameId, cardId)); // <---- PROBLEM CODE
                    }
                });
            });
        }
    });
});

The function inside

$("#"+cardId+".card").click(addCardToInventory(playerId, gameId, cardId));

Is being run when the element loads through the AJAX, then it doesn't work when I click the element. Not sure why.

Any suggestions?

addCardToInventory(playerId, gameId, cardId) will call the function directly instead of when clicked. Use anonymous function $("#" + cardId + ".card").click(function () { addCardToInventory(playerId, gameId, cardId) }); — Tushar
– Tushar, Commented Oct 13, 2015 at 3:20
Try calling your function in an anonymous function - $("#"+cardId+".card").click(function(){ addCardToInventory(playerId, gameId, cardId)}); — Guruprasad J Rao
– Guruprasad J Rao, Commented Oct 13, 2015 at 3:22
@GuruprasadRao No, no. Anonoymous function within a loop is a bad practice. — Yeldar Kurmangaliyev
– Yeldar Kurmangaliyev, Commented Oct 13, 2015 at 3:26
@YeldarKurmangaliyev thanks for sharing.. but if it is the requirement of OP, to pass it as argument without modifying function then there is no other approach right? — Guruprasad J Rao
– Guruprasad J Rao, Commented Oct 13, 2015 at 3:29
@GuruprasadRao Yeah, you are right. In this case, use anonymous function :) — Yeldar Kurmangaliyev
– Yeldar Kurmangaliyev, Commented Oct 13, 2015 at 3:30

Yeldar Kurmangaliyev · Accepted Answer · 2015-10-13 03:25:40Z

You shouldn't pass function call this way.

It should be an anonymous function:

$("#"+cardId+".card").click(function() {
    functionaddCardToInventory(playerId, gameId, cardId);
});

It is usually the most suitable approach, but it is a bad practice to use anonymous functions within a loop.

Or it can be a function without arguments:

$("#" + cardId + ".card").click(functionaddCardToInventory);

In this case, you can describe your values as a global values or within data attributes:

HTML:

<div id="cardId" class="card" data-game-id="7" data-card-id="11"></div>

JS:

var playedId = 14;    

function functionaddCardToInventory()
{
    var cardId = $(this).data("card-id");
    var gameId = $(this).data("game-id");
    // and playerId is global
}

// Function without arguments can be passed this way
$("#" + cardId + ".card").click(functionaddCardToInventory);

Collectives™ on Stack Overflow

AJAX jQuery .click Element running function on load instead of click

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related