They said javascript code in HTML considered bad practice, how to solve this?

Question

So, I understand that mixing javascript into HTML is bad, even in head. But lets see this code (I want to remove items the from list)

.
.
.
<div>item1 <a href="#" onclick="return deleteThisItemById (1);">delete me</a><br />
<div>item4 <a href="#" onclick="return deleteThisItemById (4);">delete me</a><br />
<div>item5 <a href="#" onclick="return deleteThisItemById (5);">delete me</a><br />
.
.
.

This list is generated by PHP. deleteThisItemById() is in an external .js file. Is this still considered bad? If so, what would be a canonical solution? jQuery is acceptable, as is raw javascript.

Answer 1

$(function() {
  $('a#my_id1').click(function() {
    var myId = $(this).attr('id');
    return deleteThisItemByID(myId);
  });
});

<div>
    item1 
    <a id="my_id1" href="#">delete me</a>
</div>

By the way, your HTML is invalid. You have improperly nested div tags.

Answer 2

Simple... give the item you want removed a class or id (only use an id if its only one item)

<div>item1 <a href="#" class='deleteMe'>delete me</a><br /></div>
<div>item4 <a href="#" class='deleteMe'>delete me</a><br /></div>
<div>item5 <a href="#" class='deleteMe'>delete me</a><br /></div>

Then target the elements with jQuery

$('.deleteMe').on('click', function(event) {
    $(this).remove();
});

Note... if you wanted to remove the entire div, use this selector:

$('.deleteMe').closest('div').on('click', function(event) {
    $(this).remove();
});

Answer 3

you could use addEventListener to do this. Then no js-code will be in your html:

var elems = document.querySelectorAll("div.removable");
for(var i=0;i<elems.length;i++){      
    elems[i].querySelector("a").addEventListener("click",function(e){
        var toRemove = this.parentNode;
        var parent = toRemove.parentNode;
        parent.removeChild(toRemove);
    },false);
}

however, I iterate over the divs, so I added a class for this:

<div>
    <div  class="removable">item1 <a href="#">delete me</a></div><br />
    <div  class="removable">item2 <a href="#">delete me</a></div><br />
    <div  class="removable">item2 <a href="#">delete me</a></div><br />
</div>

Have a look at this fiddle

Answer 4

I like to stash id's and similar metadata in "data-" attributes on the dom elements that represent those objects.

In this case I will usually use a similar setup

<ul class="posts">
  <li class="post" data-post="####">
    <p>content...</p>
    <ul class="actions">
      <li>
        <a class="delete">delete</a>
      </li>
    </ul>
  </li>
</ul>

Then in an .js file included at the bottom of the page:

$('.posts').on('click', '.actions .delete',
function() {
  // which post should be deleted?
  var $post = $(this).closest('.post');
  // remove the DOM element
  $post.remove();
  // some sort of ajax operation
  $.ajax({
    // url, dataType, etc
    data: { post_id: $post.data('id') }
  });
});

This one handler will apply to all ".action .delete" elements in the main list, meaning that if you want to add a loader or pagination that uses ajax to modify or add times to the list, the one event handler will still work wonderfully.