\$\begingroup\$
\$\endgroup\$
0
I created my first "program": a to-do list. I would appreciate it if somebody could look at the code and give me some hints on how to improve it.
var list = document.createElement("ol");
list.id = "list";
list.className = "list";
var addTask = document.getElementById("inputBtn");
addTask.addEventListener('click', function (){
// checking whether the field is empty
var task = document.getElementById("task").value;
if(task=="") alert("Are you sure you do not have any task to do?");
else
{
// adding an element
addElement (list, task);
//clear the input field
document.getElementById("task").value ="";
}
}, false);
function addElement(ol, text)
{
var element = document.createElement("li");
element.id = "element";
element.className = "element";
element.appendChild(document.createTextNode(text));
ol.appendChild(element);
document.getElementById("board").appendChild(ol);
//adding a remove button
addRemoveBtn(element);
//adding a function which change the decoration of the text
addCrossStyle(element);
}
function addRemoveBtn(li)
{
//creating a remove button
var removeTask = document.createElement("input");
removeTask.setAttribute("type", "button");
removeTask.setAttribute("value", "Remove");
removeTask.setAttribute("class", "remove");
//remove task function
removeTask.addEventListener('click', function (){
li.parentNode.removeChild(li);
}, false);
li.appendChild(removeTask);
}
function addCrossStyle(li)
{
var check = false;
li.addEventListener('click', function (){
if (check==false)
{
li.style.textDecoration = "line-through";
li.style.textDecorationColor = "red";
check = true;
}
else
{
li.style.textDecoration = "none";
check = false;
}
}, false);
} body
{
margin: 0;
background-color: #26282E;
color: #b3b3ff;
font-family: 'Arial', sans-serif;
font-size: 28px;
}
h1
{
font-size: 64px;
font-weight: 400;
text-align: center;
letter-spacing: 5px;
margin-bottom: 0px;
margin-top: 40px;
}
.addtask
{
margin-top: 20px;
text-align: center;
}
input
{
margin-top: 10px;
border: solid;
padding: 10px;
font-size: 20px;
background-color: #b3b3ff;
}
input::placeholder
{
color: white;
text-align: center;
}
.inputBtn
{
background-color: #8080ff;
margin-top: 20px;
border: 3px black solid;
color: white;
padding: 15px 32px;
text-decoration: none;
display: inline-block;
font-size: 28px;
}
.inputBtn:hover
{
cursor:pointer;
background-color: #6b6b99;
}
.list
{
width: 900px;
margin: auto;
background-color: #b3b3ff;
margin-top: 10px;
color: white;
}
.remove
{
background-color: #b30000;
border: 3px black solid;
color: white;
padding: 5px 15px;
font-size: 12px;
position: absolute;
margin-right: 5px;
right: 0;
top: 0;
opacity: 0.7;
}
.remove:hover
{
cursor: pointer;
opacity: 1;
background-color: red;
}
.element
{
position: relative;
padding: 15px 32px;
font-size: 22px;
word-wrap: break-word;
padding-bottom: 10px;
border-bottom: solid #8080ff;
cursor: pointer;
}
.element:hover
{
background: #8080ff;
}<!DOCTYPE html>
<html lang="pl">
<head>
<meta charset="utf-8">
<title>To Do List</title>
<meta name="author" content="Szymon Wolny">
<meta http-equiv="X-Ua-Compatible" content="IE=edge,chrome=1">
<link rel="stylesheet" href="todo.css">
</head>
<body>
<header>
<h1> To Do List</h1>
</header>
<main>
<article>
<div class="addtask">
<form>
<input type ="text" name="task" placeholder="New task..." id="task" size="45" ><br>
</form>
<button type="button" id="inputBtn" class="inputBtn">Add a new task to the List </button>
</div>
<div id="board"></div>
</article>
</main>
<script src="todo.js"></script>
</body>
</html>
Phrancis
20.5k6 gold badges69 silver badges155 bronze badges
1 Answer 1
\$\begingroup\$
\$\endgroup\$
0
so just 2 things here:
if(task=="") alert("Are you sure you do not have any task to do?");such comparison is a bit risky in JS. In case of checking for empty values it's better to doif(!condition) {} else {}or at leastif(task ==="")(strong type checking). Not saying it's wrong in this case, just a rule of a thumb. Read more about issues with JS comparison / type casting, you'll find out what I mean.- Creating UI in code is fine of course, and it's nice to understand what's going on under the hood, but for readability of your UI you mind find it better to define everything up-front in HTML and than later just show / hide parts of it or add remove classes, depending on your application state. This is not a general guideline, because it only makes sense for small-medium apps, for larger, more complex, you probably will use some framework anyway (angular or such). Other than that, fine simple app (assuming it works, didn't run it to be honest) ;).
answered Oct 23, 2017 at 20:32
lang-css