CODEWITHSUNDEEP
javascripthtml
devdropper87
devdropper87

Reputation: 4187

deleting an element with pure javascript

I am trying to delete an element in a ul (list item) like so using only javascript. since I am dynamically generating the html, I am trying to set the onclick as I create it below. However when I try to click (see fiddle) elements are not getting deleted. relevant functions below:

var appendHTML = function (el) {
    var list = document.getElementById("events");
    for (var i = 0; i < filteredDates.length; i++) {
        var listItem = filteredDates[i].name;
        listItem.className = "event-list";
        listItem.setAttribute("onclick", "deleteEvent()");
        list.appendChild(listItem);
    }
};


var deleteEvent = function () {
    var listItem = this.parentNode;
    var ul = listItem.parentNode;
    //Remove the parent list item from the ul
    ul.removeChild(listItem);
    return false;
};

https://jsfiddle.net/youngfreezy/7jLswj9b/5/

Upvotes: 0

Views: 110

Answers (3)

Felix Kling
Felix Kling

Reputation: 816422

There are a couple of things wrong with your jsFiddle and your code.

Scope of the event handler

You are using an inline event handler. Inline event handlers are executed in global scope. However, because the fiddle by default wraps your JavaScript code inside a function, deleteEvent is not global. The console shows the error:

Uncaught TypeError: Cannot read property 'appendChild' of null

There are three ways to solve this:

this inside the event handler

Because you are calling the function as deleteEvent(), this won't refer to the DOM element that triggered the event. It will refer to window, the global object, which doesn't have a parentNode property.

If you use a different way to bind the handler, as suggested above, then that problem might solve itself. Otherwise, you have to call deleteEvent in such a way that this refers to the element:

onclick="deleteEvent.call(this)"

Of course you could also change the definition of deleteEvent to accept the element as argument instead.

However, in your fiddle you actually attached the event handler to the <ul> element, not to each <li> element. In that case you have to pass the event object to function instead and get the original target from it:

onclick="deleteEvent(event)"

and

var deleteEvent = function (event) {
  //Remove the parent list item from the ul
  event.target.parentNode.removeChild(event);
}

DEMO

Wrong node reference

It seems you are deleting the wrong node in deleteEvent. listItem will actually be the <ul> element, because that's the parent of the clicked element. Consequently, ul will be the parent of the <ul> element.

Since this already refers to the <li> element, there is need for listItem here, and ul should be this.parentNode. In shorter form:

var deleteEvent = function () {
  //Remove the parent list item from the ul
  this.parentNode.removeChild(this);
}

return false has no effect

return false is often used to prevent the default action of the event. However, clicking on a li element has no default action. Furthermore, the value has to be return from the event handler. deleteEvent is not your event handler, it is called by your event handler. So if you want to return its return value, you have to do exactly that:

listItem.setAttribute("onclick", "return deleteEvent()");

But again, you should prefer a different of binding the event handler.

Invalid node creation

In the code in your question you are trying to call setAttribute on an arbitrary object and try to append an arbitrary object to a DOM element. That won't work. You have to use document.createElement to create a new element:

var li = document.createElement('li');

It seems you are doing this correctly in your fiddle.

Upvotes: 1

Leon Adler
Leon Adler

Reputation: 3341

If you use the onclick attribute to set event code as a string, the this context gets lost.

The best way to register event handlers is via addEventListener:

listItem.addEventListener("click", deleteEvent);

As a fallback, set your event code as a function instead:

listItem.onclick = deleteEvent;

This is discouraged as you can only set 1 event handler per event this way.

If you really really have to set the code Inline, pass the value of this via an argument:

<ul id="events" onclick="deleteEvent(this)">...</ul>

function deleteEvent(element) {
  // ...
}

Upvotes: 1

Saar
Saar

Reputation: 2306

Your first problem was that you had the "deleteEvent" in your html, jsFiddle doesn't like it.

second problem was that you took the wrong element in the deleteEvent so the parent was incorrect as well.

See updated fiddle: https://jsfiddle.net/7jLswj9b/7/

    var deleteEvent = function (ev) {
      var listItem = ev.target;
      var ul = listItem.parentNode;
      //Remove the parent list item from the ul
      ul.removeChild(listItem);
        return false;
    }

 var list = document.getElementById("events");
list.addEventListener("click",deleteEvent,false);

Upvotes: 5

Related Questions