0

I have a js file that is supposed to create a list of things with a delete button

The list gets created, and the first time that an element is deleted, it works fine. However, when I click on the delete button the second time, nothing happens. Why is this?

let sales = [{
    "salesperson": "James D. Halpert",
    "client": "Shake Shack",
    "reams": 100
  },
  {
    "salesperson": "Stanley Hudson",
    "client": "Toast",
    "reams": 400
  },
  {
    "salesperson": "Michael G. Scott",
    "client": "Computer Science Department",
    "reams": 1000
  },
];

function makeEntries(sales) {
  $("#main").empty()
  $.each(sales, function(index, value) {

    var outer = $("<div class='main-page'>")
    var start = $("<div class='start'></div>")
    start.html(value["salesperson"])
    var nameAttr = index;
    var mid1 = $("<div class='mid-1'></div>")
    mid1.html(value["client"])
    var mid2 = $("<div class='mid-2'></div>")
    mid2.html(value["reams"])
    var ends1 = $("<div class='ends'>")
    var ends = $("<button class='delete-button' id='" + nameAttr + "'></button></div>")
    outer.append(start)
    outer.append(mid1)
    outer.append(mid2)
    outer.append(ends1)
    outer.append(ends)
    $("#main").prepend(outer)

  })
}
$(document).ready(function() {

  makeEntries(sales)
  console.log("does this get hoisted?")
  console.log(sales)


  $(".delete-button").click(function() {
    names = Number(this.id)
    sales.splice(names, 1)
    makeEntries(sales)
  })

})
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.6.0/jquery.min.js"></script>

<div id="main"></div>
mplungjan
  • 169,008
  • 28
  • 173
  • 236
Niki
  • 173
  • 10
  • You need *event delegation*. `$(".delete-button").click(function() {` -> `$(document).on("click", ".delete-button", function() {` – freedomn-m Feb 15 '22 at 11:26
  • `makeEntries(sales)` is called *before* `$(".delete-button").click` so would not need event delegation, *however*, delete-button click also calls `makeEntries` so will create new `.delete-button`s which won't have the event handler, so *do* need event delegation. – freedomn-m Feb 15 '22 at 11:28

2 Answers2

1

You need an updated index of the Object in Array. The simplest way to achieve it is: sales.splice(sales.indexOf(item), 1);

Don't rebuild your entire elements again and again. Just remove that single one both from DOM and Array.

Regarding your code, here's a better way to code it using jQuery:

const sales = [{
  salesperson: "James D. Halpert",
  client: "Shake Shack",
  reams: 100,
}, {
  salesperson: "Stanley Hudson",
  client: "Toast",
  reams: 400,
}, {
  salesperson: "Michael G. Scott",
  client: "Computer Science Department",
  reams: 1000,
}];

const $main = $("#main");

const makeEntries = (sales) => {
  const outers = sales.map(item => {
    const $outer  = $("<div>", {class:"outer"});
    const $person = $("<div>", {text: item.salesperson, class: "start", appendTo: $outer});
    const $client = $("<div>", {text: item.client, class: "mid-1", appendTo: $outer});
    const $reams  = $("<div>", {text: item.reams, class: "mid-2", appendTo: $outer});
    const $delete = $("<button>", {
      text: "Delete",
      class: "delete-button",
      appendTo: $outer,
      on: {
        click() {
          sales.splice(sales.indexOf(item), 1);
          $outer.remove();
          console.log(sales);
        }
      }
    });
    return $outer;
  });
  
  $main.empty().append(outers);
};

jQuery($ => {
  makeEntries(sales);
});
#main { display: flex; }
.outer {flex: 1; padding: 10px; border: 1px solid #aaa;}
<main id="main"></main>
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
Roko C. Buljan
  • 196,159
  • 39
  • 305
  • 313
0

You rewrite the list each time but only add the eventlistener the very first time

Instead delegate

$("#main").on("click",".delete-button", function() {
  const names = Number(this.id); 
  sales.splice(names, 1);
  makeEntries(sales)
})

let sales = [{
    "salesperson": "James D. Halpert",
    "client": "Shake Shack",
    "reams": 100
  },
  {
    "salesperson": "Stanley Hudson",
    "client": "Toast",
    "reams": 400
  },
  {
    "salesperson": "Michael G. Scott",
    "client": "Computer Science Department",
    "reams": 1000
  },
];

function makeEntries(sales) {
  $("#main").empty()
  $.each(sales, function(index, value) {
    var outer = $("<div class='main-page'>")
    var start = $("<div class='start'></div>")
    start.html(value["salesperson"])
    var nameAttr = index;
    var mid1 = $("<div class='mid-1'></div>")
    mid1.html(value["client"])
    var mid2 = $("<div class='mid-2'></div>")
    mid2.html(value["reams"])
    var ends1 = $("<div class='ends'>")
    var ends = $("<button class='delete-button' id='" + nameAttr + "'></button></div>")
    outer.append(start)
    outer.append(mid1)
    outer.append(mid2)
    outer.append(ends1)
    outer.append(ends)
    $("#main").prepend(outer)

  })
}
$(document).ready(function() {

  makeEntries(sales)
  console.log("does this get hoisted?")
  console.log(sales)


  $("#main").on("click",".delete-button", function() {
    const names = Number(this.id); 
    sales.splice(names, 1);
    makeEntries(sales)
  })
})
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.6.0/jquery.min.js"></script>

<div id="main"></div>

Alternatively do not rewrite

$(".delete-button").on("click", function() {
  const names = Number(this.id); 
  sales.splice(names, 1);
  $(this).closest(".main-page").remove()
})

let sales = [{
    "salesperson": "James D. Halpert",
    "client": "Shake Shack",
    "reams": 100
  },
  {
    "salesperson": "Stanley Hudson",
    "client": "Toast",
    "reams": 400
  },
  {
    "salesperson": "Michael G. Scott",
    "client": "Computer Science Department",
    "reams": 1000
  },
];

function makeEntries(sales) {
  $("#main").empty()
  $.each(sales, function(index, value) {
    var outer = $("<div class='main-page'>")
    var start = $("<div class='start'></div>")
    start.html(value["salesperson"])
    var nameAttr = index;
    var mid1 = $("<div class='mid-1'></div>")
    mid1.html(value["client"])
    var mid2 = $("<div class='mid-2'></div>")
    mid2.html(value["reams"])
    var ends1 = $("<div class='ends'>")
    var ends = $("<button class='delete-button' id='" + nameAttr + "'></button></div>")
    outer.append(start)
    outer.append(mid1)
    outer.append(mid2)
    outer.append(ends1)
    outer.append(ends)
    $("#main").prepend(outer)

  })
}
$(document).ready(function() {

  makeEntries(sales)
  console.log("does this get hoisted?")
  console.log(sales)


  $(".delete-button").on("click", function() {
    const names = Number(this.id); 
    sales.splice(names, 1);
    $(this).closest(".main-page").remove()
  })
})
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.6.0/jquery.min.js"></script>

<div id="main"></div>
mplungjan
  • 169,008
  • 28
  • 173
  • 236
  • 2
    I, too, was a bit surprised to see `makeEntries` in the delete function (didn't expect it there) - OPs code would benefit from removing just the line being clicked. – freedomn-m Feb 15 '22 at 11:31