3
\$\begingroup\$

This is how my app looks like:

Screenshot

I can add and remove tasks, mark them as complete, move them up and down on a list, and filter them.

I have few questions about some parts of my code that I'm pasting here.

First thing that concerns me is that in addTask() function I have tons of document.createElement one by one and it looks like a wall of code. Is this ok?

Second thing that makes me wonder is that in the same function (addTask) I am appending many things and it feels like I am repeating myself. Is there a cleaner solution?

My third question is about editTask() function. I feel it needs lots of improvement, but I don't really know how to do it. For example two lines are setting my inputButton.style. Should I move them to CSS file and set style just to get one line of code less?

Of course if there are any mistakes please point them out!

const vApp = {

addTask: () => {
    const taskText = document.getElementById('newTask'),
        contentBox = document.getElementById('tasksholder'),
        taskCreator = document.createElement('div'),
        textBox = document.createElement('div'),
        deleteButton = document.createElement('button'),
        editButton = document.createElement('button'),
        downButton = document.createElement('button'),
        upButton = document.createElement('button'),
        checkBox = document.createElement('input'),
        label = document.createElement('label');

    if (taskText.value.length > 0) {        // protection against empty input
        taskCreator.className = 'task-box';
        textBox.innerHTML = taskText.value;
        textBox.className = 'textBox';
        contentBox.insertBefore(taskCreator, contentBox.childNodes[0]);
        taskText.value = '';                // clear input box

        deleteButton.style.backgroundImage = "url('img/delete.png')";
        deleteButton.addEventListener('click', function() {
            contentBox.removeChild(this.parentNode);
        });

        editButton.style.backgroundImage = "url('img/edit.png')";
        editButton.addEventListener('click', function() {
            vApp.disableEditing(this.parentNode, true);
            vApp.editTask(this.parentNode);
        });

        downButton.style.backgroundImage = "url('img/down.png')";
        downButton.addEventListener('click', function() {
            if (this.parentNode ===  contentBox.lastChild) {
                return false;
            } else {
                vApp.swapElements(this.parentNode, this.parentNode.nextSibling);
            }
        });

        upButton.style.backgroundImage = "url('img/up.png')";
        upButton.addEventListener('click', function() {
            if (this.parentNode ===  contentBox.firstChild) {
                return false;
            } else {
                vApp.swapElements(this.parentNode, this.parentNode.previousSibling);
            }
        });

        checkBox.type = "checkbox";
        checkBox.addEventListener('click', function() {
           vApp.manageCheckbox(this.parentNode);
        });

        label.appendChild(checkBox);
        taskCreator.appendChild(textBox);
        taskCreator.appendChild(label);
        taskCreator.appendChild(downButton);
        taskCreator.appendChild(upButton);
        taskCreator.appendChild(editButton);
        taskCreator.appendChild(deleteButton);
        taskText.placeholder = 'What else needs to be done?';

    } else {
        vApp.warn();
    }
},

disableEditing: (buttonParent, enabling) => {
    buttonParent.childNodes[4].disabled = enabling;
},

editTask: (taskToEdit) => {
    const bubble = document.createElement('span'),
        inputField = document.createElement('input'),
        inputButton = document.createElement('button'),
        textContainer = document.createElement('div');

    inputField.value = taskToEdit.innerText;
    inputField.setAttribute('type', 'text');
    inputField.setAttribute('maxlength', '40');
    taskToEdit.removeChild(taskToEdit.firstChild);
    textContainer.className = 'textBox';
    bubble.className = 'bubble';
    inputButton.innerText = 'Ok';
    inputButton.style.fontSize = '18px';
    inputButton.style.color = "#dddddd";
    inputButton.addEventListener('click', function() {
        textContainer.innerHTML = inputField.value;
        taskToEdit.insertBefore(textContainer, taskToEdit.childNodes[0]);
        vApp.disableEditing(taskToEdit,false);
        bubble.parentNode.removeChild(bubble);
    });
    bubble.appendChild(inputField);
    bubble.appendChild(inputButton);
    taskToEdit.appendChild(bubble);
},


};
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
  1. If you want, you can always refactor functions like document.getElementById() so that you type less. Something that I see a lot of people do is:

    const E = id => document.getElementById(id);
    
    ...
    
    const taskText = E('newTask'),
    ...
    
  2. Same thing for document.createElement():

    const Element = tag => document.createElement(tag);
    
  3. Anything involving styles should probably be in an external CSS stylesheet.

  4. You can paste your HTML into the question and make a runnable snippet so that we can try out your application.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Is 1. and 2. really a good practice? It seems like making code less verbal. \$\endgroup\$
    – Yewhral
    Jul 16, 2017 at 7:52
  • \$\begingroup\$ JavaScript is VERY verbal, almost too verbal when it comes to this. You can choose a different name for the alias to make it more readable to you if you want. \$\endgroup\$
    – omgimanerd
    Jul 17, 2017 at 18:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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