Vanilla JavaScript to-do list

Question

I'm just learning JavaScript and I started this simple to-do list project to practice on. I got most of the way there, but I couldn't get the clear button to work on any dynamically added inputs. I think this has something to do with where/when the selector variable is defined, but I couldn't work out how to fix it. I'm sure some of the code is not the best way to write it, so any recommendations or fixes would be much appreciated!

var add = document.getElementById('add');
var input = document.getElementsByClassName("input-container");
var remove = document.querySelectorAll('.remove');
var addField = function() {
 var field = document.getElementsByClassName("input-container")[0];
 var clone = field.cloneNode(true);
 var latestInput = document.getElementById("list-container").appendChild(clone);
 latestInput.childNodes[1].value = '';
};
var removeField = function() {
 if (input.length > 1) {
 var parent = this.parentElement.parentElement.parentElement;
 var child = this.parentElement.parentElement;
 parent.removeChild(child);
 }
};
add.addEventListener('click', addField);
remove.forEach(function(field) {
 field.addEventListener('click', removeField);
});

@import url('https://fonts.googleapis.com/css?family=Quicksand:300,400,500,700');
@import url('https://fonts.googleapis.com/css?family=Chewy');
* {
 box-sizing: border-box;
 font-family: 'quicksand', 'sans-serif';
}
html,
body {
 background: #E0E6ED;
 width: 100%;
 height: 100%;
 font-family: 'quicksand', 'sans-serif';
}
h1 {
 font-size: 50px;
 color: #E85D75;
 font-family: 'Chewy', cursive;
}
.main-container h1 {
 margin-bottom: 20px;
}
.text-center {
 text-align: center;
}
.text-input {
 width: 100%;
 border: none;
 border-radius: 4px;
 padding: 10px;
 font-size: 12px;
}
.input-container {
 position: relative;
 margin-top: 8px;
}
.input-container:first-child {
 margin-top: 0;
}
.remove-container {
 position: absolute;
 right: -22px;
 top: 0;
 line-height: 35px;
}
input:focus {
 outline: none;
}
.main-container {
 width: 200px;
 position: relative;
 top: 50%;
 margin-left: auto;
 margin-right: auto;
 transform: perspective(1px) translateY(-50%);
}
.plus-container {
 display: block;
 margin-top: 10px;
}
.fa.fa-plus,
.fa.fa-close {
 color: #C0C8D2;
 cursor: pointer;
}
.fa.fa-plus:hover,
.fa.fa-close:hover {
 color: #E85D75;
}

<div class="main-container text-center">
 <h1>To-Do List</h1>
 <div id="list-container" class="list-container">
 <div class="input-container">
 <input class="text-input" type="text" placeholder="Type something">
 <div class="remove-container">
 <i class="remove fa fa-close"></i>
 </div>
 </div>
 </div>
 <div class="plus-container text-center">
 <i id="add" class="fa fa-plus"></i>
 </div>
</div>
<script src="https://use.fontawesome.com/e5fb0a9868.js"></script>

https://codepen.io/mrseanbaines/pen/aLyByJ

• insertusernamehere: It seems impossible to remove elements except the first one. Keep in mind that .cloneNode "[...] does not copy event listeners added using addEventListener() [...]". See Node.cloneNode().
• mrseanbaines: @insertusernamehere Ah, very interesting! I wasn't aware of that! It sounds like that is what is causing the problem, I'll have a read of those docs. Thanks!

Answer 1

Your "clear" button

To get your clear button to work, you just need to add the event listener to each button as its created, rather than at page load. To do that, take out your .forEach() function and add this line to the end of your addField function:

remove[remove.length - 1].addEventListener('click', removeField);

You'll also want to change the remove declaration so that instead of using querySelectorAll() (not a live list) it uses .getElementsByClassName() (a live list):

var remove = document.getElementsByClassName('remove');

Optimization suggestions

In addField, the field variable is only used once. You can make the code a little more concise by removing that variable and changing your declaration of clone to this:

var clone = document.getElementsByClassName("input-container")[0].cloneNode(true);

You can condense it even further by changing the clone declaration to:

var clone = input[0].cloneNode(true);

In removeField, if you want to reduce the number of variables (and lines) you can replace these lines inside your if statement:

var parent = this.parentElement.parentElement.parentElement;
var child = this.parentElement.parentElement;
parent.removeChild(child);

with these:

var child = this.parentElement.parentElement;
child.parentNode.removeChild(child);

Enhancement suggestion

If you want to add autofocus to each newly-cloned element, so that as it's added it automatically has focus, you could do so easily by adding this line to addField:

latestInput.childNodes[1].focus();

and adding this line to the bottom of your code, to run on load:

input[0].childNodes[1].focus();

---

Here's what your code looks like with all of the changes mentioned above:

var add = document.getElementById('add');
var input = document.getElementsByClassName("input-container");
var remove = document.getElementsByClassName('remove');
var addField = function() {
 var clone = input[0].cloneNode(true);
 var latestInput = document.getElementById("list-container").appendChild(clone);
 latestInput.childNodes[1].value = '';
 latestInput.childNodes[1].focus();
 remove[remove.length - 1].addEventListener('click', removeField);
};
var removeField = function() {
 if (input.length > 1) {
 var child = this.parentElement.parentElement;
 child.parentNode.removeChild(child);
 }
};
add.addEventListener('click', addField);
remove[0].addEventListener('click', removeField);
input[0].childNodes[1].focus();

@import url('https://fonts.googleapis.com/css?family=Quicksand:300,400,500,700');
@import url('https://fonts.googleapis.com/css?family=Chewy');
* {
 box-sizing: border-box;
 font-family: 'quicksand', 'sans-serif';
}
html,
body {
 background: #E0E6ED;
 width: 100%;
 height: 100%;
 font-family: 'quicksand', 'sans-serif';
}
h1 {
 font-size: 50px;
 color: #E85D75;
 font-family: 'Chewy', cursive;
}
.main-container h1 {
 margin-bottom: 20px;
}
.text-center {
 text-align: center;
}
.text-input {
 width: 100%;
 border: none;
 border-radius: 4px;
 padding: 10px;
 font-size: 12px;
}
.input-container {
 position: relative;
 margin-top: 8px;
}
.input-container:first-child {
 margin-top: 0;
}
.remove-container {
 position: absolute;
 right: -22px;
 top: 0;
 line-height: 35px;
}
input:focus {
 outline: none;
}
.main-container {
 width: 200px;
 position: relative;
 top: 50%;
 margin-left: auto;
 margin-right: auto;
 transform: perspective(1px) translateY(-50%);
}
.plus-container {
 display: block;
 margin-top: 10px;
}
.fa.fa-plus,
.fa.fa-close {
 color: #C0C8D2;
 cursor: pointer;
}
.fa.fa-plus:hover,
.fa.fa-close:hover {
 color: #E85D75;
}

<div class="main-container text-center">
 <h1>To-Do List</h1>
 <div id="list-container" class="list-container">
 <div class="input-container">
 <input class="text-input" type="text" placeholder="Type something">
 <div class="remove-container">
 <i class="remove fa fa-close"></i>
 </div>
 </div>
 </div>
 <div class="plus-container text-center">
 <i id="add" class="fa fa-plus"></i>
 </div>
</div>
<script src="https://use.fontawesome.com/e5fb0a9868.js"></script>

• mrseanbaines: Thank you so much for your extensive and detailed answer (with explanations!). I've implemented all of this and have learnt a lot, thanks! :)

Answer 2

• You perform too many DOM operations than necessary,
• Preferably, don't load external resources from within CSS,
• Some of your i.a. containers in DOM and variables in JS are not necessary,
• https://fonts.googleapis.com/css?family=Chewy|Quicksand is a link for both fonts with one request,
• Flexbox allows for more comfortable centering,
• Some elements that could bear particular id use class instead. The same goes for referencing to elements in CSS,
• With so many font requests, it would be good to create a custom font subset,
• Rather than <div class="main-container"... use <main>. Semantic HTML has advantage i.a. in SEO,
• Top margin of h1 negatively affects vertical centering.

---

Here is rewrite with a lot more changes than those listed above (a lot could still be done, though):

const listContainer = document.getElementById("list-container"),
 inputContainer = document.getElementsByClassName('input-container')[0];
const removeField = event => listContainer.removeChild(event.target.closest('.input-container'));
document.getElementsByClassName('remove')[0].addEventListener('click', removeField);
const createInputContainer = () => {
 const clone = inputContainer.cloneNode(true);
 clone.getElementsByClassName('remove')[0].addEventListener('click', removeField);
 clone.getElementsByTagName('input')[0].value = '';
 return clone;
};
document.getElementById('add').addEventListener('click', () => {
 const node = createInputContainer();
 listContainer.appendChild(node);
 node.getElementsByTagName('input')[0].focus();
});

* {
 box-sizing: border-box;
}
html, body {
 align-items: center;
 background: #E0E6ED;
 display: flex;
 font-family: 'quicksand', 'sans-serif';
 height: 100%;
 justify-content: center;
 margin: 0;
}
main > h1 {
 color: #E85D75;
 font: 50px 'Chewy', cursive;
 margin: 0 0 12px;
}
main, #add {
 text-align: center;
}
.input-container {
 position: relative;
 margin-top: 8px;
}
input {
 width: 100%;
 border: none;
 border-radius: 4px;
 padding: 10px;
 font-size: 12px;
}
input:focus {
 outline: none;
}
.remove {
 position: absolute;
 right: -22px;
 top: 0;
 line-height: 35px !important;
}
#add {
 display: block;
 margin-top: 10px;
}
#add, .remove {
 color: #C0C8D2;
 cursor: pointer;
}
#add:hover, .remove:hover {
 color: #E85D75;
}

<link href="https://maxcdn.bootstrapcdn.com/font-awesome/4.7.0/css/font-awesome.min.css" rel="stylesheet"/>
<link href="https://fonts.googleapis.com/css?family=Chewy|Quicksand" rel="stylesheet"/>
<main>
 <h1>To–Do List</h1>
 <div id="list-container" class="list-container">
 <div class="input-container">
 <input placeholder="Type something" autofocus="autofocus"/>
 <i class="remove fa fa-close"></i>
 </div>
 </div>
 <i id="add" class="fa fa-plus"></i>
</main>