0

Working on building this to-do list app to learn JS better.

I am able to insert text into box and have it add a new div element to the page with the relevant text residing within the div. The code that adds the div to the page automatically applies the class .newItem then adds an incrementing id i0, i1, i2, etc each time it's clicked. Everything works without an issue. Now, I have been fiddling around with the code to be able to click a single div element and have it remove itself from the page, but can't seem to get it to work.

var iDN = 0;
//Function that adds a new div with a custom incrementing id number
document.getElementById('add_button').onclick = function () {
    var taskName = document.getElementById('add_task').value; // Store the value in the textbox into a variable
    document.querySelector('.shit_to_do').innerHTML += '<div class = "newItem" id = "i' + iDN + '"' + 'onclick = removeEl()' + '>' + taskName + '</div>';
    iDN += 1;
};

document.getElementById('testing').onclick = function () {
    var parentNode = document.querySelector('.shit_to_do');
    parentNode.removeChild(parentNode.children[0]);
}

function removeEl() {
    for (i = 0; i < iDN; i++) {
        if (document.getElementById('i' + i).onclick) {
            document.getElementById('i' + i).display.style = 'none';
        }
        alert(i);
    }
}

The for loop was really some random option I was trying to figure out how things were working onclick for each div, but didn't get to far with that one.

tl;dr: I want to add click events to the new divs added onto the page in a single, universal function.

Paul S.
  • 64,864
  • 9
  • 122
  • 138
ryan
  • 625
  • 3
  • 10
  • 23

1 Answers1

1

The value of document.getElementById('i' + i).onclick will be null if you've not set a handler to this attribute/property, otherwise it will be a function. null is always falsy, a function is always truthy.

To remove your element, you'll either have to look at this or e.target where e is the click event, and then call the DOM method node.removeChild(child).

The "quick and dirty" solution is to pass this into removeEl and remove it that way,

// ...
    document.querySelector('.shit_to_do').innerHTML += '<div class="newItem" id="i' + iDN + '" onclick="removeEl(this)">' + taskName + '</div>';
// ...
function removeEl(elm) {
    elm.parentNode.removeChild(elm);
}

I also removed the strange spacing between attribute names and values in your HTML

A perhaps "cleaner" solution is to create your nodes and attach listeners all by using DOM methods

function createDiv(index, taskname) {
    var d = document.createElement('div');
    d.setAttribute('id', 'i' + index);
    d.textContent = taskname;
    return d;
}

function removeElm() {
    this.parentNode.removeChild(this);
}

var iDN = 0;
document.getElementById('add_button').addEventListener('click', function () {
    var taskName = document.getElementById('add_task').value,
        list = querySelector('.shit_to_do'),
        div = createDiv(iDN, taskName);
    div.addEventListener('click', removeElm);
    list.appendChild(div);
    iDN += 1;
});

This way means the browser does not re-parse any HTML as it not use element.innerHTML, which is a dangerous property may destroy references etc (along with usually being a bit slower)

Helpful links

Paul S.
  • 64,864
  • 9
  • 122
  • 138