Best way to handle clicks on buttons in JavaScript (vanilla) [closed]

Question

Edit: My question might have been overly broad (still learning), but I nevertheless received very useful answers. So thank you all very much for your input. I've also corrected my code to incorporate passing on 'event'.

---

This is my first question asked here, so I'm a bit nervous...

Here we go: What are the best ways (regarding best practise, speed, compatibility and so on) to manage clicks on buttons and let them trigger different functions?

When researching this question, I found an example on Eloquent JavaScript and took this as a basis.

Then I extended it, by using a “mapper” (is this the correct term?) to find the right function, depending on the id of the button who is triggering the event.

Here’s the code:

// functions I want to trigger
function one(e) {
  alert("You clicked " + e.textContent);
}

function two(e) {
  alert("You clicked " + e.textContent);
}

// mapper object to hold my functions
var buttonMap = {
  b_one: function(event) {
    one(event.target)
  },
  b_two: function(event) {
    two(event.target)
  }
}

// Event Listener
var buttonClick = document.body.addEventListener('click', function(event) {
  if (event.target.nodeName == "BUTTON") {
    var tar = event.target.id;
    buttonMap[tar](event)
  }
});

<button id="b_one" type="button">Button One</button>
<button id="b_two" type="button">Button Two</button>

Is this solution ok? How can it be improved? Are there any problems I could run into?

Little bonus question: Additionally, I thought about wrapping the whole thing inside document.addEventListener("DOMContentLoaded", function() {}).

Is this a good idea or even needed? In what cases should I check if the DOM content is loaded and when can I omit this check?

Answer 1

Just for completeness, you may also consider to use the onClick HTML attribute.

function myFunction() {
  alert('hey!');
}

<button onClick="myFunction()">Do something</button>

It's probably old fashion but it's damn easy to understand and maintain, and it's just what happens nowadays with libraries like React*.

*: well, not really, but the API is similar.

Answer 2

You have created a simple event delegation that will give you the ability to add/remove buttons without the need to add/remove event handlers.

If you've got a lot of buttons that you create or destroy dynamically, it's better to use event delegation, than pay the cost of manually managing the handlers.

You can put the event handling code at the end of your body, instead of using the DOMContentLoaded event. As long as the container (body in this case) exists, it doesn't matter if the children exist or not.

Attach the global event handler to the closest container, and not to the body if you can. In addition, use data-* attributes instead of the id or the value attribute to add data to the buttons. An id creates a global variable, and value will be used in forms.

<div class="buttonsContainer">
  <button type="button" data-value="one">Button One</button>
  <button type="button" data-value="two">Button Two</button>
</div>

You should pass the event object to the event handler:

var buttonClick = container && container.addEventListener('click', 
    function(event) {
      var target = event.target;
      var handler;
      if (target.nodeName == "BUTTON" && (handler = target.getAttribute('data-handler'))) {
        buttonMap[handler](event)
      }
    });

Demo:

function one(e) {
  alert("You clicked " + e.textContent);
}

function two(e) {
  alert("You clicked " + e.textContent);
}

// mapper object to hold my functions
var buttonMap = {
  b_one: function(event) {
    one(event.target)
  },
  b_two: function(event) {
    two(event.target)
  }
}

// Event Listener
var container = document.querySelector('.buttonsContainer');
var buttonClick = container && container.addEventListener('click', function(event) {
  var target = event.target;
  var handler;
  if (target.nodeName == "BUTTON" && (handler = target.getAttribute('data-handler'))) {
    buttonMap[handler](event)
  }
});

<div class="buttonsContainer">
  <button type="button" data-handler="b_one">Button One</button>
  <button type="button" data-handler="b_two">Button Two</button>
</div>

Answer 3

Your approach is fine, but it's not working the way it's written right now. Your mapped functions call your handlers with event.target, but you never pass event when you call the mapped function. The mapped functions need to look like this in order to work:

...
b_one: function(event) {
  one(event.target)
}
...

... and your invokation of it in the event handler needs to look like this:

buttonMap[tar](event)

What you're doing, attaching the event listener to the body and invoking a handler based on the event target, is called event delegation. You should add a check to make sure the clicked button really has an associated mapped function in order to make sure it doesn't crash when clicking some other button:

if (typeof buttonMap[tar] === 'function') {
  buttonMap[tar](event);
}

---

The purpose of running code on the DOMContentLoaded is to make sure the code runs after the DOM has been parsed and is available. This can also be accomplished by placing the script block after the DOM elements it's relying on. For instance, at the end of the body element.