Liwei Su's Archive

Poor Project Reading

The github repo is here. This is a project that I read for the first time, and it's seriously poor.

There are two main problems in this project.

  1. the function is refactor needed.
  2. the delete function is not working at pure html.

Refactor Needed

After read the function of getTodos, I know why after refresh. The page become normal because after refresh, the getTodos function is called, and there also a create todo logic in it, since there the logic is right, the page will still perform right.

But this also come up an issue that, there are repetitive codes in addTodo and getTodos, we can DRY that code.

I DRY that code myself, but seems some bugs occur, so I think I should read twice before coding.

Before, the functions addTodo and getTodos are looked like this:

function addTodo(e) {
  e.preventDefault();
  
  // Create todo div
  const todoDiv = document.createElement("div");
  todoDiv.classList.add("todo");
  
  // Create list
  const newTodo = document.createElement("li");
  newTodo.innerText = todoInput.value;
  
  // Save to local
  saveLocalTodos(todoInput.value);
  newTodo.classList.add("todo-item");
  todoDiv.appendChild(newTodo);
  todoInput.value = "";
  
  // Create Completed Button
  const completedButton = document.createElement("button");
  completedButton.innerHTML = `✓`;
  completedButton.classList.add("complete-btn");
  todoDiv.appendChild(completedButton);
  
  // Create trash button
  const trashButton = document.createElement("button");
  trashButton.innerHTML = `✗`;
  trashButton.classList.add("trash-btn");
  todoDiv.appendChild(trashButton);
  
  // Final Todo
  todoList.appendChild(todoDiv);
}
function getTodos() {
  let todos;
  
  if (localStorage.getItem("todos") === null) {
    todos = [];
  } else {
    todos = JSON.parse(localStorage.getItem("todos"));
  }
  
  todos.forEach(function (todo) {
    // Create todo div
    const todoDiv = document.createElement("div");
    todoDiv.classList.add("todo");
    
    // Create list
    const newTodo = document.createElement("li");
    newTodo.innerText = todo;
    newTodo.classList.add("todo-item");
    todoDiv.appendChild(newTodo);
    todoInput.value = "";
    
    // Create Completed Button
    const completedButton = document.createElement("button");
    completedButton.innerHTML = `✓`;
    completedButton.classList.add("complete-btn");
    todoDiv.appendChild(completedButton);
    
    // Create trash button
    const trashButton = document.createElement("button");
    trashButton.innerHTML = `✗`;
    trashButton.classList.add("trash-btn");
    todoDiv.appendChild(trashButton);
    
    // Final Todo
    todoList.appendChild(todoDiv);
  });

I think it's hard to refactor now, at least at this form.

About these code, here are some questions:

  • what is 'e' and what is preventDefault do?
  • why we need todoInput.value = "";

As we all know, when a button is submit type, its default action is submit the form to the remote server. But since here we only use local memory to keep the todo list, so don't need to do that. And the 'e' here is just a parameter to pass the related variable of clicking event define by browser.

Delete Myth

After commenting out the style.css link in index.html, I find a strange problem of this code: when I input, submit, and delete the note, it didn't delete at once, but after a refresh. Code is here:

function deleteTodo(e) {
  const item = e.target;

  if (item.classList[0] === "trash-btn") {
    const todo = item.parentElement;
    
    todo.classList.add("fall");
    todo.classList.add("completed");
    removeLocalTodos(todo);
    
    todo.addEventListener("transitionend", (e) => {
      todo.remove();
    });
  }
  
  if (item.classList[0] === "complete-btn") {
    const todo = item.parentElement;
    
    todo.classList.toggle("completed");
  }
}

Here I use console.log to test what is item, and found it is button. But something beyond my surprise:

const todo = item.parentElement;
console.log(todo)
todo.classList.add("fall");
todo.classList.add("completed");
console.log(todo)
removeLocalTodos(todo);

The first and the second log show the same content! which is a div have 3 class.

Thoughts? Leave a comment