Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript

Todo List - Adding multiple events with Minimal code

I have a todo list app which you can edit, delete, complete, uncomplete tasks.

Code originates from "Interactive Web Pages with JavaScript" Course by Andrew Chalkley.

My first problem was that when you are editing a task you can only confirm save by clicking the edit button. Then I decided to add if user pressed enter then it also saves edited task. But I would say that I did some code repeating. So how would I make my code more DRY but still readable, especially how I wrote editTask and editTaskKeyboard functions.

var taskInput = document.getElementById('item');
var addButton = document.getElementById('add');
var todoList  = document.getElementById('todo');
var completedList = document.getElementById('completed');


var createNewTask = function(text) {

    var listItem = document.createElement('li');
    listItem.classList.add('todo-item');

    var label = document.createElement('label');
    label.innerText = text;

    var editInput = document.createElement('input');
    editInput.type = 'text';

    var buttons = document.createElement('div');
    buttons.classList.add('item-buttons');

    var editButton = document.createElement('button');
    editButton.classList.add('item-button', 'edit');

    var deleteButton = document.createElement('button');
    deleteButton.classList.add('item-button', 'delete');

    var completeButton = document.createElement('button');
    completeButton.classList.add('item-button', 'complete');

    buttons.appendChild(editButton);
    buttons.appendChild(deleteButton);
    buttons.appendChild(completeButton);

    listItem.appendChild(label);
    listItem.appendChild(editInput);
    listItem.appendChild(buttons);

    return listItem;

};

var addTask = function() {

    var listItem = createNewTask(taskInput.value);
    todoList.insertBefore(listItem, todoList.childNodes[0]);
    taskEvents(listItem, taskCompleted);

    taskInput.value = '';

};

var editTask = function() {

    var listItem = this.parentNode.parentNode;

    var editInput = listItem.querySelector('input[type=text]');
    var label = listItem.querySelector('label');

    var containsClass = listItem.classList.contains('edit-mode');

    if (containsClass) {

        label.innerText = editInput.value;

    } else {
        editInput.value = label.innerText;
    }

    listItem.classList.toggle('edit-mode');

    editInput.focus();

};

var editTaskKeyboard = function() {

    var editInput = document.activeElement;
    var listItem = editInput.parentNode;

    var label = listItem.querySelector('label');
    var containsClass = listItem.classList.contains('edit-mode');

    if (containsClass) {

        label.innerText = editInput.value;

    } else {
        editInput.value = label.innerText;
    }

    listItem.classList.toggle('edit-mode');

};

var deleteTask = function() {

    var listItem = this.parentNode.parentNode;
    var ul = listItem.parentNode;

    ul.removeChild(listItem);

};

// Mark a task as complete 
var taskCompleted = function() {

    var listItem = this.parentNode.parentNode;
    completedList.insertBefore(listItem, completedList.childNodes[0]);
    taskEvents(listItem, taskIncomplete);

};

// Mark a task as incomplete
var taskIncomplete = function() {

    var listItem = this.parentNode.parentNode;
    todoList.insertBefore(listItem, todoList.childNodes[0]);
    taskEvents(listItem, taskCompleted);

};

// Task Events binded to items
var taskEvents = function(taskItem, checkBoxEventHandler) {

    //select taskItem's children
    var editItem = taskItem.querySelector('.item-button.edit');
    var editInput = taskItem.querySelector('input[type=text]');
    var deleteItem = taskItem.querySelector('.item-button.delete');
    var completeItem = taskItem.querySelector('.item-button.complete');

    //bind editTask to edit button
    editItem.onclick = editTask;

    editInput.addEventListener('keydown', function (e) {

        if (e.code === 'Enter' && editInput.value ) {
            editTaskKeyboard();
        }

    });

    //bind deleteTask to delete button
    deleteItem.onclick = deleteTask;

    //bind completItem to complete button
    completeItem.onclick = checkBoxEventHandler;

};

// Set the click handler to the addTask function
addButton.addEventListener('click', addTask);

// Set keydown 'Enter' handler to the addTask function
taskInput.addEventListener('keydown', function (e) {

    if (e.code === 'Enter' && taskInput.value ) {
        addTask();
    }

});

HTML:

<body>
    <header>
        <div class="wrapper">
            <input type="text" placeholder="Enter text here.." id="item">
            <button id="add">
            </button>
        </div>
    </header>
    <div class="wrapper">

        <div class="container">
            <!-- Uncompleted tasks -->
            <ul class="todo" id="todo"></ul>

            <!-- Completed tasks -->
            <ul class="todo" id="completed"></ul>
        </div>
    </div>

    <script src="js/main.js"></script>

</body>

3 Answers

Still not working. Correct code was this:

var editTask = function(item, keyboard) {

    var editInput, listItem;

    if (keyboard) {

        editInput = document.activeElement;
        listItem = editInput.parentNode;

    } else {

        listItem = item.parentNode.parentNode; // if it was 'this' instead of function parameter, then it would have called the window object
        editInput = listItem.querySelector('input[type=text]');

    }

    var label = listItem.querySelector('label');
    var containsClass = listItem.classList.contains('edit-mode');

    if (containsClass) {

        label.innerText = editInput.value;

    } else {

        editInput.value = label.innerText;

    }

    listItem.classList.toggle('edit-mode');
    if (!keyboard) editInput.focus();

};

For click event had to be used addEventListener and function argument must be passed as 'this'. Otherwise it would again call the window object and therefore would not work.

    //bind editTask to edit button
    editItem.addEventListener('click', function () {
        editTask(this);
    });

    editInput.addEventListener('keydown', function (e) {

        if (e.code === 'Enter' && editInput.value ) {
            editTask(this, true);
        }

    });

But still I appreciate your help Steven Parker. Thank you.

Steven Parker
Steven Parker
243,644 Points

Good idea, I was thinking you could avoid the second anonymous function, but I didn't actually test it. But the main thing was the idea of consolidating the functions.

It occurred to me later that you could use event.type to trigger the differences and not need another argument at all.

Steven Parker
Steven Parker
243,644 Points

You could consolidate two functions into one.

When you have two functions that are mostly the same, one option for compacting is to have one function where the differences are controlled by conditional statements that use a passed argument as the condition. For example:

function editTask(keyboard) {
  var editInput, listItem;
  if (keyboard) {
    editInput = document.activeElement;
    listItem = editInput.parentNode;
  } else {
    listItem = this.parentNode.parentNode;
    editInput = listItem.querySelector("input[type=text]");
  }
  var label = listItem.querySelector("label");
  var containsClass = listItem.classList.contains("edit-mode");

  if (containsClass) {
    label.innerText = editInput.value;
  } else {
    editInput.value = label.innerText;
  }
  listItem.classList.toggle("edit-mode");
  if (!keyboard) editInput.focus();
};

And then when you establish the handlers...

            editTask(true);                         // was "editTaskKeyboard()"

It says it can't read parentNode of undefined at editTask

function editTask(keyboard) {

    var editInput, listItem;

    if (keyboard) {

        editInput = document.activeElement;
        listItem = editInput.parentNode;

    } else {

        listItem = this.parentNode.parentNode; // This parentNode
        editInput = listItem.querySelector('input[type=text]');

    }

    var label = listItem.querySelector('label');
    var containsClass = listItem.classList.contains('edit-mode');

    if (containsClass) {

        label.innerText = editInput.value;

    } else {

        editInput.value = label.innerText;

    }

    listItem.classList.toggle('edit-mode');
    if (!keyboard) editInput.focus();

}

taskEvents function is this:

var taskEvents = function(taskItem, checkBoxEventHandler) {

    var editItem = taskItem.querySelector('.item-button.edit');
    var editInput = taskItem.querySelector('input[type=text]');
    var deleteItem = taskItem.querySelector('.item-button.delete');
    var completeItem = taskItem.querySelector('.item-button.complete');

    //bind editTask to edit button
    editItem.onclick = editTask.bind(null, false);

    editInput.addEventListener('keydown', function (e) {

        if (e.code === 'Enter' && editInput.value ) {
            editTask(true);
        }

    });

    deleteItem.onclick = deleteTask;
    completeItem.onclick = checkBoxEventHandler;

};
Steven Parker
Steven Parker
243,644 Points

I was counting on the argument to come in as "undefined" for the button, which should be handled as "falsey". I didn't actually reconstruct the workspace to test it out .. I mainly wanted to illustrate the concept of merging functions.