To-do list with counting of checked items

Question

For practice JavaScript I created simple To-do app.
This app can add TODOs and track the number of total TODOs as well as the number of unchecked TODOs.
For this I written 4 functions:

for add newTodo - newTodo(), where I add new todo to todos array
for render list of todo - drawTodoList(), where I also add event listener to each checkbox item
confirmCheck() for change state of todo checked property
countItem() for counting all items and unchecked items

My questions:

was it right to use innerHTML to add li items?
adding event listener to all checkboxes in my case takes 4 line (in end drawTodoList). How can I short this?
how can I improve my function countItem()?

const todos = []
let newTodoId = 0
let checkboxes 
let checkboxArray

const classNames = {
  TODO_ITEM: 'todo-container',
  TODO_CHECKBOX: 'todo-checkbox',
  TODO_TEXT: 'todo-text',
  TODO_DELETE: 'todo-delete',
}

const list = document.getElementById('todo-list')
const itemCountSpan = document.getElementById('item-count')
const uncheckedCountSpan = document.getElementById('unchecked-count')

function newTodo() {
  let getTodo = prompt("Add new to-do", "")

  if(getTodo == null || getTodo == "") {
    console.log("you don't add new to-do")
  } else {
    todos.push({
      id: newTodoId ++,
      text: getTodo,
      isChecked: false,
    })
    drawTodoList()
  }
}

function drawTodoList() {
  const liItem = todos.map((todo) => {
    if (!todo.isChecked) {
      return `<li id=${todo.id} class=${classNames.TODO_ITEM}>
    <input type='checkbox' class=${classNames.TODO_CHECKBOX}>${todo.text}</li>`
    } else {
      return `<li id=${todo.id} class=${classNames.TODO_ITEM}>
    <input type='checkbox' class=${classNames.TODO_CHECKBOX} checked>${todo.text}</li>`
    }
  })
  list.innerHTML = liItem.join('')

  checkboxes = document.querySelectorAll('input[type=checkbox]')
  checkboxArray = Array.from(checkboxes)
  checkboxArray.forEach(function(checkbox) {
    checkbox.addEventListener('change', confirmCheck)
  })
  countItem()
}

function confirmCheck() {
  let checkedToDo = todos.find(todo => {
    return todo.id == this.parentElement.id
  })
  checkedToDo.isChecked = !checkedToDo.isChecked
  countItem()
}

function countItem() {
  let itemCount = document.querySelectorAll('input[type="checkbox"]').length
  let checkedCount = document.querySelectorAll('input[type="checkbox"]:checked').length
  let uncheckedCount = itemCount - checkedCount
  itemCountSpan.innerHTML = itemCount
  uncheckedCountSpan.innerHTML = uncheckedCount
}

.center {
  align-self: center;
}

.flow-right {
  display: flex;
  justify-content: space-around;
}
.container {
  max-width: 800px;
  margin: 0 auto;
  padding: 10px;
  display: flex;
  flex-direction: column;
  background-color: white;
  height: 100vh;
}

.title, .controls, .button {
  flex: none;
}

.button {
  padding: 10px 20px;
}

<!DOCTYPE html>
<html>
  <head>
    <title>TODO App</title>
    <link rel="stylesheet" type="text/css" href="./styles.css" />
  </head>
  <body>
    <div class="container center">
      <h1 class="center title">My TODO App</h1>
      <div class="flow-right controls">
        <span>Item count: <span id="item-count">0</span></span>
        <span>Unchecked count: <span id="unchecked-count">0</span></span>
      </div>
      <button class="button center" onClick="newTodo()">New TODO</button>
      <ul id="todo-list" class="todo-list"></ul>
    </div>
    <script src="./script.js"></script>
  </body>
</html>

Community · Accepted Answer · 2020-06-10 13:24:26Z

Adding to the DOM

There is no one right or wrong way to add to the DOM, each has its pros and cons.

Some options

Markup direct to the page via element.innerHTML (as you have done).
Using the document API to create a node tree, assign properties and then add to the DOM using any of the various node insert functions like node.appendChild.
Cloning the content of a template element that defines the nodes you want to add. Then use the document API to make the needed change and append to the page

So which to use?

Its a rather subjective question and it would be best that you try them all.

was it right to use innerHTML to add li items?

Yes and no.

The method of adding was not wrong (element.innerHTML), but the design of the function and overall approch to procedural generated content is wrong.

You are re-creating the whole displayed todo list whenever you add to it. That's like building a wall one brick at a time, but before you add the next brick you smash down the wall and rebuild it to the point where you add the next brick.

You need only add the new todo item to the end (or top) of the list of items. The rest of them can stay where you put them before.

Event listeners

adding event listener to all checkboxes in my case takes 4 line (in end drawTodoList). How can I short this?

You are referring to the lines...

checkboxes = document.querySelectorAll('input[type=checkbox]')    
checkboxArray = Array.from(checkboxes)   
checkboxArray.forEach(function(checkbox) {
   checkbox.addEventListener('change', confirmCheck)    
})

...which could be shortened to

document.geteElementById("#"+toDo.id).addEventListener('change', confirmCheck );

But the question can not be directly answered as the function drawTodoList is flawed, as I described in Adding to the DOM.

The click function can be added to the un-ordered list element once, at the start of the app, thus one event covers all todo items.

The checkbox that is clicked on can be found in event.target. Adding a property to the checkbox that holds the todo item id lets you apply the changes to the todolist via the todo id.

Information, data, and the display

how can I improve my function countItem()?

The count is a behavior of the information not the display, get it by stepping over the items in the todo list.

The rewrite.

I started from your HTML and CSS.

It is a bit over kill but I got lost in the writing of it. The code contains how I have implemented the 3 questions you ask. Adding to the DOM, creating event listener for new items, and counting the number of unchecked items.

const todoDataJSON = `[{ "id" : 1, "checked" : false, "text" : "Click Something todo" },{ "id" : 2, "checked" : false, "text" : "Check Click Something todo as done" }]`;


// the todo information and behaviours
const todoList = (() => {
    var UId = 1;                // NOTE must start above 0 to be truthy true
    const items = [];   
    const indexed = new Map();  // For quick lookups by id
    const vetRecord = record => true; // This stub Make sure incoming data is valid and safe
    const vetText = text => typeof text === "string" && text !== "";
    const create = (text, checked = false, id = UId++) => ({id, text, checked});
    const reset = () => (items.length = 0, indexed.clear(), UId = 1);

    const toDoList = Object.freeze({
        get length() { return items.length },
        get uncheckedCount() { return items.reduce((uc, record) => uc += record.checked ? 0 : 1, 0) },
        set data(jsonText) { 
            var maxId = UId; 
            try {                    
                const data = JSON.parse(jsonText);
                reset();
                data.forEach(item => {
                    if (!vetRecord(item)) { throw new Error(); }
                    maxId = Math.max(item.id, maxId);
                    const record = create(item.text, item.checked, Number(item.id));
                    items.push(record);
                    indexed.set(record.id, record);
                });
                UId = maxId + 1;
            } catch(e) { reset() } // make data safe again
        },
        get data() { return JSON.stringify(items) },
        each(callback) { for (const record of items) { callback({...record}) } }, 
        toggleCheck(id) {
            const record = indexed.get(id);
            if (record) { record.checked = !record.checked }
        },
        add(text) {
            if(vetText(text)) {
                const record = create(text);
                items.push(record);
                indexed.set(record.id, record);
                return {...record};  // returns a copy not the original
            }
        }
    });    
    return toDoList;
})();


/* Display an UI */
;(()=>{
    const elements = {
        list : query('#todo-list'),
        count : query('#item-count'),
        uncheckedCount : query('#unchecked-count'),
        template : query('#todo-item'),
    };
    
    elements.list.addEventListener("click", listItemClick);
    query("#add-button").addEventListener("click", promptForNew); 
    
    function query(query, el = document) { return el.querySelector(query) }
    const assign = (el, props) => Object.assign(el, props);
    const append = (node, ...items) => {
        items.forEach(el => node.appendChild(el));
        return node;
    }
    
    function listItemClick(event) {
        if(event.target.todoId) { 
            todoList.toggleCheck(event.target.todoId) 
            updatePage();
        }
    }
    function displayToDoItem(todo) {
        const listItem = document.importNode(elements.template.content, true);
        assign(query("li", listItem), {id : "todoItem" + todo.id });
        assign(query("input", listItem), { 
            checked : todo.checked,
            name : "todo" + todo.id,
            todoId : todo.id,   // added but will not be vissible in markup
        });
        assign(query("label", listItem), { 
            "for" : "todo" + todo.id,
            textContent : todo.text
        });
        append(elements.list, listItem); 
    }
    function promptForNew() {
        const todo = todoList.add(prompt("What is it you want to do", ""));
        if (todo) {  displayToDoItem(todo) }
        updatePage();
    }
    function updatePage() {
        elements.count.textContent = todoList.length;
        elements.uncheckedCount.textContent = todoList.uncheckedCount;
    }
    
    // source data and give to todolist
    todoList.data = todoDataJSON;
    todoList.each(displayToDoItem); // display existing
    updatePage();
    
})();

.center {
  align-self: center;
}

.flow-right {
  display: flex;
  justify-content: space-around;
}
.container {
  max-width: 800px;
  margin: 0 auto;
  padding: 10px;
  display: flex;
  flex-direction: column;
  background-color: white;
  height: 100vh;
}

.title, .controls, .button {
  flex: none;
}

.button {
  padding: 10px 20px;
}

<template id="todo-item"> 
  <li class="todo-container">
     <input type="checkbox" class="todo-checkbox">
     <label></label>
  </li>
</template>
<div class="container center">
  <h1 class="center title">Things to do</h1>
  <div class="flow-right controls">
    <span>All things done & to do: <span id="item-count">0</span></span>
    <span>Things left to do: <span id="unchecked-count">0</span></span>
  </div>
  <button class="button center" id="add-button">I Have Something To Do</button>
  <ul id="todo-list" class="todo-list"></ul>
</div>

Stack Exchange Network

To-do list with counting of checked items

My questions:

1 Answer 1

Adding to the DOM

Some options

So which to use?

Event listeners

Information, data, and the display

The rewrite.

You must log in to answer this question.

Hot Network Questions

To-do list with counting of checked items

My questions:

1 Answer 1

Adding to the DOM

Some options

So which to use?

Event listeners

Information, data, and the display

The rewrite.

You must log in to answer this question.

Related

Hot Network Questions