5
\$\begingroup\$

I would like to share with you the code from the application I just finished. It is a web application written in HTML, CSS, JavaScript and jQuery. It uses LocalStorage to store your tasks. The application also displays the date of adding and completing the task. You can add, edit, delete and mark tasks as done.

I would ask for some code review. I do not know good practices in JavaScript yet so I need help from specialists. The application works fully and can be tested here.

The whole code (if it is illegible here) can be found on my GitHub here.

$(function() {
    if (typeof(Storage) !== "undefined") {
        var AddTaskButton = document.querySelector("#add-task-text");
        var input = document.querySelector("#add-task-input");
        var tasks = (JSON.parse(localStorage.getItem('ToDoApp')) != null) ? JSON.parse(localStorage.getItem('ToDoApp')) : [];
        
        AddTaskButton.addEventListener('click', () => {
            addTask();
        });
    
        input.addEventListener('focus', () => {
            input.addEventListener('keydown', (key) => {
                if(key.keyCode === 13) {
                    addTask();
                }
            });
        });

        var addTask = () => {
            var taskContent = document.querySelector('#add-task-input').value;
            taskContent = taskContent.replace(/^\s+|\s+$/g, '');
            taskContent = taskContent.trim();
            
            if(taskContent != 0 ){
                var task = prepareTaskForLocalstorage(null, getDateAndTime(), taskContent, false);

                tasks.push(task);
                saveTaskToLocalStorage(tasks);
                listTasksFromLocalStorage(tasks, true);
                document.querySelector('#add-task-input').value = '';
            }
        };

        var getDateAndTime = () => {
            var d = new Date();
            var year = d.getFullYear();
            var month = d.getMonth();
            month += month;
            
            if (month < 10) {
                month = '0' + month;
            }
        
            var day = d.getDate();
            if(day < 10) {
                day = '0' + day;
            }

            var hour = d.getHours();
            var minutes = d.getMinutes();

            if (minutes < 10) {
                minutes = '0' + minutes;
            }

            var fullDateAndTime = year + '-' + month + '-' + day + ' ' + hour + ':' + minutes;

            return fullDateAndTime;
        };

        var prepareTaskForLocalstorage = (completion_date, create_date, content, ifChecked) => {
            return {
                "completion_date": completion_date,
                "create_date": create_date,
                "content": content,
                "checked": ifChecked
            };
        };

        var saveTaskToLocalStorage = (task) => {
            localStorage.setItem("ToDoApp", JSON.stringify(task));
        };

        var listTasksFromLocalStorage = (ta, animate) => {
            document.getElementById('list-of-todos').innerHTML = "";
            document.getElementById('list-of-done-tasks').innerHTML = "";
                for (var i = 0; i < ta.length; i++) {
                    if(!ta[i].checked) {
                        var ul = document.querySelector('#list-of-todos');
                        var li = document.createElement('li');
                        if(animate == true && i == ta.length-1) {
                            li.classList.add('anim');
                        }
                        var div_structure = `
                            <div class="check-box">
                                <input type="checkbox">
                                <label for="checkBox"></label>
                            </div>
                            <div class="task-text">` + ta[i].content + `</div>
                            <div class="edit-delete-date-hour">
                                <span class="edit">Edit</span>
                                <span class="delete">Delete</span>
                                <span class="date-hour" title="The date and time the task was added">` + ta[i].create_date + `</span>
                            </div>
                            `;
                        li.innerHTML = div_structure;
                        ul.prepend(li);
                    } else {
                        var ul = document.querySelector('#list-of-done-tasks');
                        var li = document.createElement('li');
                        var div_structure = `
                            <div class="task-text">` + ta[i].content + `</div>
                            <div class="edit-delete-date-hour">
                                <span class="moveToToDo">To Do</span>
                                <span class="delete">Delete</span>
                                <span class="date-hour" title="The date and time the task was added">` + ta[i].create_date + `</span>
                                <span class="date-hour date-hour-completion" title="The date and time when the task was completed">` + ta[i].completion_date + `</span>
                            </div>
                            `;
                        li.innerHTML = div_structure;
                        ul.prepend(li);
                    }
                }        
        };
        listTasksFromLocalStorage(tasks, false);

        $(document).on('click', '.check-box label', function() {
            makeTaskDone(this);
        });

        var makeTaskDone = (t) => {
            var doneTaskContent = t.parentNode.parentNode.getElementsByClassName('task-text')[0].innerHTML;
            var obj = tasks.find(o => o.content === doneTaskContent);
            obj.checked = true;
            obj.completion_date = getDateAndTime();
            saveTaskToLocalStorage(tasks);
            listTasksFromLocalStorage(tasks, false);
        };

        $(document).on('click', '.moveToToDo', function() {
            moveTaskToToDoList(this);
        });

        var moveTaskToToDoList = (t) => {
            var clickedElement = t.parentNode.parentNode;
            var taskContent = clickedElement.getElementsByClassName('task-text')[0].innerHTML;
            var obj = tasks.find(o => o.content === taskContent);
            obj.checked = false;
            saveTaskToLocalStorage(tasks);
            listTasksFromLocalStorage(tasks, false);
        };

        $(document).on('click', '.delete', function() {
            deleteTask(this);
        });

        var deleteTask = (t) => {
            var clickedElement = t.parentNode.parentNode;
            clickedElement.classList.add('anim-hide');
            
            setTimeout(function() {
                var taskContent = clickedElement.getElementsByClassName('task-text')[0].innerHTML;
                var obj = tasks.find(o => o.content === taskContent);
                tasks.splice(tasks.indexOf(obj), 1);
                saveTaskToLocalStorage(tasks);
                listTasksFromLocalStorage(tasks, false);
            }, 400);
        };

        $(document).on('click', '.edit', function() {
            editTask(this);
        });

        var editTask = (t) => {
            var clickedElement = t.parentNode.parentNode;
            var taskElement = clickedElement.getElementsByClassName('task-text')[0];
            var taskContent = clickedElement.getElementsByClassName('task-text')[0].innerHTML;
            taskElement.setAttribute("contenteditable", "true");
            taskElement.focus();
            var obj = tasks.find(o => o.content === taskContent);
            
            taskElement.addEventListener('focusout', () => {
                taskElement.setAttribute("contenteditable", "false");
                var taskNewContent = clickedElement.getElementsByClassName('task-text')[0].innerHTML;
                obj.content = taskNewContent;
                saveTaskToLocalStorage(tasks);
                listTasksFromLocalStorage(tasks, false);
            });

            taskElement.addEventListener('keydown', (key) => {
                if(key.keyCode === 13) {
                    taskElement.setAttribute("contenteditable", "false");
                    var taskNewContent = clickedElement.getElementsByClassName('task-text')[0].innerHTML;
                    obj.content = taskNewContent;
                    saveTaskToLocalStorage(tasks);
                    listTasksFromLocalStorage(tasks, false);
                }
            });
        };
    } else {
        confirm.log('Unfortunately, LocalStorage does not work on your computer');
    }
    });
html, 
body {
    margin: 0;
    padding: 0;
    width: 100%;
    min-height: 100%;
    height: auto;
    height: auto;
}

html {
    position: relative;
}

body {
    font-family: 'Roboto', sans-serif;
    background: #005AA7; 
    background: -webkit-linear-gradient(to bottom, #FFFDE4, #005AA7);
    background: linear-gradient(to bottom, #FFFDE4, #005AA7);
}

#container {
    width: 100%;
	max-width: 2000px;
	margin-left: auto;
	margin-right: auto;
    height: 100%;
    padding-bottom: 50px;
}

h1 {
    margin: 30px 0;
    font-size: 75px;
    color: #fff;
    text-shadow: 5px 0px 1px rgba(198, 167, 39, 1);
}

header, footer {
    text-align: center;
}

#lists {
    display: flex;
    flex-wrap: wrap;
    justify-content: space-evenly;
}

#todo > header,
#done > header {
    margin-top: 20px;
    font-size: 30px;
    font-weight: 400;
    color: #fff;
}

.tasks-list {
    border:20px ridge #e6c335;
    min-height: 600px;
    width: 500px;
    background: #20002c;
    background: -webkit-linear-gradient(to top, #cbb4d4, #20002c);
    background: linear-gradient(to top, #cbb4d4, #20002c);
}

.tasks {
    width: 100%;
    text-align: left;
}

ul {
    list-style-type: none;
    padding: 0;
}

li {
    font-size: 25px;
    width: 96%;
    margin-left: auto;
    margin-right: auto;
    border-radius: 4px;
    margin-bottom: 10px;
    height: 100%;
    box-sizing: border-box;
    padding: 6px 10px;
    background: #3f4c6b;
    color: #fff;
}

li:last-child {
    border-bottom: 0px;
}

.check-box {
    width: 20px;
    position: relative;
    float: left;
}

.check-box label {
    cursor: pointer;
    position: absolute;
    width: 20px;
    height: 20px;
    top: 50%;
    left: 50%;
    transform:  translate(-50%, -50%);
    background: #e7c41c;
}

.check-box label:after {
    opacity: 0.3;
    content: '';
    position: absolute;
    width: 9px;
    height: 5px;
    background: transparent;
    top: 5px;
    left: 5px;
    border: 3px solid #514405;
    border-top: none;
    border-right: none;
    transform: rotate(-45deg);
}

.check-box label:hover::after {
    opacity: 0.5;
}

.check-box input[type=checkbox]:checked + label:after {
    opacity: 1;
}

.task-text {
    float: left;
    margin-left: 10px;
    max-width: 90%;
    padding-top: 4px;
    min-height: 20px;
    line-height: 20px;
    font-weight: 400; 
    word-wrap: break-word;
}

#list-of-done-tasks {
    margin-top: 50px;
}

#list-of-done-tasks > li > .task-text {
    margin-left: 0px;
}

.edit-delete-date-hour {
    clear: both;
    font-size: 13px;
    padding-top: 7px;
    -webkit-touch-callout: none;
    -webkit-user-select: none;
    -khtml-user-select: none;
    -moz-user-select: none;
    -ms-user-select: none;
    user-select: none;
}

.moveToToDo:hover,
.edit:hover,
.delete:hover {
    cursor: pointer;
}

.moveToToDo::after,
.edit::after {
    content: '';
    margin-right: 3px;
}

.delete::after {
    content: "\00a0\00a0\007C";
    color: #74f90e;
}

.date-hour {
    font-size: 11px;
}

#list-of-done-tasks > li > .edit-delete-date-hour > .date-hour-completion::before {
    content: "\00a0\2192\00a0\00a0";color: #74f90e;
}

.date-hour::before {
    content: "\00a0";
}

#add-task {
    width: 96%;
    margin-left: auto;
    margin-right: auto;
    border-radius: 4px;
    height: 35px;
    box-sizing: border-box;
    margin-top: 50px;
    margin-bottom: 40px;
    display: flex;
}

input {
    border: none;
}

input[type="text"], 
textarea {
    width: 90%;
    background: #e7c41c;
    color: #171c2b;
    box-sizing: border-box;
    padding: 1px 10px;
    font-size: 20px;
    border: 2px dashed #e7c41c;
    border-right: 3px solid #bc9f14;
}

input[type="text"]:focus,
textarea:focus {
    color: #0b0e16;
    border: 2px dashed #a38a11;
    outline: none!important;
}

#add-task-input,
#add-task-text {
    float: left;
}

#add-task-text{
    text-align: center;
    font-size: 22px;
    font-weight: 400;
    width: 14.5%;
    box-sizing: border-box;
    height: 35px;
    line-height: 33px;
    -webkit-touch-callout: none;
    -webkit-user-select: none;
    -khtml-user-select: none;
    -moz-user-select: none;
    -ms-user-select: none;
    user-select: none;
    background: #e7c41c;
    color: #0b0e16;
}

#add-task-text:hover {
    cursor: pointer;
    background: #d8b21a;
}

footer {
    position: absolute;
    bottom: 10px;
    left: 0;
    right: 0;
    font-size: 15px;
    color: #b1ea20;
}

.anim {
    animation-name: animation-show;
    animation-duration: 1s;
}

@keyframes animation-show {
    from {opacity: 0;}
    to {opacity: 1;}
}

.anim-hide {
    animation-name: animation-hide;
    animation-duration: 0.5s;
}

@keyframes animation-hide {
    from {opacity: 1;}
    to {opacity: 0;}
}
<!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">
    <meta name="Description" content="ToDo application for planning your precious time">
    <meta name="author" content="Fabian Zwoliński">
    <title>ToDo App</title>
    <link rel="stylesheet" type="text/css" href="styles/normalize.css">
    <link href="https://fonts.googleapis.com/css?family=Roboto:300,400,500,700" rel="stylesheet">
    <link rel="stylesheet" type="text/css" href="styles/style.css">
    <link type="text/css" media='(max-width: 1120px)' rel='stylesheet' href='styles/responsive.css' />
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
</head>
<body>
    <div id="container">
        <header>
            <h1>Simple ToDo  App</h1>
        </header>
        <div id="lists">
            <div id="todo" class="tasks-list">
                <header>TASKS TO DO</header>
                <div class="tasks">
                    <div id="add-task">
                        <input type="text" id="add-task-input" maxlength="60">
                        <div id="add-task-text">ADD</div>
                        <div style="clear: both;"></div>
                    </div>
                    <ul id="list-of-todos">
                    </ul>
                </div>
            </div>
            <div id="done" class="tasks-list">
                <header>DONE TASKS</header>
                <div class="tasks">
                    <ul id="list-of-done-tasks">
                    </ul>
                </div>
            </div>
        </div>
    </div>
    <footer>
        &copy; Copyright 2018, Fabian Zwoliński
    </footer>
    <script src="scripts/script.js"></script>
</body>
</html>

\$\endgroup\$
2
\$\begingroup\$

ECMA Script 6 Features

You are using ES6 Template Literals, but you aren't fully utilizing the syntactic sugar they provide.

Template literals don't just give you multi line strings, they give you string interpolation, which does away with the old string-concatenation style of building a string dynamically.

Note that:

var div_structure = `
                    <div class="task-text">` + ta[i].content + `</div>
                    <div class="edit-delete-date-hour">
                        <span class="moveToToDo">To Do</span>
                        <span class="delete">Delete</span>
                        <span class="date-hour" title="The date and time the task was added">` + ta[i].create_date + `</span>
                        <span class="date-hour date-hour-completion" title="The date and time when the task was completed">` + ta[i].completion_date + `</span>
                    </div>
                    `;

Can become:

var div_structure = `
                    <div class="task-text">
                        ${ta[i].content}
                    </div>
                    <div class="edit-delete-date-hour">
                        <span class="moveToToDo">To Do</span>
                        <span class="delete">Delete</span>
                        <span class="date-hour" title="The date and time the task was added">
                            ${ta[i].create_date}
                        </span>
                        <span class="date-hour date-hour-completion" title="The date and time when the task was completed">
                            ${ta[i].completion_date}
                        </span>
                    </div>
                    `;

And a few more line breaks and indentation around the tokens can assist with readability:

Instead of this:

<div class="task-text">${ta[i].content}</div>

Use this:

<div class="task-text">
    ${ta[i].content}
</div>

Using jQuery

Without seeing your whole project, it appears the only coupling this code has to jQuery is by utilizing its Event Delegation feature. Since you are only filtering elements by class name, it's easy to roll your own event delegation framework.

However, if you are using jQuery heavily elsewhere, I see no need to decouple that from your code.

But a simple event delegation framework could be:

function delegateEvent(eventType, isMyElement, handleEvent) {
    document.documentElement.addEventListener(eventType, (event) => {
        var currentTarget = event.target;

        while (currentTarget && !isMyElement(currentTarget)) {
            currentTarget = currentTarget.parentNode;
        }

        if (currentTarget) {
            handleEvent(event, currentTarget);
        }
    });
}

And to use it:

delegateEvent("click", (el) => el.classList.contains("edit"), (event, target) => {
    event.preventDefault();
    editTask(target);
});

No need for jQuery. And no need to wait for the document to be "ready" or "loaded" because the document.documentElement property references the <html> tag the very moment that JavaScript begins executing. Bubbling events do bubble up to the <html> element.

\$\endgroup\$
  • \$\begingroup\$ Very helpful answer. Thank you very much, I will definitely take advantage of these valuable advices and improve my code. \$\endgroup\$ – Giovacho May 31 '18 at 18:12
1
\$\begingroup\$

Hopefully this answer will do, until an actual expert turns up.
I am aware that other opinions are available.

Wasn't familiar with the => operator in JavaScript, and it's not supported by all browsers

You are hardly using jQuery at all. jQuery helps with browser compatibility. The following have jQuery equivalents which will make your code last longer:

  • addEventListener
  • querySelector
  • innerHTML
  • setAttribute
  • createElement
  • classList

The documentation for jQuery is here.

The way I approach JavaScript is that it's the language of last resort.

For example, instead of generating HTML elements in JavaScript, which makes it hard to change them later:

  • add that HTML to the page, hidden with style="display: none;"
  • when you want to display it, make a copy using .clone(), modify the copy then .show() it
\$\endgroup\$
  • \$\begingroup\$ Thank you for valuable advice but I didn't understand one thing. In your opinion, should I get rid of using jQuery in this project (because it is unnecessary) or should I use it more wherever it's possible? And you are recommending me to change all 'arrow function' to normal 'function ()' ? \$\endgroup\$ – Giovacho May 31 '18 at 16:42
  • \$\begingroup\$ i would recommend use jQuery and the normal function syntax, both for reasons of browser compatibility. Presumably you want as many people as possible to be able to run your code. \$\endgroup\$ – user106794 Jun 1 '18 at 8:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.