I have build a to-do list with JavaScript can you please check my script file?

Code Feedback
#1 
let taskArray=[];


function deletetask(currentitem,button){
    button.remove();
    currentitem.remove();  
}

function addTask(){

    let task=document.getElementById('task').value;

    if(task!= ''){
        
        taskArray.push(task);
    
        const ul=document.getElementById('list');
        const li=document.createElement('li');
        const button=document.createElement('input');
        
        button.setAttribute('type','button');
        button.setAttribute('value','Delete task');
        button.setAttribute('onclick','deletetask(this.previousElementSibling,this)');
    
        li.innerText=task;
        ul.append(li);
        ul.append(button);
    
        li.classList.add('list-items');
    }
    else{
        alert("Please Enter Task");
    }
    
}

HTML FILE

<body>
    <div class="container">
      <h1>To-Do List</h1>
      <form action="">
        <input type="text" id="task" />
        <input type="button" value="Add Task" onclick="addTask()" />

        <ul id="list"></ul>
      </form>
    </div>
  </body>
#2

Was there anything particular you want us to check?

#3

My comments/suggestions:

  1. I see an array declared but never used.
  2. I see you appending input elements as children to the ul element which even though it appears to “work”, it is not syntactically correct. ul elements should only have li elements as direct children. You should be appending the input element to the applicable li element instead.
  3. I would initialize the ul variable outside of any function, since it only needs to be done once instead of executing the getElementById method every time a new task is added.
  4. I would make it so when a new task gets added, the text box is empty so a user does not have to clear out the last item typed.
1 Like
#4

I made changes as you suggested but was unable to clear the last item typed in input field.
I used task.innerText=" " ; but it’s not working

const ul=document.getElementById('list');

function deletetask(currentitem){
    currentitem.remove();
}

function addTask(){

    let task=document.getElementById('task').value;

    if(task!= ''){
        
        const li=document.createElement('li');
        const button=document.createElement('input');
        
        button.setAttribute('type','button');
        button.setAttribute('value','Delete task');
        
        button.setAttribute('onclick','deletetask(this.parentElement)');
    
        li.innerText=task;
        ul.append(li);
        li.append(button);
   
        li.classList.add('list-items');
    
        task.innerText=``;

    }
    else{
        alert("Please Enter Task");
    }

}
#5

That is because task is not an element. It is just a string value that you originally created in the following line:

let task=document.getElementById('task').value;

I suggest creating a taskElem variable outside of the function, so that inside your function, you could do things like:

li.innerText = taskElem.value;

and

taskElem.innerText = '';
1 Like
#6

Thank you for your help.

document.getElementById('task').value=``;

this worked.

#7

Great!

Below is an alternate way of doing the same thing:

const ul = document.getElementById("list");
const taskInput = document.getElementById("task");

function deleteTask(task) {
  task.remove();
}

function addTask() {
  const taskText = taskInput.value;

  if (taskText != "") {
    const taskElemStr = `
      <li class="list-items">
        <div>${taskText}</div>
        <div>
          <input
            onclick="deleteTask(this.parentElement.parentElement)"
            type="button"
            value="Delete task"
          />
        </div>
      </li>
    `;
    ul.innerHTML += taskElemStr;
    taskInput.value = "";
  } else {
    alert("Please Enter Task");
  }
}
#8

Thanks for the solution :smile:

#9

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.