jquery bind click event in for loop

Question

I'm trying to bind click event in for loop but any of the remove buttons remove only the last list item. Do you have any ideas why this happens?

for ( var key in editedPredef.predefinition) {
      var listItem = $("<li></li>").addClass("ui-widget-content");
      listItem.text("some text");
      var removeListItem = $("<span class=\"ui-icon ui-icon-closethick\"></span>");
      removeListItem.click(function() {
        listItem.remove();
      });
      listItem.append(removeListItem);
      containerElement.append(listItem);
    }

[Javascript closure inside loops - simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — Andreas, May 27 '14 at 11:59
@C-link it doesn't matter. I'm using it to set text of list item. — newbieee, May 27 '14 at 12:01

score 5 · Accepted Answer · answered May 27 '14 at 11:58

5

Don't write that inside a for loop just make use of event-delegation,

$(document).on('click','.ui-icon.ui-icon-closethick',function(){
  $(this).closest('li').remove();
});

And note that place this code outside of that for loop.

answered May 27 '14 at 11:58

Rajaprabhu Aravindasamy

66,513
17
101
130

1

@AmitJoki Yeah my PM and TL went outside the office.. :) – Rajaprabhu Aravindasamy May 27 '14 at 12:00
PM and TL? Who are they? – Amit Joki May 27 '14 at 12:01
3

Project Manager and Team Leader ;) – T J May 27 '14 at 12:01
@AmitJoki: you are too young to know this bro ;) – Milind Anantwar May 27 '14 at 12:14
@MilindAnantwar, yeah.. maybe – Amit Joki May 27 '14 at 12:17

score 1 · Answer 2 · edited May 23 '17 at 12:20

Change

removeListItem.click(function() {
    listItem.remove();
  });

to

removeListItem.click(function() {
    $(this).parent('li').remove();
  });

Side Note: this will bind an event handler to each element. You can make use of event delegation like Rajaprabhu mentioned in his answer. However, delegated event handlers will be slower than directly binder event handlers. So it's a matter of how many such elements you are likely to have in your application. If you're going for delegation, it'll be better to bind the event handler to an immediate parent element, which is not dynamically added, so that the response will be faster. For e.g., you can bind the event handler to the parent <ul> if it's not dynamically added and won't be removed from page like

$('ul').on('click','.ui-icon.ui-icon-closethick',function(){
 $(this).parent('li').remove();
});

Harsh downvote. It wasn't me, although for the sake of someone leaving feedback it seems like it was because this could create *X* anonymous functions. One for each button. Event delegation would mean only creating and calling a single function — CodingIntrigue, May 27 '14 at 12:00
@RGraham right, didn't think of that, this is the first thing that came to my mind when i looked at the code… :) i'll mention delegation anyway. — T J, May 27 '14 at 12:04

jquery bind click event in for loop

2 Answers2