0

I am trying to make a simple list using html input box and js. the list are creating on clicking "add skill" and get removed when click on "remove". But when I try to add some skill after remove, the last removed item also get back.

   var skillList="";
   var i = 0;
   function addSkill(){
    var skills= document.getElementById("addSkill").value;
    if(skills != ""){
     skillList += "<li><span name='skillItem' id='skillItem"+ i +"'>" + skills + "</span> " +
     "<a onclick='removeSkill()'>remove</a></li>";
     i++;
     document.getElementById("skill").innerHTML = skillList;
     document.getElementById("addSkill").value="";    
    }
   }
   
   function removeSkill(){
    skillList="";
    var items = document.querySelectorAll("#skill li"),index,tab = [];
    for(var j = 0; j < items.length; j++){
     tab.push(items[j].innerHTML);
    }
    for(var j = 0; j < items.length; j++){
     items[j].onclick = function(){
        
      index = tab.indexOf(this.innerHTML);
      items[index].parentNode.removeChild(items[index]);
      tab.splice(j,1);
     };
    }
    console.log(tab);
    for(var j=0; j<tab.length;j++){
     skillList += "<li>" + tab[j] + "</li>";
    }
   }
<td><label>skills:</label></td>
         <td>
          <ul id="skill"></ul>
          <input type="text" name="skill" id="addSkill"/>
          <a onclick="addSkill();" value="">add skill</a>
         </td>

2 Answers2

1

You got all lost in the loops and indexes in your removeSkill function and it's way easier than what you are doing - one single line of code. No loops and no arrays are needed.

You've also got a bunch of other styles of coding that are ancient and should be avoided.

  • You don't need to use a hyperlink just because you want to give someone something to click on. All visible elements support a click event. Only use hyperlinks when you are actually navigating somewhere.
  • You can't use table cells without a table.
  • Don't use inline HTML event attributes (onclick). Separate all your JavaScript from your HTML and do the event binding with .addEventListener().
  • Don't create new HTML by concatenating strings together. It becomes a nightmare fast and requires you to use .innerHTML, which has performance and security implications. Instead, create new DOM objects, configure their properties as needed and then append them into the document.

See the comments inline below:

// Get all the element references you'll need just once:
var skillList = document.querySelector("#skillList");
var newSkill = document.querySelector("#newSkill");
var btnAddSkill = document.querySelector("input[type='button'");

// Do all of your event binding in JavaScript, not with inline HTML event attributes
btnAddSkill.addEventListener("click", addSkill);

function addSkill(){
 if(newSkill.value !== ""){
   // Don't build new HTML by concatenating strings. Create elements and configure them as objects
   var li = document.createElement("li");
   li.textContent = newSkill.value;
   
   // Only use hyperlinks for navigation, not to have something to click on. Any element can be clicked
   var span = document.createElement("span");
   span.classList.add("remove");
   span.textContent = "remove skill";
   span.addEventListener("click", removeSkill);  
   li.appendChild(span);  // Add the span to the bullet
   skillList.appendChild(li); // Add the bullet to the list
   newSkill.value = ""; 
 }
}
   
function removeSkill(){
  // Just remove the closest <li> ancestor to the <span> that got clicked
  skillList.removeChild(this.closest("li"));
}
.remove { display:inline-block; margin-left:10px; padding:2px 4px; background:rgba(200,0,225,.7); color:#ff0; cursor:pointer; }

li {margin:8px 0; border-bottom:1px dashed #e0e0e0; }
<h1>Skills:</h1>
<input type="text" name="skill" id="newSkill">
<input type="button" value="add skill">

<ul id="skillList"></ul>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
0

Just delete this piece of code

                for(var j=0; j<tab.length;j++){
                skillList += "<li>" + tab[j] + "</li>";
            }

Your were adding it again... Working now...

var skillList="";
   var i = 0;
   function addSkill(){
    var skills= document.getElementById("addSkill").value;
    if(skills != ""){
     skillList += "<li><span name='skillItem' id='skillItem"+ i +"'>" + skills + "</span> " +
     "<a onclick='removeSkill()'>remove</a></li>";
     i++;
     document.getElementById("skill").innerHTML = skillList;
     document.getElementById("addSkill").value="";    
    }
   }
   
   function removeSkill(){
    skillList="";
    var items = document.querySelectorAll("#skill li"),index,tab = [];
    for(var j = 0; j < items.length; j++){
     tab.push(items[j].innerHTML);
    }
    for(var j = 0; j < items.length; j++){
     items[j].onclick = function(){
        
      index = tab.indexOf(this.innerHTML);
      items[index].parentNode.removeChild(items[index]);
      tab.pop(j,1);
     };
    }
    

   }
<td><label>skills:</label></td>
         <td>
          <ul id="skill"></ul>
          <input type="text" name="skill" id="addSkill"/>
          <a onclick="addSkill();" value="">add skill</a>
         </td>
mindmaster
  • 1,828
  • 12
  • 22