Javascript Delete Entry Array Button OnClick

Question

I am currently trying to delete entries from my local storage list (array) with delete buttons next to each line. I made the function to delete it, and also the function that adds the button / each line in general. It looks like this:

function printData() {
 let output,
 i;
 let element;
 output = "";
 for(i=0; i<commentsData.length; i++) {
 // adding the delete button to each entry:
 element = "<em id='i'>"+commentsData[i].date+"</em> -- " +
 commentsData[i].contents + "<button id='delete' onclick='deleteEntry()'>Delete</button>" + "<br>";
 output += element;
 }
 $("#comments").html(output);
}

And this is my delete function:

function deleteEntry() {
 alert("Test");
 if (commentsData.length > 0) {
 commentsData.splice(commentsData.indexOf($(this)),1);
 }
 printData();
}

I also tried calling it in my init function:

 function init() {
 loadData();
 printData();
 $("#delete").click(deleteEntry);
}; 

But currently it is not working when i click the buttons. What could be the reason for that? I am not yet sure if my deleteEntry is working right now, but my problem is that the clicking of the buttons does not work. You dont need to fix my deleteEntry method - i will do that myself, i just need your help to get my buttons to work please, i would be very greatful! :)

• Adrian: Buttons must have unique ids for a start.
• Mark Schultheiss: Using <em element for emphasis is OK but perhaps better to style using CSS.
• Mark Schultheiss: Indeed deleteEntry has issues as you note, keeping the display and array or whatever comments.Data is in sync with each other will be a part of your final solution.

Answer 1

$("#delete").click(deleteEntry); would work for those elements which are already present on DOM but not for those which are being created dynamically.

You need to bind these elements and yes, as others mentioned, IDs should be unique hence you should use class.

$(document).on('click', '.myDelButton', function() {
 deleteEntry();
});

I think this should work, let me know if not.

• Mark Schultheiss: Best to also mention that you should attach the event handler to the closest possible parent element wrapper instead of docuement if possible.
• Tushar Shukla: Agreed! attaching to closest parent element wrapper is indeed a nicer/better way to do this.
• Mark Schultheiss: Also you can only execute printData() once as a second execution will propagate duplicate ID's in the markup. For a complete answer, address the issues within that function.

Answer 2

ids should always be unique. in your for loop :

 element = "<em id='"+i+"'>"+commentsData[i].date+"</em> -- " +
 commentsData[i].contents + "<button class='deleteBtn'>Delete</button>" + "<br>";

and in your init function :

$("body").on('click', '.deleteBtn', deleteEntry);

• Mark Schultheiss: I did not down vote but id='i'> please fix that also to not duplicate the id
• Mark Schultheiss: Also you can only execute printData() once as a second execution will propagate duplicate ID's in the markup.