Trouble with to-do list close button

Question

I have some code that seems to be working, but in a rather odd fashion. When I first refresh the page I have the close button that seems to work fine, but when I make a new to-do list item the close button seems to cease working and I can't pinpoint why.

let addItem = document.getElementById("submitButton");
let userInput = document.getElementById("toDoInput");
let output = document.getElementById("output");
let toDoItem = document.querySelector(".toDoItem");
let close = document.querySelector(".close");
let toDo = document.querySelector(".todo");

/*User clicked the addItem Button
If there is any text inside the text field then add that text to the todo list */
addItem.addEventListener("click", addToDo);
function addToDo(){
  var html = `
  <ul class="todo">
      <li class="toDoItem">
        <p>${userInput.value}</p>
        <div class="close">X</div>
      </li>
  </ul>
  `;
output.innerHTML += html;
// Resetting input to blank once a submit button has been added.
userInput.value = '';
}


// Figure out how to make closing functionality simple this implementation
// isn't working
close.addEventListener("click", function(e){
  console.log("clicked");
  let x = e.target.parentElement;
  x.style.display = "none";
  e.preventDefault();
});

<header>
    <input type="text" placeholder="Enter Item Here..." id="toDoInput">
    <button id="submitButton">+</button>
</header>

<section id="output">
    <ul class="todo">
        <li class="toDoItem">
          <p>Clean Room!</p>
          <div class="close">X</div>
        </li>
    </ul>
</section>
<script src="todo.js"></script>

I'm also not sure if I'm using best practice as I'm new to web development, so any tips would be thoroughly appreciated as well!

You're building the `.close` elements dynamically, so you need to make use of `event delegation`. — Obsidian Age, Mar 13 '18 at 00:13

score 1 · Answer 1 · answered Mar 13 '18 at 00:19

1

You need a live event handler on your close button(s). This example should help. To offer something more, it's easier and more straight forward to use jQuery for it if you can and don't mind using a JS library.

jQuery example:

$(document).on("click", ".close", function() {
    $(this).parent().hide();
});

No need to prevent default behavior since it's a div.

answered Mar 13 '18 at 00:19

BSK

733
2
7
13

This answer is in JQuery so remember to place it OUTSIDE `document.ready` – suchislife Mar 13 '18 at 00:45

score 0 · Answer 2 · answered Mar 13 '18 at 00:12

0

The issue here is that when you re-render the content of the "output" section you lose the event listener bound to the original ".close" element. A few options to work around the issue, have a look at this thread for some examples.

answered Mar 13 '18 at 00:12

Georgy Malanichev

328
1
6

Exactly, it cannot listen to what wasn't there. – Justin Mar 13 '18 at 01:40

score 0 · Answer 3 · answered Mar 13 '18 at 00:40

You got pretty close man, and you definitely do not need jQuery. As you can see below, you don't need to push the <ul> dynamically. It will never change!

<header>

    <input type="text" placeholder="Enter Item Here..." id="toDoInput">
    <button id="submitButton">+</button>

</header>

<section id="output">
    <ul class="todo">
    </ul>
</section>

And here is your refactored javascript:

let addItem = document.getElementById("submitButton");
let userInput = document.getElementById("toDoInput");
let output = document.getElementById("output");
let toDoItem = document.querySelector(".toDoItem");
let toDo = document.querySelector(".todo");

/*User clicked the addItem Button
If there is any text inside the text field then add that text to the todo 
list */
addItem.addEventListener("click", addToDo);

function addToDo(e){
    e.preventDefault();
    var html = `<li class="toDoItem">
                <p>${userInput.value} </p> <p class="close" 
                onclick="removeChildElement(this);">X</p>
                </li>`;
   output.innerHTML += html;
   let close = document.querySelector(".close")
    // Resetting input to blank once a submit button has been added.
    userInput.value = '';
}


// Figure out how to make closing functionality simple this implementation
// isn't working
function removeChildElement(e) {
    let x = e.parentElement;
    let xParent = x.parentElement;
    xParent.removeChild(x);
    console.log(xParent);
}

As you can see i made a few changes. Most importantly your close button issue. The function gets the parent on its parent ( ^ 2 ) and then removes its child. Which would be your <li> element!

Enjoy the fiddle: https://jsfiddle.net/fjbyy6uw/35/

zer00ne · Answer 4 · 2018-03-13T01:35:13.250

Use Event Delegation. Details are commented in Demo. Added a <form> so HTMLFormControlsCollection API can be used, it's simpler, less writing, and I'm lazy.

/* All form controls are referenced by HTMLFormControlsCollection */
var form = document.forms.toDo;
var td = form.elements;
var add = td.add;
var inp = td.input;
var out = td.output;
var toDo = document.querySelector('.toDo');

add.addEventListener("click", addToDo);

/* Limited the dynamically created node to `<li>`. It doesn't make sense to
|| have several `<ul>` having only one `<li>` each.
*/
function addToDo() {
  var html = `
       <li class="item">
        <span>${inp.value}</span>
        <b class="close">X</b>
      </li>
  `;
  toDo.innerHTML += html;

}
/* Event Delegation is a way of leveraging event bubbling so
|| that a single ancestor node can be registered to listen for
|| an event (e.currentTarget) and by means event propagation
|| (bubbling) can locate the event origin (node clicked/e.target).
|| In this demo e.currentTarget is output#output and e.target are
|| any b.close. This was possibble by using e.target in conditions
*/
/* removeChild() is used because display:none is not entirely 
|| gone. The markup remains just not in the DOM, so it may not
|| look like it's there, under certain conditions a node could be
|| considered present.
*/
out.addEventListener("click", function(e) {
  if (e.target !== e.currentTarget) {
    if (e.target.className === "close") {
      let x = e.target.parentElement
      x.parentElement.removeChild(x);
    }
  }
});

.item {
  display: flex;
  max-width: 250px;
  justify-content: space-between;
}

.item span,
.item b {
  display: table-cell;
}

.item b {
  cursor: pointer
}

input,
output,
button {
  font: inherit
}

<form id='toDo'>
  <header>
    <input type="text" placeholder="Enter Item Here..." id="input">
    <button id="add" type='button'>+</button>
  </header>

  <output id="output">
    <ul class="toDo">
        <li class="item">
          <span>Clean Room!</span>
          <b class="close">X</b>
        </li>
    </ul>
</output>

</form>

Trouble with to-do list close button

4 Answers4