0

I have a menu class loading data from a received json file.

in the constructor I build the menu, so I have a for loop with this (extracted part) js:

for (var i = 0; i<data.length; i++)
    {
        var btn = $('<div>'+data[i].label+'</div>').appendTo(object.container);
        btn.click(function()
            {  
                if($('.blockingFrame').length == 0)//pas de blocking
                {
                    data[i].action()
                }
            });
    }

Now, obviously this doesn't work because on runtime data[i] doesn't exist anymore... data[i].action contains a valid js function.

This works, but doesn't contains the condition..:

for (var i = 0; i<data.length; i++)
    {
        var btn = $('<div>'+data[i].label+'</div>').appendTo(object.container);
        btn.click(data[i].action);
    }

So I thought I could store this action inside the jquery object and call it like this, but it doesn't work:

for (var i = 0; i<data.length; i++)
    {
        var btn = $('<div>'+data[i].label+'</div>').appendTo(object.container);
        btn.action = data[i].action;
        btn.click(function()
            {  
                if($('.blockingFrame').length == 0)//pas de blocking
                {
                    $(this).action();
                }
            });
    }

A partial solution I came u with, was to store the action in another event, like dblclick, and trigger a dblclick inside the condition, but this seams ugly.

Any idea how to do this?

Vincent Duprez
  • 3,772
  • 8
  • 36
  • 76
  • 1
    About your attempt to create a method on the jQuery wrapper: jQuery doesn't use something like a lookup table to return objects of previously matched selectors, obviously because the DOM could change anytime and these wrappers would be obsolete then. So when you call `$(this)`, a new jQuery wrapper is created that does not have the `action` method set. Instead you should use `btn.data('action', data[i].action);` and call it later with `$(this).data('action')();` – Kiruse Oct 05 '13 at 09:11

2 Answers2

1

Use an immediately-executing function to create a closure that holds i.

for (var i = 0; i<data.length; i++) {
    var btn = $('<div>'+data[i].label+'</div>').appendTo(object.container);
    btn.click(function(i) {
        return function() {  
            if($('.blockingFrame').length == 0)//pas de blocking {
                data[i].action();
        }
    }(i));
}
Barmar
  • 741,623
  • 53
  • 500
  • 612
1

for loops don't work properly with closures. Consider using iterator methods instead:

$.each(data, function(index, elem) {
    var btn = $('<div>'+elem.label+'</div>').appendTo(object.container);
    btn.click(function()
        {  
            if($('.blockingFrame').length == 0)//pas de blocking
            {
                elem.action()
            }
        });
}

Iterators are usually more elegant and compact than for loops, especially when you have them nested.

The reason why your last snippet doesn't work if that btn = $(...) is a temporary jquery object and disappears once you leave the scope, with everything you have assigned to it. When later a click handler is being invoked, you create a new jquery object via $(this), which doesn't carry your changes from the previous step. If you want to keep any data attached permanently to an element, use the data method - but in this case there's no need for that.

Community
  • 1
  • 1
georg
  • 211,518
  • 52
  • 313
  • 390