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);
    }

@C-link it doesn't matter. I'm using it to set text of list item. — newbieee
– newbieee, Commented May 27, 2014 at 12:01

Rajaprabhu Aravindasamy · Accepted Answer · 2014-05-27 11:58:37Z

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, 2014 at 11:58

Rajaprabhu Aravindasamy

67.2k17 gold badges107 silver badges133 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Rajaprabhu Aravindasamy Over a year ago

@AmitJoki Yeah my PM and TL went outside the office.. :)

Amit Joki Over a year ago

PM and TL? Who are they?

T J Over a year ago

Project Manager and Team Leader ;)

Milind Anantwar Over a year ago

@AmitJoki: you are too young to know this bro ;)

Amit Joki Over a year ago

@MilindAnantwar, yeah.. maybe

Community · Accepted Answer · 2017-05-23 12:20:55Z

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
@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.

Collectives™ on Stack Overflow

jquery bind click event in for loop

2 Answers 2

5 Comments

2 Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

2 Comments

Linked

Related