0

I'm attempting to make a remove button for my dynamically created input boxes, however I can't seem to get it to work. I get the feeling its because the "child" didn't exist upon initially loading the script, but I'm not sure.

Any help would be appreciated. Thanks.

var addButton = document.getElementById("add-button");
var removeButton = document.getElementById("remove-button");
var incomeSection = document.getElementById("income");
var incomeRow = document.getElementById("income-row");
var itemCounter = 2

incomeSection.addEventListener("click", activateItem);

function activateItem(e) {
  if (e.target.id == "add-button") {
    incomeSection.innerHTML += '<div id="income-row"><input class="input" type="text" name="income-type-1" placeholder="Income source ' + itemCounter + '"><span class="currency">£</span><input class="input income-amount" type="text" name="income-amount-1" placeholder="Amount"><input id="remove-button" class="button" type="button" value="-"></div>';
    itemCounter++;
  }

  if (e.target.id == "remove-button") {
    incomeSection.removeChild(incomeRow);
  }
}
html body {
  background-color: #c9c9c9;
  font-family: 'Montserrat', sans-serif;
  color: #686868;
}
#income {
  padding: 10px;
}
.input {
  padding: 8px;
  font-size: 14px;
  margin-right: 10px;
  margin-bottom: 5px;
  width: 30%;
}
.button {
  width: 25px;
  height: 25px;
}
.currency {
  padding: 10px;
  font-size: 14px;
  margin-right: -4px;
  margin-bottom: 5px;
  background-color: darkgray;
  color: white;
}
<div id="income">
  <div id="income-displayed">
    <h2>Income: <span></span></h2>
  </div>
  <div id="income-row">
    <input class="input" type="text" name="income-type-1" placeholder="Income source">
    <span class="currency">£</span>
    <input class="input income-amount" type="text" name="income-amount-1" placeholder="Amount">
    <input id="add-button" class="button" type="button" value="+">
  </div>
</div>
  • 3
    `incomeRow` is defined only once at the beginning of the code. Also, you've to rethink the logic, `id`s must be unique within the document. – Teemu Sep 30 '16 at 13:16
  • 1
    http://stackoverflow.com/questions/203198/event-binding-on-dynamically-created-elements – Vinay Sep 30 '16 at 13:17
  • When you did `incomeSection.innerHTML += '...'`, you destroyed the `incomeRow` and recreated it. Yes, IDs must be unique, but that's not the problem here. –  Sep 30 '16 at 13:19
  • You have to use class instead of id because you are adding number of rows there so Id will be repeated and why you are not using jQuery, is it easily done by that. – Harsh Sanghani Sep 30 '16 at 13:19
  • @HarshSanghani: There are many reasons not to use jQuery, and replicating this code in jQuery would fail too. –  Sep 30 '16 at 13:20
  • @Novice: He's not trying to bind an event to a dynamically created element; he's already using event delegation in his code. –  Sep 30 '16 at 13:21
  • @squint Ok Ok I got it, No issue – Harsh Sanghani Sep 30 '16 at 13:24
  • Can you please try my answer? – Harsh Sanghani Sep 30 '16 at 13:24
  • Just using javascript because I'm in the process of learning it. jQuery would be faster, but I think I'm probably better learning javascript first because it will make me a better developer. Also thank you, I'll keep that in mind for ID/classes. – Paul O'Brien Sep 30 '16 at 13:30
  • @squint The `id`s are currently not a problem, but when the code is fixed, it might become very actuall, depending on how the code will be fixed, ofcourse. – Teemu Sep 30 '16 at 13:30
  • @Teemu: Right, it definitely needs to be corrected. Trouble here seems to be removing an element that has been already removed by the `.innerHTML +=` destruction. –  Sep 30 '16 at 13:34
  • @PaulO'Brien: jQuery isn't faster. –  Sep 30 '16 at 13:34

2 Answers2

0

Just replace the following line it will resolved your issue :-

incomeSection.removeChild(incomeRow);

with the following

e.target.parentNode.remove();

var addButton = document.getElementById("add-button");
var removeButton = document.getElementById("remove-button");
var incomeSection = document.getElementById("income");
var incomeRow = document.getElementById("income-row");
var itemCounter = 2

incomeSection.addEventListener("click", activateItem);

function activateItem(e) {
    if (e.target.id == "add-button") {
        incomeSection.innerHTML += '<div id="income-row"><input class="input" type="text" name="income-type-1" placeholder="Income source '+itemCounter+'"><span class="currency">£</span><input class="input income-amount" type="text" name="income-amount-1" placeholder="Amount"><input id="remove-button" class="button" type="button" value="-"></div>'; itemCounter ++;
    }
    
    if (e.target.id == "remove-button") {
        e.target.parentNode.remove();
    }
}
html body {
  background-color: #c9c9c9;
  font-family: 'Montserrat', sans-serif;
  color: #686868;
}
#income {
  padding: 10px;
}
.input {
  padding: 8px;
  font-size: 14px;
  margin-right: 10px;
  margin-bottom: 5px;
  width: 30%;
}
.button {
  width: 25px;
  height: 25px;
}
.currency {
  padding: 10px;
  font-size: 14px;
  margin-right: -4px;
  margin-bottom: 5px;
  background-color: darkgray;
  color: white;
}
<div id="income">
  <div id="income-displayed"><h2>Income: <span></span></h2></div>
  <div id="income-row">
    <input class="input" type="text" name="income-type-1" placeholder="Income source"><span class="currency">£</span><input class="input income-amount" type="text" name="income-amount-1" placeholder="Amount"><input id="add-button" class="button" type="button" value="+"></div>
</div>
Harsh Sanghani
  • 1,666
  • 1
  • 14
  • 32
  • Thank you, it worked. So I assume I have to refer to it in this manner because it was created dynamically? I'll read into this more, thanks again. – Paul O'Brien Sep 30 '16 at 13:34
  • @PaulO'Brien: It's important to understand the underlying problem. Your `incomeRow` variable refers to the original `income-row` element. When you did `incomeSection.innerHTML += '...'`, you ***destroyed*** the original content of `incomeSection` and replaced it with almost identical new nodes. That means your `incomeSection` variable now refers to an old node that is no longer in the DOM. That's why it's just such a bad idea to use `+=` on `.innerHTML`. Also, in your demo, type some text into the first row before you make the new row. See it disappear? That's another side-effect. –  Sep 30 '16 at 13:44
  • 1
    So what HarshSanghani's answer does is to fetch the element *relative* to the one that was clicked. This would've been needed to be done no matter what, because your `incomeRow` variable was still only referencing that first row. So even if you didn't destroy the original `income-row` by your use of `.innerHTML +=`, it would still have been wrong because the remove button would have been targeting that first row. –  Sep 30 '16 at 13:46
  • 1
    ...while I personally don't like using HTML to create elements, you can use the [`.insertAdjacentHTML()`](https://developer.mozilla.org/en-US/docs/Web/API/Element/insertAdjacentHTML) method to do a non-destructive addition of new elements from HTML. This avoids serializing, destroying, recreating and inserting the original nodes, which leads to all sorts of problems. –  Sep 30 '16 at 13:48
  • @PaulO'Brien thank you for accepting answer, If you up vote also in the answer then its batter if Its solved your problem. – Harsh Sanghani Oct 01 '16 at 03:47
0

This Snippet demonstrates how to add and remove elements to and from the DOM utilizing:

Further details are commented in the source

SNIPPET

// #incFormA element will bind with eventListener
var form = document.getElementById("incFormA");
// #setA will contain the #dupes
var set = document.getElementById("setA");
// Reference to clone template #dupe
var dupe = document.getElementById('dupe');
// Increment counter that will ensure unique ids
var cntr = 0;
// If any part of the form is clicked call actDeact
form.addEventListener("click", actDeact);

/*
| actDeact()
| -If the clicked button's id === addA
| then clone #dupe and add an id (#dupe+cntr)
| -Iterate through it's children
| and assign unique ids and names to 
| the 1st and 3rd children of (#dupe+cntr)
| -Append #dupe+cntr to #setA
*/
/*
| -If the clicked button was a .remA
| find it's parentNode and remove it.
*/
function actDeact(e) {
  cntr++;
  if (e.target.id === 'addA') {

    var clone = dupe.cloneNode(true);
    clone.id = 'dupe' + cntr;
    var i;
    var grp = clone.children;
    var qty = grp.length;

    for (i = 0; i < qty; i++) {
      switch (i) {
        case 0:
          grp[0].id = 'inc' + cntr;
          grp[0].name = 'inc' + cntr;
          break;
        case 2:
          grp[2].id = 'amt' + cntr;
          grp[2].name = 'amt' + cntr;
          break;
        default:
          break;
      }

    }
    setA.appendChild(clone);

  } else if (e.target.className === "remA") {

    var clone = e.target.parentNode;
    setA.removeChild(clone);

  } else return false;
}
html body {
  background-color: #c9c9c9;
  font-family: 'Montserrat', sans-serif;
  color: #686868;
}

#setA {
  padding: 10px;
}


/* The clone template is out of the way 
| yet still accessible for cloning.
*/

#dupe {
  display: none;
}

.input {
  padding: 8px;
  font-size: 14px;
  margin-right: 10px;
  margin-bottom: 5px;
  width: 30%;
}

button {
  width: 25px;
  height: 25px;
  float: right;
}

.currency {
  padding: 10px;
  font-size: 14px;
  margin-right: -4px;
  margin-bottom: 5px;
  background-color: darkgray;
  color: white;
}


/*
| This ruleset is for positioning #addA
*/

.block {
  position: relative;
  display: block;
  min-height: 100%;
  width: 20%;
  left: 80%;
  bottom: 15px;
}
<!--Use <form> to post collect and send data*-->
<form id='incFormA' name='incFormA'>
  <fieldset id="setA">

    <legend class="display">
      <!--Use <output> to calculate and display data*-->
      <h2>Income: <output id='outA'></output></h2>
    </legend>
    <div class='block'>
      <!--One add <button>-->
      <button id="addA" type='button'>+</button>
    </div>
    <!--#dupe is the group of elements kept as a clone template-->
    <span id='dupe'>
      <input id='inc' class="input" name="inc" type="text" placeholder="Income source">
      <span class="currency">£</span>
    <input id="amt" class="input" name="amt" type="text" placeholder="Amount">
    <button class="remA">-</button>
    </span>

  </fieldset>
</form>

<!--*Choice of elements optional-->
Community
  • 1
  • 1
zer00ne
  • 41,936
  • 6
  • 41
  • 68