Skip to main content
Added reference to Batter's answer for type function, added note about rewriting unnecesary parts of the DOM
Source Link

(See Batter's answer for a more simple way to write the type function)

Breaking your code into small bits with a single purpose often improves maintainability of the code. Seperate functions for each task. Note there are probably even more efficient ways then this, but I hope it points you in a direction ;)

Also note that this particular code rewrites the entire list on an addition/deletion, from an performance perspective this might not be optimal.

Breaking your code into small bits with a single purpose often improves maintainability of the code. Seperate functions for each task. Note there are probably even more efficient ways then this, but I hope it points you in a direction ;)

(See Batter's answer for a more simple way to write the type function)

Breaking your code into small bits with a single purpose often improves maintainability of the code. Seperate functions for each task. Note there are probably even more efficient ways then this, but I hope it points you in a direction ;)

Also note that this particular code rewrites the entire list on an addition/deletion, from an performance perspective this might not be optimal.

Source Link

If you notice yourself repeating a particular pattern often, there's probably a way to write it more efficiently (DRY, do not repeat yourself). For example your type function. It could also be more loosely coupled with the rest of your application (if you want to use the type-writing effect elsewhere), and easier to maintain by making it more generic:

// function to add Typewrite effect
const type = function(el, s, interval) {
  const typeInterval = setInterval(function() {
    el.textContent += s.charAt(el.textContent.length);
    if (el.textContent.length == s.length) clearInterval(typeInterval);
  }, interval);
};

type(title, "To-do application", 250);

The type function rewritten, you pass 3 parameters. The element you want to update, the string you want to write and the interval, which determines the speed.

The interval is being cleared as soon as the string written in the dom equals the string passed as param.

Also you might want to check when to use const, and when to use let. At the top of your document you are creating references to DOM elements, such as the addbutton which won't change, your might want to use const.

To give you an idea how to make it more managable I did some refactoring of the JS:

// Instance of a div to add a work
const addButton = document.getElementById("addbutton");

// Instance of work list
const list = document.getElementById("workslist");

// Instance of title div
const title = document.getElementById("text");

// Number of works
let works = [];

// function to add Typewrite effect
const type = function(el, s, interval) {
  const typeInterval = setInterval(function() {
    el.textContent += s.charAt(el.textContent.length);
    if (el.textContent == s) clearInterval(typeInterval);
  }, interval);
};

const renderWorks = function() {
  list.innerHTML = "";
  if (works.length > 0) {
    works.forEach(function(workTitle, i) {
      let li = document.createElement("li");
      let div = document.createElement("div");
      let span = document.createElement("span");

      div.className = "works title";
      div.id = "works" + i;

      span.className = "works remove";
      span.id = "remove" + i;
      span.textContent = "remove";

      span.addEventListener("click", () => removeWork(i));

      div.textContent = workTitle.toUpperCase();

      list
        .appendChild(li)
        .appendChild(div)
        .appendChild(span);
    });
  } else {
    // No works to do
  }
};

const removeWork = function(i) {
  works.splice(i, 1);
  setWorks();
  renderWorks();
};

const addWork = function(work) {
  works.push(work);
  setWorks();
  renderWorks();
};

const getWorks = function(callback) {
  works =
    localStorage.getItem("works").length > 0
      ? localStorage.getItem("works").split(",")
      : [];
  callback();
};

const setWorks = function() {
  localStorage.setItem("works", works.toString());
};

const promptNewWork = function() {
  const work = prompt();
  addWork(work);
};

type(title, "To-do application", 250);
addButton.addEventListener("click", promptNewWork);
getWorks(renderWorks);

Note: I didn't add all the right classname, so disable the CSS to see it working.

Breaking your code into small bits with a single purpose often improves maintainability of the code. Seperate functions for each task. Note there are probably even more efficient ways then this, but I hope it points you in a direction ;)