To-do list app using jQuery

Question

I created a simple todo kind of app using JavaScript. It is working fine but the code which I wrote is not proper. Please review my code and improve it if it is needed.

$(function () {
    var todo = new Todo('contents');

    $('.addBtn').on('click', function() {
        var name = $(this).parent().find('input[type="text"]').val();
        todo.add(name);
    });

    $('.contents').on('click', '.remove', function() {
        var el = $(this).parent();
        todo.remove(el);
    });

    $('.contents').on('click', '.update', function() {
        var dom = $(this);
        todo.addUpdateField(dom);
    });

    $('.contents').on('click', '.updateBtn', function() {
        var el = $(this);
        todo.update(el);
    });
});

Here is my todo.js file

var Todo = function(c) {
    var contents = $('.' + c),
    name;

    add = function (name) {
        if(name != "") {
            var div = $('<div class="names"></div>');
            div.append('<span>' + name + '</span>');
            div.append("<button class='update' class='update'>Edit</button>");
            div.append("<button class='remove' name='remove'>Remove</button>");
            contents.prepend(div);
            $('.name').val('').focus();
        }
        return;
    },

    addUpdateField = function (dom) {
        var name = dom.parent().find('span').text(),
            field = $('<input type="text" value="' + name + '" />'),
            update = $('<button class="updateBtn">Update</button>');

        return dom.parent().html('').append(field).append(update);
    },

    update = function(el) {
        var val = el.parent().find('input').val();
        el.parent().html('<span>' + val + '</span>')
        .append('<button class="update" class="update">Edit</button>')
        .append('<button class="remove" class="remove">Remove</button>');
    },

    remove = function (el) {
        return el.remove();
    };

    return {
        add             : add,
        update          : update,
        remove          : remove,
        addUpdateField  : addUpdateField
    };
};

Answer 1

As I read it, you want:

• one <div class="names"> per todo (in this case inside .contents) surrounding the todo text in a span and the buttons "Edit" / "Remove"
• on clicking "Edit", the span is exchanged for an input field and an "Update" button, "Edit" and "Remove" are removed.
• on clicking "Update", the prior state with the new text is recreated (span, Edit, Remove)

After you follow Inkbug`s advice from his comment, you should probably do this:

• cache the todo root element (var todoRoot = $(...);)
• rename addUpdateField/.update to edit/.edit and updateBtn to update - consistent with the button texts and easier to read

• change the api of your todo.js functions. For update / edit / remove, just use the dom node of the div as the argument:

  function(target, action) {
      todoRoot.on('click', target, function(e) {
          e.preventDefault();
          e.stopPropagation();
          action.call(todo, $(this).parents('.names').first());
      });
  }
  
  bindClick('.remove', todo.remove);
  bindClick('.edit',   todo.edit);
  bindClick('div',     todo.update);
  

• don't use that many appends. Put it all in a string (that can contain more than one DOM node!)

• don't create / delete all the nodes. Create all three buttons, use .toggleClass(...) and CSS to make all buttons/fields disappear that you don't need. That moves all your HTML into the add function and goes easy on the DOM
• move the bindings into the Todo function. You don't want to have to remember to bind these events for every Todo list you create