JavaScript delete modal onclick and decrease counter

Question

I want to make a basic inbox function. It contains 3 messages. So I want to make that when the user click onto the DELETE button, set the msg1's display to none, and decrease the messages value. Here is the example code:

var x = 2;

function deleteMsg1() {
  
  var msg1 = document.getElementsByClassName("cont");
    if (confirm("Are you sure to want to delete this message?")) {
      msg1[0].style.display = "none";
      x = x-1;
    } else {

    }

}

function deleteMsg2() {
  
  var msg2 = document.getElementsByClassName("cont2");
    if (confirm("Are you sure to want to delete this message?")) {
      msg2[0].style.display = "none";
      x = x-1;
    } else {

    }

}

document.getElementById("msgcount").innerHTML = x;

.cont, .cont2 {
background-color: red;
padding: 5px;
width: 100px;
margin: 25px 0;
}

.show {
display: block;
}

<h1>There are <span id="msgcount"></span>messages</h1>
<button onclick="deleteMsg1()">Delete</button>
<div class="cont">
  Some text...
</div>
<br><br>
<button onclick="deleteMsg2()">Delete</button>
<div class="cont2">
  Some text...
</div>

I know this isn’t the best idea, but I guess it’s bad. I think I should do this with one function() and try something event listener but I don't really know how to do that. Any idea or help?

Answer 1

Explained

Here's a simple enough solution, you need to update the HTML manually every time you want to update the value of x. That's why I created an updateX function, it'll just take the value & update the DOM, it's quite that simple.

const updateX = (x) => {
  document.getElementById("msgcount").innerHTML = x;
};

let x = 2;

const del = (className) => {
  const msg = document.getElementsByClassName(className);

  if (confirm("Are you sure to want to delete this message?")) {
    msg[0].style.display = "none";
    x--;
  } else {
    console.log("===");
  }

  updateX(x);
};

updateX(x);

.cont,
.cont2 {
  background-color: red;
  padding: 5px;
  width: 100px;
  margin: 25px 0;
}

.show {
  display: block;
}

<h1>There are <span id="msgcount"></span>messages</h1>
<button onclick="del('cont')">Delete</button>
<div class="cont">
  Some text...
</div>
<br/><br/>
<button onclick="del('cont2')">Delete</button>
<div class="cont2">
  Some text...
</div>

Answer 2

You should wrap each message's HTML in a parent element so that you can then treat each set of elements that comprise a message as a single unit and delete it all at once.

To be able to do this with a single function, you can use this to reference the element that triggered the callback function in the first place and .closest() to access the single parent wrapper.

Notes:

• Do not use inline HTML event attributes, like onclick. Separate your HTML and your JavaScript and use .addEventListener() to bind elements to event callbacks. Even MDN recommends not using them.
• Do not use .getElementsByClassName() as it is a 25+ year old API that has significant performance implications. Instead, use the modern .querySelectorAll() method.
• Do not use .innerHTML if you can avoid it as it has security and performance implications. Since the text you are wanting to update doesn't have any HTML in it anyway, .innerHTML is not warranted. Instead, use .textContent.

// Do your event binding in JavaScript, not HTML
document.querySelectorAll("button").forEach(function(element){
  element.addEventListener("click", function(){
    if (confirm("Are you sure to want to delete this message?")) {
      // All you need to do is delete the nearest complete
      // ancestor message construct, which can be done with
      // the .closest() method
      this.closest(".message").remove();
      updateMessageCount();
    }
  });
});

function updateMessageCount(){
  // Set the count equal to the length of the 
  // collection returned by searching for all the
  // messages
  document.getElementById("msgcount").textContent =
       document.querySelectorAll(".message").length;
}

updateMessageCount();

.cont, .cont2 {
background-color: red;
padding: 5px;
width: 100px;
margin: 25px 0;
}

.show {
display: block;
}

<h1>There are <span id="msgcount"></span> messages</h1>

<!-- By wrapping each message, you can treat all its HTML
     as one single unit. -->
<div class="message">
  <button>Delete</button>
  <div class="cont">
    Some text...
  </div>
</div>
<br><br>
<div class="message">
  <button>Delete</button>
  <div class="cont">
    Some text...
  </div>
</div>

Answer 3

My advice to you: Never declare events js inside html structure tags! As here:

<button onclick="deleteMsg1()">Delete</button>

This is a very bad practice. This has many disadvantages. And this can lead to bad consequences.

I made a solution for you with the forEach() method, without using javascript in html.

The Delete button is also removed.

let msg = document.querySelectorAll(".cont");
let btn_del = document.querySelectorAll('.btn_del');
let x = 2;

btn_del.forEach(function (btn_del_current, index) {
    btn_del_current.addEventListener('click', function () {
        if (confirm("Are you sure to want to delete this message?")) {
            this.style.display = "none";
            msg[index].style.display = "none";
            x = x - 1;
            document.getElementById("msgcount").innerHTML = x;
        } else {}
    });
});

.cont, .cont2 {
  background-color: red;
  padding: 5px;
  width: 100px;
  margin: 25px 0;
}

.show {
  display: block;
}

<h1>There are <span id="msgcount"></span>messages</h1>
<button class="btn_del">Delete</button>
<div class="cont">
  Some text...
</div>
<br><br>
<button class="btn_del">Delete</button>
<div class="cont">
  Some text...
</div>