chairmanwow
chairmanwow

Reputation: 41

How to set onclick listener with JQuery

I'm trying to dynamically create a list images from a JSON array that will invoke a JavaScript function upon clicking one of the images. The function will send a request to the server, flagging that image. I currently have the following:

        var GLOBAL_POST_ID_LIST = [];
    function createList(jsonData) {
        for (var i = 0; i < jsonData.posts.length; i++) {
            var curPostId = jsonData.posts[i].post_id;

            // Check if the current post is in the list
            if ($.inArray(curPostId, window.GLOBAL_POST_ID_LIST) < 0) {
                jQuery('<img/>', {
                    id: curPostId,
                    src: jsonData.posts[i].image_url,
                    selected: false,
                    onclick: onPostClick(curPostId)
                }).appendTo('#postDiv');
                // At the current post to the list
                window.GLOBAL_POST_ID_LIST.push(curPostId);
            }
        }
    }

However, the onclick function is called immediately as the object is initialized, and not when the object is clicked on. How can I caused the onPostClick to be invoked with the correct post_id when any given post is initialized?

Upvotes: 1

Views: 2415

Answers (3)

bumbumpaw
bumbumpaw

Reputation: 2528

You can use this:

$("#yourId").click(function(){...});

Upvotes: 5

Barmar
Barmar

Reputation: 782693

You need to wrap it in a function:

onclick: function() { onPostClick(curPostId); }

Also, you need to get curPostId saved in a closure, otherwise you'll get the same value for all the elements (see Javascript infamous Loop issue?). So it should be:

onclick: (function(curPostId) {
            return function () { onPostClick(curPostId); };
         })(curPostId)

However, why do you need to pass the ID argument to onPostClick in the first place? jQuery automatically binds this to the event target in an event handler, so onPostClick should be able to use this.id to get the ID. If you fix the function to do that, you can just write:

onclick: onPostClick

You could also avoid this whole issue by giving your images a class and using event delegation. See Event binding on dynamically created elements?.

Upvotes: 5

Kevin Bowersox
Kevin Bowersox

Reputation: 94499

Invoke the function within an anonymous function:

  // Check if the current post is in the list
            if ($.inArray(curPostId, window.GLOBAL_POST_ID_LIST) < 0) {
                jQuery('<img/>', {
                    id: curPostId,
                    src: jsonData.posts[i].image_url,
                    selected: false,
                    onclick: function(){
                        onPostClick(curPostId);
                    }
                }).appendTo('#postDiv');
                // At the current post to the list
                window.GLOBAL_POST_ID_LIST.push(curPostId);
            }

This will prevent the onclick property from being assigned the result of invoking the onPostClick function.

Since you are binding this event handler within a loop you will need to create a closure to avoid getting stuck on the last value of the loop.

           jQuery('<img/>', {
                id: curPostId,
                src: jsonData.posts[i].image_url,
                selected: false,
                onclick: (function(id){
                    return function(){
                       onPostClick(id);
                    };
                })(curPostId)
            }).appendTo('#postDiv');

Upvotes: 3

Related Questions