I have three different boxes which all have a button inside.

I want to register an onclick event for every button with a specific data attribute.

I've started a jsfiddle, any suggestions?

/

var buttons = document.querySelectorAll("button[data-donation]");

for (i = 0; i < buttons.length; i++) {
    document.querySelectorAll("button[data-donation]")[i].onclick = console.log(buttons[i]);
}

function letsGo(init) {

    var input = document.body.querySelector("input[name='amount']"),
        amount = parseInt(input.value, 10) * 100,
        url = ";freeamount=1&amount=";

    if (input.value === '' || parseInt(input.value, 10) >= 0) {
        window.location.href = url + init * 100;
    } else {
        window.location.href = url + amount;
    }
}

<div class="shopping-item">
    <p><span>£33</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£33" class="shopping-1-input">
    <button data-donation="33" class="donate-button shopping-1-button">donate now</button>
</div>
    
<div class="shopping-item">
    <p><span>£50</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£50" class="shopping-2-input">
    <button data-donation="50" class="donate-button shopping-2-button">donate now</button>
</div>
    
<div class="shopping-item">
    <p><span>£188</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£188" class="shopping-3-input">
    <button data-donation="188" class="donate-button shopping-3-button">donate now</button>
</div>

I have three different boxes which all have a button inside.

I want to register an onclick event for every button with a specific data attribute.

I've started a jsfiddle, any suggestions?

http://jsfiddle/mwils/w6gofb30/

var buttons = document.querySelectorAll("button[data-donation]");

for (i = 0; i < buttons.length; i++) {
    document.querySelectorAll("button[data-donation]")[i].onclick = console.log(buttons[i]);
}

function letsGo(init) {

    var input = document.body.querySelector("input[name='amount']"),
        amount = parseInt(input.value, 10) * 100,
        url = "https://donate.shelter.uk/b?cid=152&freeamount=1&amount=";

    if (input.value === '' || parseInt(input.value, 10) >= 0) {
        window.location.href = url + init * 100;
    } else {
        window.location.href = url + amount;
    }
}

<div class="shopping-item">
    <p><span>£33</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£33" class="shopping-1-input">
    <button data-donation="33" class="donate-button shopping-1-button">donate now</button>
</div>
    
<div class="shopping-item">
    <p><span>£50</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£50" class="shopping-2-input">
    <button data-donation="50" class="donate-button shopping-2-button">donate now</button>
</div>
    
<div class="shopping-item">
    <p><span>£188</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£188" class="shopping-3-input">
    <button data-donation="188" class="donate-button shopping-3-button">donate now</button>
</div>

• javascript

Share Improve this question Follow edited May 19, 2015 at 12:18 epascarello 208k2020 gold badges205205 silver badges244244 bronze badges asked May 19, 2015 at 12:10 Michael WilsonMichael Wilson 1,57933 gold badges2222 silver badges4444 bronze badges 2

• jsfiddle/arunpjohny/wwtrhkb2/1 – Arun P Johny Commented May 19, 2015 at 12:15
• 1 You should put your code into the question. I did it for you. – epascarello Commented May 19, 2015 at 12:19

Add a ment  | 

5 Answers 5

Sorted by: Reset to default 3

While there are really many ways to do that i, most of the time, prefer a simple click delegation. I set that on elements that contain the buttons(could be the window or document.body itself).

To make it simple i show you that with a container first.

<div id="container">
 <button>1</button>
 <button>2</button>
 <button>3</button>
 <button>4</button>
 <button>5</button>
</div>

here is the js

document.getElementById('container').onclick=function(e){
 if(e.target.nodeName=='BUTTON'){
  alert(e.target.textContent);
 }
}

What does this???

if i click on the container it checks if the target element is a button. if yes it alerts the textContent of the button. simple right? Doing so you avoid alot of extra variables.

---

In your case

document.onclick=function(e){
 if(e.target.dataset['donation']){
  alert(e.target.dataset['donation']);
 }
}

shorter

document.onclick=function(e){
 e=e.target.dataset; // reuse the variable e
 !e['donation']||alert(e['donation']);
}

using

<button data-donation="33">donate now</button>

DEMO's

http://jsfiddle/j8xgqfmk/

Extra

button text followed by $ symbol done with css

http://jsfiddle/j8xgqfmk/1/

preceded by a £

http://jsfiddle/j8xgqfmk/2/

but returning the desidered value

Why?

1. No loops!!!
2. Only one event handler!!!!
3. Less variables!!!
4. Short code !!!
5. Faster then multiple plex eventhandlers.

other considerations:

var buttons = document.querySelectorAll("button[data-donation]");

for (i = 0; i < buttons.length; i++) {
 document.querySelectorAll("button[data-donation]")[i].onclick=console.log(buttons[i]);
}

Should be written

var buttons=document.querySelectorAll('button[data-donation]'),
    btnlength=buttons.length; //cache variables

for (var i = 0; i < btnlength; i++) { // var i
 buttons[i].onclick=function(){ //buttons is already defined
  console.log(this);
 }
 // and you need to pass a function to the onclick event
}

or even better

 function handler(e){
  console.log(this,e);
 }

 var btns=document.querySelectorAll('button[data-donation]'),
     l=btns.length;

 while(l--)btns[l].addEventListener('click',handler,false);

or the "Haters gonna hate" version

http://jsfiddle/Lbk1ueme/

var B=document.querySelectorAll('button[data-donation]'),
    L=B.length,
    I=0,
    H=function(){console.log(this)};

for(;I<L;B[I++].onclick=H);

https://stackoverflow./a/21353032/2450730

if you have difficulties to understand that i can help you to write it based on your requests. For any other question just ask.

keep stuff simple

Loop through your element array (as you already are doing) and use addEventListener to bind the click event to each...

var buttons = document.querySelectorAll("button[data-donation]");

for (i = 0; i < buttons.length; i++) {
    buttons[i].addEventListener('click', function(event) {
        alert(this.getAttribute('data-donation')); // alert the value of data-donation attribute
    });
}

<div class="shopping-item">
    <p><span>£33</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£33" class="shopping-1-input">
    <button data-donation="33" class="donate-button shopping-1-button">donate now</button>
</div>
    
<div class="shopping-item">
    <p><span>£50</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£50" class="shopping-2-input">
    <button data-donation="50" class="donate-button shopping-2-button">donate now</button>
</div>
    
<div class="shopping-item">
    <p><span>£188</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£188" class="shopping-3-input">
    <button data-donation="188" class="donate-button shopping-3-button">donate now</button>

The problem is you are not attaching events correctly to the element. You are assigning console.log and not a function to it. Seconds querySelectorAll is expensive. You should not keep looking things up in a loop. Create a variable and store the live html node collection it returns into it.

var btns = document.querySelectorAll("button[data-donation]");
for (var i=0; i<btns.length; i++) {
    btns[i].onclick = function() {
        console.log(this);
        console.log(this.getAttribute("data-donation"));
    }
}

function letsGo(init) {

    var input = document.body.querySelector("input[name='amount']"),
        amount = parseInt(input.value, 10) * 100,
        url = "https://donate.shelter.uk/b?cid=152&freeamount=1&amount=";

    if (input.value === '' || parseInt(input.value, 10) >= 0) {
        window.location.href = url + init * 100;
    } else {
        window.location.href = url + amount;
    }
}

<div class="shopping-item">
    <p><span>£33</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£33" class="shopping-1-input">
    <button data-donation="33" class="donate-button shopping-1-button">donate now</button>
</div>
    
<div class="shopping-item">
    <p><span>£50</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£50" class="shopping-2-input">
    <button data-donation="50" class="donate-button shopping-2-button">donate now</button>
</div>
    
<div class="shopping-item">
    <p><span>£188</span> could help a family find and keep a home</p>
    <input type="number" name="amount" placeholder="£188" class="shopping-3-input">
    <button data-donation="188" class="donate-button shopping-3-button">donate now</button>
</div>

You've got the selector right already, i.e. var buttons = document.querySelectorAll("button[data-donation]") is pulling in all the buttons having data-donation, but there's no need to do another querySelectorAll inside your loop, you just need to reference each button in your result with buttons[i].

Your other mistake is setting onclick = console.log(), instead of onclick = function() { console.log() }. So all you need to change to get it working is:

var buttons = document.querySelectorAll("button[data-donation]");

for (i = 0; i < buttons.length; i++) {
    var button = buttons[i];
    buttons[i].onclick = function() { console.log(button) };
}

http://jsfiddle/w6gofb30/4/

document.querySelectorAll("button[data-donation]").every(function(e,i,a){
    e.addEventListener("click", function(){
        console.log(this);
        console.log(this.getAttribute("data-donation"));
    }, false);
});

but before this u need:

NodeList.prototype.every = function(f){
    for(var k=0;k<this.length;k++)
        f(this[k],k,this);
}

http://jsfiddle/sa0ezut7/