3

I was creating a todo's list with JS. I have attached the code snippet for the same. I Have three hard coded todos in html and with each todo there are two buttons('x' to remove the todo 'y' to mark it as done) associated. Now for this hard coded todos everything is working fine. Now, to add a new todo I have this tag where todo text is entered and after clicking enter I am embedding them to html using innerHTML, while embedding the dynamically added todos are visible in DOM. But The problem is that the buttons('x' and 'y') associated with the todo is not working as expected. Further I tried to debug my problem and I found out that the buttons('x' and 'y') that are not getting selected. I could not understand why is this happening. Any help will be highly appreciated.

const todoInp = document.querySelector('.add input')
const todos = document.querySelector('.todos')


const remove = document.querySelectorAll('.remove')

const doneBtns = document.querySelectorAll('.check')

todoInp.addEventListener('keyup', (e) => {

  if (e.key === 'Enter' && e.target.value !== '') {
    const todoVal = e.target.value
    const todo = document.createElement('div')
    todo.classList.add('todo')

    todo.innerHTML = `
        <p>${todoVal}</p>
        <div class="buttons">
          <button class="remove">x</button>
          <button class="check">y</button>
        </div>
    `

    todos.appendChild(todo)

    todoInp.value = ''
  }
})






doneBtns.forEach((btn) => {
  btn.addEventListener('click', () => {
    const mainPar = btn.parentElement.parentElement
    mainPar.classList.toggle('done')
  })
})

remove.forEach((btn) => {
  btn.addEventListener('click', () => {
    const parent = btn.parentElement
    const mainPar = parent.parentElement
    mainPar.remove()
  })
})
* {
    box-sizing: border-box;
}
body {
    display: flex;
    justify-content: center;
    align-items: center;
    flex-direction: column;
    margin: 0;
    padding: 0;
    margin-top: 50px;
    height: 100vh;
    background-color: aliceblue;
}

.container{
    width: 600px;
    height: auto;
    padding: 50px;
}

.add,
.todos{
    display: flex;
    flex-direction: column;
    align-items: flex-start;
}
.add label{
    font-family: 'Courier New', Courier, monospace;
    font-weight: bold;
    color: darkblue;
}
.add input{
    background: white;
    padding: 13px;
    border: 0.5px solid #ccc;
    width: 90%;
    border-radius: 4px;
}

.add input:focus{
    outline: none;
}

.add input::placeholder{
    color:#ccc
}

.todo{
    color: darkblue;
    width: 90%;
    display: flex;
    justify-content: space-between;
    align-items: center;
    border-bottom: 1px solid #ccc;
}
.todo:last-child{
    border: 0
}

.todo p{
    font-size: 24px;
    font-weight: bold;
}

.buttons button{
    background-color: transparent;
    color: #aaa;
    border: 0;
    font-size: 24px;
    cursor: pointer;
    transition: all .5s ease-in-out;
}

.buttons button:hover{
    color:black;
}
.todo.done p{
    text-decoration: line-through;
    text-decoration-thickness: 2px;
    color: orangered;
    text-decoration-style:double;
}
<div class="container">
  <div class="add">
    <label>ADD TODO</label>
    <input type="text" placeholder="Write your To do">
  </div>
  <div class="todos">
    <div class="todo">
      <p>Todo 1</p>
      <div class="buttons">
        <button class="remove">x</button>
        <button class="check">y</button>
      </div>
    </div>
    <div class="todo">
      <p>Todo 2</p>
      <div class="buttons">
        <button class="remove">x</button>
        <button class="check">y</button>
      </div>
    </div>
    <div class="todo">
      <p>Todo 3</p>
      <div class="buttons">
        <button class="remove">x</button>
        <button class="check">y</button>
      </div>
    </div>
  </div>
</div>
Uchiha Itachi
  • 119
  • 9
  • 4
    A good solution to these kinds of problems is the use of [event delegation](https://stackoverflow.com/questions/1687296/what-is-dom-event-delegation). The reason your X and Y buttons are not working is because the `querySelectorAll` has already run before the new buttons exist, hence the `addEventListener` could not be added. – Reyno Jun 14 '21 at 07:26
  • Ooh right @Reyno, so I will be able to solve my problem using Event Delegation right? – Uchiha Itachi Jun 14 '21 at 07:32
  • 1
    That's correct! If you can't figure out event delegation just comeback and update your question with your attempt(s) and we help you from there. Success! – Reyno Jun 14 '21 at 07:35
  • yea sure will do, thank you so much @Reyno – Uchiha Itachi Jun 14 '21 at 07:36
  • 1
    Ugly but it works, unbind the buttons and bind them again each time you add a todo item. – Grumpy Jun 14 '21 at 08:11

3 Answers3

2

While delegation is a perfectly fine solution, I tend to take an (in my opionion) simpler approach, and simply bind the handlers to the new elements:

const todoInp = document.querySelector('.add input')
const todos = document.querySelector('.todos')


const remove = document.querySelectorAll('.remove')

const doneBtns = document.querySelectorAll('.check')

todoInp.addEventListener('keyup', (e) => {

  if (e.key === 'Enter' && e.target.value !== '') {
    const todoVal = e.target.value
    const todo = document.createElement('div')
    todo.classList.add('todo')

    todo.innerHTML = `
        <p>${todoVal}</p>
        <div class="buttons">
          <button class="remove">x</button>
          <button class="check">y</button>
        </div>
    `

    todos.appendChild(todo)
    bindRemove(todo.querySelector('.remove'));
    bindDone(todo.querySelector('.check'));
    todoInp.value = ''
  }
})

const bindDone = (btn) => {
  btn.addEventListener('click', () => {
    const mainPar = btn.parentElement.parentElement
    mainPar.classList.toggle('done')
  })
}

const bindRemove = (btn) => {
 btn.addEventListener('click', () => {
    const parent = btn.parentElement
    const mainPar = parent.parentElement
    mainPar.remove()
  })
}

doneBtns.forEach(bindDone)

remove.forEach(bindRemove)
* {
    box-sizing: border-box;
}
body {
    display: flex;
    justify-content: center;
    align-items: center;
    flex-direction: column;
    margin: 0;
    padding: 0;
    margin-top: 50px;
    height: 100vh;
    background-color: aliceblue;
}

.container{
    width: 600px;
    height: auto;
    padding: 50px;
}

.add,
.todos{
    display: flex;
    flex-direction: column;
    align-items: flex-start;
}
.add label{
    font-family: 'Courier New', Courier, monospace;
    font-weight: bold;
    color: darkblue;
}
.add input{
    background: white;
    padding: 13px;
    border: 0.5px solid #ccc;
    width: 90%;
    border-radius: 4px;
}

.add input:focus{
    outline: none;
}

.add input::placeholder{
    color:#ccc
}

.todo{
    color: darkblue;
    width: 90%;
    display: flex;
    justify-content: space-between;
    align-items: center;
    border-bottom: 1px solid #ccc;
}
.todo:last-child{
    border: 0
}

.todo p{
    font-size: 24px;
    font-weight: bold;
}

.buttons button{
    background-color: transparent;
    color: #aaa;
    border: 0;
    font-size: 24px;
    cursor: pointer;
    transition: all .5s ease-in-out;
}

.buttons button:hover{
    color:black;
}
.todo.done p{
    text-decoration: line-through;
    text-decoration-thickness: 2px;
    color: orangered;
    text-decoration-style:double;
}
<div class="container">
  <div class="add">
    <label>ADD TODO</label>
    <input type="text" placeholder="Write your To do">
  </div>
  <div class="todos">
    <div class="todo">
      <p>Todo 1</p>
      <div class="buttons">
        <button class="remove">x</button>
        <button class="check">y</button>
      </div>
    </div>
    <div class="todo">
      <p>Todo 2</p>
      <div class="buttons">
        <button class="remove">x</button>
        <button class="check">y</button>
      </div>
    </div>
    <div class="todo">
      <p>Todo 3</p>
      <div class="buttons">
        <button class="remove">x</button>
        <button class="check">y</button>
      </div>
    </div>
  </div>
</div>
dave
  • 62,300
  • 5
  • 72
  • 93
2

You need to update the buttons list, after delete or add todo. This is hard to do and I suggest you use it this way: You must edit HTML buttons to look like this:

 <button class="remove" onclick="remove(this)">x</button>
 <button class="check" onclick="done(this)">y</button>

After did that change your javascript file to:

const todoInp = document.querySelector('.add input')
const todos = document.querySelector('.todos')


todoInp.addEventListener('keyup', (e) => {

if (e.key === 'Enter' && e.target.value !== '') {
  const todoVal = e.target.value
  const todo = document.createElement('div')
  todo.classList.add('todo')

  todo.innerHTML = `
    <p>${todoVal}</p>
    <div class="buttons">
      <button class="remove" onclick="remove(this)">x</button>
      <button class="check" onclick="done(this)">y</button>
    </div>
  `

todos.appendChild(todo)

todoInp.value = ''
    
}
})



function done(btn){
    const mainPar = btn.parentElement.parentElement
    mainPar.classList.toggle('done')
}

function remove(btn){
    const parent = btn.parentElement
    const mainPar = parent.parentElement
    mainPar.remove()
}
HoM
  • 131
  • 1
  • 2
1

The reason your X and Y buttons are not working is because the querySelectorAll has already run before the new buttons exist, hence the addEventListener could not be added.

Using event delegation you only need a single click event on your parent element. Then inside the event handler you can check what element was pressed.

const todoInp = document.querySelector('.add input');
const todos = document.querySelector('.todos');

todoInp.addEventListener('keyup', (e) => {
  if (e.key === 'Enter' && e.target.value !== '') {
    const todoVal = e.target.value;
    const todo = document.createElement('div');
    todo.classList.add('todo');
    
    todo.innerHTML = `
        <p>${todoVal}</p>
        <div class="buttons">
          <button class="remove">x</button>
          <button class="check">y</button>
        </div>
    `;
    todos.appendChild(todo);
    todoInp.value = '';
  }
})

// Add a single click event to the todos wrapper
todos.addEventListener('click', (e) => {
  // exit if we don't click on a button
  if(e.target.nodeName !== 'BUTTON') return;
  
  // get `mainPar` element
  const mainPar = e.target.parentElement.parentElement;
  
  // remove todo item
  if(e.target.classList.contains('remove')) {
    return mainPar.remove();
  }
  
  // check todo item
  if(e.target.classList.contains('check')) {
    return mainPar.classList.toggle('done');
  }
}, false);
* {
    box-sizing: border-box;
}
body {
    display: flex;
    justify-content: center;
    align-items: center;
    flex-direction: column;
    margin: 0;
    padding: 0;
    margin-top: 50px;
    height: 100vh;
    background-color: aliceblue;
}

.container{
    width: 600px;
    height: auto;
    padding: 50px;
}

.add,
.todos{
    display: flex;
    flex-direction: column;
    align-items: flex-start;
}
.add label{
    font-family: 'Courier New', Courier, monospace;
    font-weight: bold;
    color: darkblue;
}
.add input{
    background: white;
    padding: 13px;
    border: 0.5px solid #ccc;
    width: 90%;
    border-radius: 4px;
}

.add input:focus{
    outline: none;
}

.add input::placeholder{
    color:#ccc
}

.todo{
    color: darkblue;
    width: 90%;
    display: flex;
    justify-content: space-between;
    align-items: center;
    border-bottom: 1px solid #ccc;
}
.todo:last-child{
    border: 0
}

.todo p{
    font-size: 24px;
    font-weight: bold;
}

.buttons button{
    background-color: transparent;
    color: #aaa;
    border: 0;
    font-size: 24px;
    cursor: pointer;
    transition: all .5s ease-in-out;
}

.buttons button:hover{
    color:black;
}
.todo.done p{
    text-decoration: line-through;
    text-decoration-thickness: 2px;
    color: orangered;
    text-decoration-style:double;
}
<div class="container">
  <div class="add">
    <label>ADD TODO</label>
    <input type="text" placeholder="Write your To do">
  </div>
  <div class="todos">
    <div class="todo">
      <p>Todo 1</p>
      <div class="buttons">
        <button class="remove">x</button>
        <button class="check">y</button>
      </div>
    </div>
    <div class="todo">
      <p>Todo 2</p>
      <div class="buttons">
        <button class="remove">x</button>
        <button class="check">y</button>
      </div>
    </div>
    <div class="todo">
      <p>Todo 3</p>
      <div class="buttons">
        <button class="remove">x</button>
        <button class="check">y</button>
      </div>
    </div>
  </div>
</div>
Reyno
  • 6,119
  • 18
  • 27