Leaflet marker event fires at wrong time

I am working with leaflet (v 0.7.7) and I have an Ajax call that gets some server data to bind to my map in the form of clickable text labels. In the loop where I am binding the JSON data that I got from the server I have the following code:

var item_name = L.marker([data.X, data.Y],
{
    icon: L.divIcon(
    {
        className:'MapTag', 
        iconAnchor: [10, 10],
        html:'<img src="/Images/Map/Item' + data.Id + '.png">' + data.Name 
    })
}).on('click', onItemClick(data.Id));

item_layer.addLayer(item_name);

Elsewhere, I have the onItemClick code:

function onItemClick(item_id)
{
    alert(item_id);
}

Now, the good news, if I comment out the event binding part of that, the loop completes, and leaflet behaves as it should. However, when I bind the click event, things get odd. The event is fired once for each item in the collection I am binding to. When the data is loaded from the server, I get an alert popping up for each time through the loop with the current item's Id. It feels like it is being fired 'onload' rather than 'onclick'. To top it off, it also doesn't register clicks on the divIcons after loading.

There must be something I am missing here, but I cannot see what it is. Any suggestions?

THis question is similar to (Marker in leaflet, click event)

Resolution: I changed the last line of the divIcon declaration to:

}).on('click', function(e){ alert(data.Id); });

This now works as intended. I am guessing that the strange binding behavior is a result of not binding a defined method reference and leaflet having some sort of functional meltdown in its event management code.

I kept the 'e' argument in, since it is what the leaflet docs show. I am not using it, but if someone else copy-pastes this, they might need it.

Solution 1:

You're confusing the concepts of function references and function calls, and you are not making a closure over the item ID.

If you declare this:

function onItemClick(id) { alert(id); }

This prints the reference to the function:

console.log( onItemClick );

And this prints the return value of calling that function (as it returns nothing, this equals undefined):

console.log( onItemClick(5) );

So when you're doing this:

L.marker(....).on('click', onItemClick(id) );

the function gets called, and on() receives the return value of that function, i.e.:

L.marker(....).on('click', undefined );

What you want to do is have a function that returns a function:

function onItemClick(id) { return function(){ alert(id); } }

This way, when you do

L.marker(....).on('click', onItemClick(5) );

That will make a function call, and use the return value, which now looks like this:

L.marker(....).on('click', function() { alert(5); } );