2
\$\begingroup\$

I'm new to programming and in front-end. It's really hard for me to learn good practices.

Right now I'm trying to learn javascript basics and bootstrap in addition. I did mine first ToDo list with bootstrap (just to train it a bit) and I feel like my JS code and HTML are just bad and should do it much better.

Can you take a look and tell me what should I do better? It works but I think not as well as it should. 

const divTodo = document.querySelector('#tasks');
const addBtn = document.querySelector('#addTask');
const addInput = document.querySelector('#addTaskText');
const taskList = []
taskList.push({
 task: "Make a dinner.",
 isDone: true,
});
// Funkcje 
//Dodaj Zadanie
const addTask = function (task) {
 taskList.push({
 task: task,
 isDone: false,
 })
}
//Zmien status isDone
const changeStatus = function (index) {
 taskList[index].isDone = !taskList[index].isDone;
}
const render = function () {
 divTodo.textContent = "";
 taskList.forEach((el, index) => {
 const newRow = document.createElement('div');
 const newCheckboxDiv = document.createElement('div');
 const newCheckbox = document.createElement('input');
 const newTask = document.createElement('div');
 const newButtonDiv = document.createElement('div');
 const newButton = document.createElement('button');
 //row bg-warning m-2 d-flex align-items-center rounded
 newRow.classList.add('row')
 newRow.classList.add('m-2')
 newRow.classList.add('d-flex')
 newRow.classList.add('align-items-center')
 newRow.classList.add('rounded')
 el.isDone == true ? newRow.classList.add('bg-success') : newRow.classList.add('bg-warning');
 //col-2 col-md-1
 newCheckboxDiv.classList.add('col-2')
 newCheckboxDiv.classList.add('col-md-1')
 //form-control form-control-lg
 newCheckbox.classList.add('form-control');
 newCheckbox.classList.add('form-control-lg');
 newCheckbox.type = 'checkbox';
 el.isDone == true ? newCheckbox.checked = true : newCheckbox.checked = false;
 newCheckbox.dataset.key = `${index}`;
 newCheckbox.addEventListener('click', () => {
 el.isDone = !el.isDone;
 render();
 })
 //col-7 col-md-8 text-justify
 newTask.classList.add('col-7');
 newTask.classList.add('col-md-8');
 newTask.classList.add('text-justify');
 newTask.textContent = el.task;
 //col-3
 newButtonDiv.classList.add('col-3');
 //btn btn-danger btn-block p-1 p-md-2
 newButton.classList.add('btn');
 newButton.classList.add('btn-danger');
 newButton.classList.add('btn-block');
 newButton.classList.add('p-1');
 newButton.classList.add('p-md-2');
 newButton.textContent = "Delete";
 // newButton.dataset.key = `${index}`;
 newButton.addEventListener('click', () => {
 taskList.splice(index, 1);
 render();
 })
 newCheckboxDiv.appendChild(newCheckbox);
 newButtonDiv.appendChild(newButton);
 newRow.appendChild(newCheckboxDiv);
 newRow.appendChild(newTask);
 newRow.appendChild(newButtonDiv);
 divTodo.appendChild(newRow);
 })
}
render();
addBtn.addEventListener('click', () => {
 if (addInput.value == "") return;
 addTask(addInput.value);
 render();
 addInput.value = "";
})
<!DOCTYPE html>
<html lang="en">
<head>
 <meta charset="UTF-8">
 <meta name="viewport" content="width=device-width, initial-scale=1.0">
 <meta http-equiv="X-UA-Compatible" content="ie=edge">
 <title>ToDo list</title>
 <!-- Bootstrap CSS -->
 <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/css/bootstrap.min.css" integrity="sha384-MCw98/SFnGE8fJT3GXwEOngsV7Zt27NXFoaoApmYm81iuXoPkFOJwJ8ERdknLPMO"
 crossorigin="anonymous">
 <link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.5.0/css/all.css" integrity="sha384-B4dIYHKNBt8Bc12p+WXckhzcICo0wtJAoU8YZTY5qE0Id1GSseTk6S+L3BlXeVIU"
 crossorigin="anonymous">
</head>
<body class="bg-info">
<!---
 <div class="container mt-md-5">
 Three big buttons - doesnt work 
 <div class="row">
 <div class="col-md-12 mr-auto ml-auto text-center">
 <div class="row">
 <div class="col-12 col-md-4 mr-auto ml-auto mt-3 mt-md-1">
 <button class="btn btn-warning btn-block">All</button>
 </div>
 <div class="col-12 col-md-4 mr-auto ml-auto mt-3 mt-md-1">
 <button class="btn btn-warning btn-block">Done</button>
 </div>
 <div class="col-12 col-md-4 mr-auto ml-auto mt-3 mt-md-1">
 <button class="btn btn-warning btn-block">To do</button>
 </div>
 </div>
 </div>
 </div>
 --->
 <!------------------------------>
 <!-- Add Tasks-->
 <div class="row">
 <div class="col-12 col-md-8 mt-2">
 <input type="text" class="form-control" placeholder="Add task..." id="addTaskText">
 </div>
 <div class="col-12 col-md-4 mt-2">
 <button class="btn btn-success btn-block" id="addTask">Add Task!</button>
 </div>
 </div>
 <!------------------------------>
 <div id="tasks">
 </div>
 <!-- Optional JavaScript -->
 <!-- jQuery first, then Popper.js, then Bootstrap JS -->
 <script src="https://code.jquery.com/jquery-3.3.1.slim.min.js" integrity="sha384-q8i/X+965DzO0rT7abK41JStQIAqVgRVzpbzo5smXKp4YfRvH+8abtTE1Pi6jizo"
 crossorigin="anonymous"></script>
 <script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.3/umd/popper.min.js" integrity="sha384-ZMP7rVo3mIykV+2+9J3UJ46jBk0WLaUAdn689aCwoqbBJiSnjAK/l8WvCWPIPm49"
 crossorigin="anonymous"></script>
 <script src="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/js/bootstrap.min.js" integrity="sha384-ChfqqxuZUCnJSK3+MXmPNIyE6ZbWh2IMqE241rYiqJxyMiZ6OW/JmZQ5stwEULTy"
 crossorigin="anonymous"></script>
 <script src="main.js"></script>
</body>
</html>

200_success
146k22 gold badges190 silver badges479 bronze badges
asked Dec 10, 2018 at 18:20
\$\endgroup\$
1
  • \$\begingroup\$ Looks pretty good actually! Creating DOM elements and adding multiple classes can get a bit verbose, so you might want to write a separate function for that, or you could add multiple classes by using newButton.className = "btn btn-danger btn-block p-1 p-md-2 Delete"; \$\endgroup\$ Commented Dec 13, 2018 at 11:28

1 Answer 1

1
\$\begingroup\$

Many of the things done are for showing alternatives. There are many ways to do things, each with pros and cons. I have also removed the parts that were not necessary so that there are less distractions.

Used a <div> with style='display: none' to provide a template of a todo task that I clone when adding a new task.

Instead of redoing the whole list each time you do anything, it now handles the changes that need to be made as they are needed.

Event delegation used so that the whole thing only needs 2 total event handlers as opposed to 1 + 2 times number of tasks.

I added const done and todo for clarifying the classes used to indicate those states.

Hopefully this is helpful in your learning journey, good luck.

const divTodo = document.getElementById('tasks');
const addBtn = document.getElementById('addTask');
const addInput = document.getElementById('addTaskText');
const taskTemplate = document.getElementById('taskTemplate').children[0];
const done = 'bg-success';
const todo = 'bg-warning';
let newIndex = 0;
const taskList = {};
// event delegation
divTodo.addEventListener('click', function(e) {
 const target = e.target;
 const row = target.parentNode.parentNode;
 if (target.type === 'checkbox') {
 const task = taskList[target.dataset.key];
 task.isDone = !task.isDone;
 target.checked = task.isDone;
 row.classList.toggle(done);
 row.classList.toggle(todo);
 } else if (target.type === 'submit') {
 delete taskList[target.dataset.key];
 divTodo.removeChild(row);
 }
});
const addTask = function(task, initial) {
 const newTask = {
 task: task,
 isDone: initial || false,
 };
 addElement(newTask);
 taskList[newIndex] = newTask;
 newIndex++;
};
const addElement = function(el) {
 const index = newIndex;
 const newRow = taskTemplate.cloneNode(true);
 const newCheckbox = newRow.querySelector('input[type=checkbox]');
 const newTask = newRow.children[1];
 const newButton = newRow.querySelector('button');
 newRow.classList.add(el.isDone == true ? done : todo);
 newCheckbox.checked = el.isDone;
 newCheckbox.dataset.key = `${index}`;
 newTask.textContent = el.task;
 newButton.dataset.key = `${index}`;
 divTodo.appendChild(newRow);
};
addTask("Make a dinner.", true);
addBtn.addEventListener('click', () => {
 if (addInput.value == "")
 return;
 addTask(addInput.value);
 addInput.value = "";
});
<!DOCTYPE html>
<html lang="en">
<head>
 <meta charset="UTF-8">
 <meta name="viewport" content="width=device-width, initial-scale=1.0">
 <meta http-equiv="X-UA-Compatible" content="ie=edge">
 <title>ToDo list</title>
 <!-- Bootstrap CSS -->
 <link rel="stylesheet" href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/css/bootstrap.min.css" integrity="sha384-MCw98/SFnGE8fJT3GXwEOngsV7Zt27NXFoaoApmYm81iuXoPkFOJwJ8ERdknLPMO" crossorigin="anonymous">
 <link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.5.0/css/all.css" integrity="sha384-B4dIYHKNBt8Bc12p+WXckhzcICo0wtJAoU8YZTY5qE0Id1GSseTk6S+L3BlXeVIU" crossorigin="anonymous">
</head>
<body class="bg-info">
 <div class="row">
 <div class="col-12 col-md-8 mt-2">
 <input type="text" class="form-control" placeholder="Add task..." id="addTaskText">
 </div>
 <div class="col-12 col-md-4 mt-2">
 <button class="btn btn-success btn-block" id="addTask">Add Task!</button>
 </div>
 </div>
 <div id="tasks"></div>
 <div id="taskTemplate" style="display : none">
 <div class="row m-2 d-flex align-items-center rounded">
 <div class="col-2 col-md-1">
 <input class="form-control form-control-lg" type="checkbox">
 </div>
 <div class="col-7 col-md-8 text-justify"></div>
 <div class="col-3">
 <button class="btn btn-danger btn-block p-1 p-md-2">Delete</button>
 </div>
 </div>
 </div>
</body>
</html>

Sᴀᴍ Onᴇᴌᴀ ♦
29.6k16 gold badges45 silver badges203 bronze badges
answered Dec 15, 2018 at 1:58
\$\endgroup\$
1
  • 1
    \$\begingroup\$ You could also consider using a <template> tag for the template HTML \$\endgroup\$ Commented Dec 31, 2018 at 21:52

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.