1

I have a function that counts every time a user clicks on and image on the page: 

for(var i = 0; i < document.getElementsByClassName("face").length; i++){
    document.getElementsByClassName("face")[i].addEventListener("click", counter)
}


function counter(){
    count ++;
}

And I want to disable it after the first click so it won't keep counting duplicate clicks of the same picture. I know adding this.onclick=null on the HTML tag will work, but I'm wondering if there a way to do this on the javascript file itself so I don't need to put javascript onto my HTML file.

jmona789
  • 2,350
  • 4
  • 19
  • 44
  • So [remove](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener) the click event inside of the click event. – epascarello Nov 14 '15 at 22:13

3 Answers3

3

You need to remove event listener from each clicked element:

function counter(){
    count++;
    this.removeEventListener('click', counter);
}
Max Koshel
  • 134
  • 7
1

If you don't want to remove the event (in case you need it reactivated again later or something), you can make count an array of booleans, and each button will flip the value of its own index. No duplicates that way and you would still be able to easily get the sum of all true flags.

var count = [];
function counter(i){
    count[i] = true;
}

Quick live example: 

var count = [];
var faces = document.getElementsByClassName("face");
var result = document.getElementById('t');

for(var i = 0; i < faces.length; i++){
    handle(i);
}

function counter(i){
  var total = 0;
  count[i] = true;  
  count.concat([]).reduce(function(p, c){c ? total++ : ''}, 0);
  result.textContent = total;
}

function handle(i){
  faces[i].addEventListener("click", function(){
    counter(i);
    this.className = 'face clicked';
  })  
}
button {width: 50px; height: 50px}
button.clicked {opacity: 0.2}
<button class="face"></button>
<button class="face"></button>
<button class="face"></button>
<button class="face"></button>
<p>Count: <span id="t">0</span></p>
Shomz
  • 36,161
  • 3
  • 52
  • 81
  • Forgive me, I am a newbie at this, but how do I get the sum of all the true flags? – jmona789 Nov 14 '15 at 22:16
  • we don't need to store worked event listeners any more, better to remove them – Max Koshel Nov 14 '15 at 22:19
  • No problem, I just wrote you a quick example with 4 clickable buttons and a counter. I used `reduce` to sum up the true flags, but there are many other ways to do it as well. – Shomz Nov 14 '15 at 22:21
0

There is a neater way to handle the functionality you seek. I think what you want is a list of the images that have been clicked like @Shomz thinks as well. Is that correct?

1) The length property of the for loop is evaluated each iteration in the loop. Fetching the element from the DOM again with the index i on each iteration is slow.

for(var i = 0; i < document.getElementsByClassName("face").length; i++){
    document.getElementsByClassName("face")[i].addEventListener("click", counter)
}

An improvement is to change it into this:

var faces = document.getElementsByClassName("face");
for (var i = 0, var len = faces.length; i < len; i++) {
    faces[i].addEventListener("click", counter)
}

2) But the code still attach an event listener to every single image. This is slow if you have many images. A better way is to let click events bubble upwards and catch them at a parent level. This is called event delegation. This way you don't need a loop, and merely a single event handler needs to be attached.

I store the id of the image, but you could store whatever you need from the node of course that is unique (the src?).

var clickedFaces = [];

document.getElementById('ImageContainer').addEventListener("click", onFaceClick);

function onFaceClick(e) {
    if (e.target.className === 'face' && clickedFaces.indexOf(e.target.id) !== -1) {
        clickedFaces.push(e.target.id);
    }
}

If you want to know how many faces have been clicked:

console.log(clickedFaces.length);
Community
  • 1
  • 1
oldwizard
  • 4,677
  • 2
  • 28
  • 29
  • This is a bit risky in case the faces have no unique features, for example, pairs of two (and you'd be able only to click one). And btw, you probably missed my latest update, I did optimize the code a little bit (originally, tried to keep it as close to OPs code as possible). – Shomz Nov 14 '15 at 22:33
  • True, if the images have nothing unique, event delegation won't work with the code the way I wrote it. It is easily solved by adding a GUID as ID to each face. I don't know how many faces you have per container, but many event listeners that all do the same thing should be avoided in my opinion. – oldwizard Nov 14 '15 at 22:38
  • That's true as well. Anyway, I think we've built way more than needed from the original 6 lines of code. :) – Shomz Nov 14 '15 at 22:39
  • Code in my example is the same number of lines, it is merely faster and use less memory. :) – oldwizard Nov 14 '15 at 22:41
  • WOW, I was referring to OPs original 6 lines and how we know nothing more than that... but if you want to get competitive, go to another website, Stack Overflow is just not the place. – Shomz Nov 14 '15 at 22:44
  • Got the nicknames confused mate! Not about competition, about knowing about performance implications. SO is not only about "fixing" the OPs problem, if OP also learns more about the tools that's a nice bonus. At least I like answers like that to my own questions. – oldwizard Nov 14 '15 at 22:46
  • 1
    Here at SO, we're primarily focused on solving 1 problem per question (which is why it's important for each question to have one specific problem). There's a SO sister site that deals with slicing the code to bits and doing reviews and optimizations: http://codereview.stackexchange.com/ Answers here should stay as close to the original code as possible, *especially* for beginners (to avoid that **WHOA!** moment when seeing a completely new code they might not understand right away). Bonus stuff is more for comments, at least that's how I see it. Cheers! – Shomz Nov 14 '15 at 22:52