1

I'm trying to take the two functions at the bottom of my code: //Add item to To-Do List with "Add" Button and "//Add item to list with ENTER KEY" and add the bulk of these functions to the //Add new item to To-Do List function, so the other functions were simpler and the code for the bottom two functions doesn't repeat.

Any advice would really be appreciated!

//Add new item to To-Do List
function addNewItem(list, itemText) {
  totalItems++;

  var listItem = document.createElement("li");

  list.appendChild(listItem);
}

//Add item to list with ENTER KEY
var totalItems = 0;
var inItemText = document.getElementById("inItemText");
inItemText.focus();
inItemText.onkeyup = function(event) {
  if (event.which === 13) {
    var itemText = inItemText.value;

    if (!itemText || itemText === "") {
      return false;
    }

    addNewItem(document.getElementById("todoList"), itemText);

    inItemText.focus();
    inItemText.select();
  }
}


  //Add item to To-Do List with "Add" Button
  var btnNew = document.getElementById("btnAdd");
  btnNew.onclick = function() {
    var itemText = inItemText.value;

    if (!itemText || itemText === "") {
      return false;
    }


    addNewItem(document.getElementById("todoList"), itemText);

    inItemText.focus();
    inItemText.select();

  }

https://jsfiddle.net/Rassisland/7bkcLfhu/

Rassisland
  • 169
  • 1
  • 3
  • 16
  • Always good idea to create a shorter, simplified example that illustrates your problem. I really doubt all that code is needed to make your point. –  Dec 24 '15 at 22:22
  • Try to delete all the functions that aren't relevant to your question. Other people will understand your code much faster if you only keep the relevant code ! – Nike Sprite Dec 24 '15 at 22:23

1 Answers1

0

Binding the button and enter key at the same time

It would be a good idea to wrap the text input and button in a form-element and bind the addNewItem to the onsubmit event of the form. Then the button and keypress wouldn't need to be handled separately since they'd both initiate the submit event. To achieve the desired functionality with a form you should:

  • Wrap the text input and button in a form element
  • Change the button to a <input type="submit" value="add">
  • Bind the addNewItem function to the onsubmit of the form
  • Call event.preventDefault() in the addNewItem function to prevent the form from making a page load

Refactoring ideas

Another thing you probably want to consider is refactoring the code so that it doesn't use global variables and functions. I've Heard Global Variables Are Bad, What Alternative Solution Should I Use?

I'd also strongly recommend using a JS library or a framework to help with browser compatibility, selectors and so on. If you're worried about loading times, most popular JS libraries can be loaded via a CDN. For example Googles cdn: https://developers.google.com/speed/libraries/ When a library is loaded via a popular cdn, it is highly likely that the users browser already has that resource cached and it doesn't cause any additional loading.

Tuure
  • 521
  • 4
  • 12
  • Hi, thanks for the response! I actually went over all of this yesterday with senior developer, but our work didn't save properly, so I'm trying to re-create what we had. Do you know how I could shorten those bottom two functions and add the work they do to the addNewItem function? – Rassisland Dec 24 '15 at 23:03
  • It seems to me that all the bottom two functions do is make sure the value is not empty and bring the focus back to the input. You could do all of that in the addNewItem function. Basically just copying the first five lines of the onclick function to the beginning of the addNewItem and the last two to the end should do the trick. After that you wouldn't need to pass the itemText to the addNewItem function since it would read it straight from the element. However I do suggest trying to fully understand everything that's going on here rather than just copying and pasting code. – Tuure Dec 24 '15 at 23:35
  • would I have to rename the inItemText and btnNew functions? would I rename them to addNewItem? – Rassisland Dec 29 '15 at 18:43