0

I am developing A tasks list app with pure js. I am trying to add a functionality that help user to move tasks up and down. User shold be able to move a particular task one step up or down when they click on the certain button. I can do this with jquery but have no idea about how to do this with pure js. Here is the DEMO

Could someone please help me and how can i improve the quality of my code

HTML

<div class="task-list-app">

        <header class="main-header">
            <h1>Tasks list <span class="version"></span></h1>

        </header>

        <section class="main-section">

            <div>
                <input type="text"  id="add-new-task" name="add-new-task" placeholder="Type task name and hit Enter ">
            </div>

           <ul id="tasks-list"></ul>

        </section>

        <div class="status-bar">
            <span class="app-status"></span>
        </div>
    </div>

JS

function TaskList() {
    "use strict";
    var appVersion = '1.0.0v';

    function setAppStatus($status) {

        var $appStatusElm = document.querySelector(".app-status");

        $appStatusElm.textContent = $status;        


    }

    function addTask() {

        var $addNewTask = document.querySelector("#add-new-task");

        $addNewTask.addEventListener("keypress", function (e) {



            if (e.which == 13) {
                var taskName = this.value;

                appendNewTask(taskName);

                this.value = '';
            }

            return false;

        });


    }

    function appendNewTask (taskName) {

        var $taskList = document.querySelector("#tasks-list");
        var $taskElm = document.createElement("li");
        var $btnsWrapper = document.createElement("div");
        var $deleteBtn = document.createElement('a');        
        var $moveUpBtn = document.createElement('a');       
        var $moveDownBtn = document.createElement('a');
        var $taskNameElm = document.createElement('span');
        var hrefAttr = document.createAttribute("href");
        var buttons = [];

        hrefAttr.nodeValue ="#";

        $taskElm.appendChild($taskNameElm);        
        $taskNameElm.textContent = taskName;        
        $taskList.appendChild($taskElm);
        $taskElm.appendChild($btnsWrapper);

        setTimeout(function () {
            $taskElm.classList.add("remove-border");
        }, 300);


        $deleteBtn.textContent = "Delete";        
        var $deleteBtnHref = hrefAttr.cloneNode(true);
        $deleteBtn.setAttributeNode($deleteBtnHref);



        $moveUpBtn.textContent = "Move up";  
        var $moveUpBtnHref = hrefAttr.cloneNode(true);
        $moveUpBtn.setAttributeNode($moveUpBtnHref);

        $moveDownBtn.textContent = "Move down";  
        var $moveDownBtnHref = hrefAttr.cloneNode(true);
        $moveDownBtn.setAttributeNode($moveDownBtnHref);

        buttons = [$deleteBtn, $moveDownBtn, $moveUpBtn];

        buttons.forEach(function (btn) {
            $taskElm.querySelector('div').appendChild(btn);
        });

        $deleteBtn.addEventListener('click', function (e) {
            e.path[3].removeChild(e.path[2]);
            setAppStatus("Task " + taskName + " has been removed");
        })

        setAppStatus("Task " + taskName + " has been recorded");

    }

    this.start = function () {
        var $versionElm = document.querySelector(".main-header  .version");


        $versionElm.textContent = appVersion;

        addTask();

        setAppStatus("Ready To Use");

    };
}


var app = new TaskList();
app.start();
It worked yesterday.
  • 4,507
  • 11
  • 46
  • 81
  • 1
    This question has been answered perfectly here: http://stackoverflow.com/questions/2943140/how-to-swap-html-elements-in-javascript Up to you :) I've been crawling StackOverflow for years, but only recently started contributing and posting. Not sure what the conventions are. If you come up with a solution based on that post, you can always come back here and answer your own question to help anyone that may come across this one as opposed to the other. – Jonny Asmar Oct 25 '15 at 03:49
  • Ok still a part of my question is unanswered.. How do i improve the quality of the code. so i am going to leave it – It worked yesterday. Oct 25 '15 at 03:56

2 Answers2

2

Here is how you would want to swap out the entire li's when they select move Up. (to achieve this for move down you would just do nextSibiling instead of previousSibiling);

Here is a working JSFiddle with the code below

$moveUpBtn.addEventListener('click',function(e){
        var $mvTaskList = e.target.closest('ul');
        var $mvTask = e.target.closest('li');
        var $prevTask = e.target.closest('li').previousSibling;
        if(typeof($prevTask)!=='undefined' && $prevTask!==null){
          $mvTaskList.insertBefore($mvTask, $prevTask);
        }
    });

As for improving your code - take consideration on grouping your code based on what is going on as well so that all of your declarations for mvUpBtn are grouped instead of spread throughout the codebase. Another way is to limited the createElement actions until you have the entire button created - although you don't have a reason in this example for the node to be abandoned but in other scenarios there would be creating a node based on some conditionals and fail out (abandon the node) after you failed a conditional. Appending to an object is much less overhead than creating a full DOM node.

var mvUpBtn = 'a';
mvUpBtn.textContent = 'Move Up';
mvUpBtn.setAttributeNode(hrefAttr.cloneNode(true));
var $mvUpBtn = document.createElement(mvUpBtn);
ioneyed
  • 1,052
  • 7
  • 12
1

One major improvement I can think of for your code would be to wrap it in a closure. The way it's written, TaskList() is accessible everywhere and can be overwritten, modified, or overwrite other methods.

Wrap your entire script in something like this:

;(function(){
    // Your code here
});

This will give you a few advantages. First the colon at the beginning will ensure any other closures where the script is injected are closed. It will also give you a private scope for your code, allowing you to set global constants/variables that can be used by other Classes you set up within your code that won't collide with any other variables present on the page executing the script.

Jonny Asmar
  • 1,900
  • 14
  • 16