CODEWITHSUNDEEP
javascripthtml
Akash Kumar Seth
Akash Kumar Seth

Reputation: 1701

Javascript why click eventListener is firing for previous image?

Basically, I am trying to build a cat clicker app where we click the cat picture to increase the no of click count. We can use the drop-down to change the cat picture being displayed. The problem is that as I change the image and click on it the count of the previous image also increase. JSBin link with code

var cats = {
  cat1: ['Tiger ', "https://lh3.ggpht.com/nlI91wYNCrjjNy5f-S3CmVehIBM4cprx-JFWOztLk7vFlhYuFR6YnxcT446AvxYg4Ab7M1Fy0twaOCWYcUk=s0#w=640&h=426", 0],
  cat2: ['Puss',
    "https://lh3.ggpht.com/kixazxoJ2ufl3ACj2I85Xsy-Rfog97BM75ZiLaX02KgeYramAEqlEHqPC3rKqdQj4C1VFnXXryadFs1J9A=s0#w=640&h=496", 0
  ],
  cat3: ['Smokey',
    "https://lh5.ggpht.com/LfjkdmOKkGLvCt-VuRlWGjAjXqTBrPjRsokTNKBtCh8IFPRetGaXIpTQGE2e7ZCUaG2azKNkz38KkbM_emA=s0#w=640&h=454", 0
  ],
  cat4: ['Misty',
    "https://images.unsplash.com/photo-1482066490729-6f26115b60dc?ixlib=rb-0.3.5&ixid=eyJhcHBfaWQiOjEyMDd9&s=2e749e5a9492af91a0c4e366e0f58bb4&auto=format&fit=crop&w=700&q=60", 0
  ],
  cat5: ['Ashes',
    "https://images.unsplash.com/photo-1457410129867-5999af49daf7?ixlib=rb-0.3.5&ixid=eyJhcHBfaWQiOjEyMDd9&s=db366be68b7ffcb315597fcf0c1b6a51&auto=format&fit=crop&w=700&q=60", 0
  ],
  cat6: ['Kitty',
    "https://images.unsplash.com/photo-1417028441456-f283323fe2d6?ixlib=rb-0.3.5&ixid=eyJhcHBfaWQiOjEyMDd9&s=5b2d5f4ce317590a93f0ff7ccc1abd39&auto=format&fit=crop&w=700&q=60", 0
  ]

}

//the following code make sure we have one cat info available as we load the page.
render(cats);

//load cats from drop down.
function showCat() {
  var val = document.getElementById('mySelect').value;
  document.getElementById('catPic').src = '';
  render(cats, val);
}

function render(cats, val = 'cat1') {
  document.getElementById('catName').innerHTML = cats[val][0];

  document.getElementById('catPic').src = cats[val][1];

  var clickCount = cats[val][2];
  console.log("click1 : " + clickCount);
  document.getElementById('catCount').innerHTML = 'Click Count : ' + clickCount;

  var elemCat = document.getElementById('catPic');
  elemCat.addEventListener('click', function() {
    clickCount++;
    cats[val][2] = clickCount;
    console.log("click2 : " + clickCount);
    document.getElementById('catCount').innerHTML = "Click Count : " + clickCount;
  }, false);
  elemCat.removeEventListener('click', function() {
    console.log('Removing Event Listener');
  }, false);
}
<html>

<head>
  <title>Cat Clicker</title>
</head>

<body>
  <select id="mySelect" onchange="showCat()" name="Select the Cat">
    <option value="cat1">Tiger</option>
    <option value="cat2">Puss</option>
    <option value="cat3">Smokey</option>
    <option value="cat4">Misty</option>
    <option value="cat5">Ashes</option>
    <option value="cat6">Kitty</option>
  </select>
  <h1 id="catName"></h1>
  <img id="catPic" src="#" alt="cat-photo">
  <h3 id="catCount"></h3>
</body>

</html>

Upvotes: 2

Views: 81

Answers (2)

intentionally-left-nil
intentionally-left-nil

Reputation: 8274

The issue

What is going on is that you are reusing the same elemCat every single time (you just set the innerHTML), but on your render(), you always set a new event listener. At the very bottom of your render function you have 

elemCat.removeEventListener('click', function () {
        console.log('Removing Event Listener');
    }, false);

which I think you mean to remove old listeners, however that is not how it works. What removeEventListener does is remove the click handler If and only if the function passed in is the same object as the function passed into addEventListener. This is not possible since you are using an anonymous function in your addEventListener call. You never save a reference to the function, so you can't remove it later.

The fix

The best way to fix this is to only call addEventListener once. That way you don't have to worry about managing the click handlers. If you only have one event handler, then how will it know which count to increment? That is where data-props come in. You can save the cat name as data-cat-name and then read it each time it's clicked:

The onClick code

Note here the only difference is pulling the catName off of the element and then incrementing the count based on it

const elemCat = document.getElementById('catPic');
elemCat.addEventListener('click', function() {
  const catName = elemCat.dataset.catName;
  cats[catName][1] += 1; // increment the count
  document.getElementById('catCount').innerHTML = 'Click Count : ' + cats[catName][1];
});

The onChange code

Your onChange code would look about the same.

const val = document.getElementById('mySelect').value;
const catPic = document.getElementById('catPic');
catPic.src = cats[val][0];
catPic.dataset.catName = val;

Upvotes: 1

CertainPerformance
CertainPerformance

Reputation: 370699

You need a static reference to the function you call addEventListener with in order to remove it later. A listener added with elemCat.addEventListener('click', function () { will never be removeable, because it's anonymous and not stored in a variable. You should assign the listener function to an outside variable so that it can be removed later:

var cats = {
  cat1: ['Tiger ', "https://lh3.ggpht.com/nlI91wYNCrjjNy5f-S3CmVehIBM4cprx-JFWOztLk7vFlhYuFR6YnxcT446AvxYg4Ab7M1Fy0twaOCWYcUk=s0#w=640&h=426", 0],
  cat2: ['Puss',
    "https://lh3.ggpht.com/kixazxoJ2ufl3ACj2I85Xsy-Rfog97BM75ZiLaX02KgeYramAEqlEHqPC3rKqdQj4C1VFnXXryadFs1J9A=s0#w=640&h=496", 0
  ],
  cat3: ['Smokey',
    "https://lh5.ggpht.com/LfjkdmOKkGLvCt-VuRlWGjAjXqTBrPjRsokTNKBtCh8IFPRetGaXIpTQGE2e7ZCUaG2azKNkz38KkbM_emA=s0#w=640&h=454", 0
  ],
  cat4: ['Misty',
    "https://images.unsplash.com/photo-1482066490729-6f26115b60dc?ixlib=rb-0.3.5&ixid=eyJhcHBfaWQiOjEyMDd9&s=2e749e5a9492af91a0c4e366e0f58bb4&auto=format&fit=crop&w=700&q=60", 0
  ],
  cat5: ['Ashes',
    "https://images.unsplash.com/photo-1457410129867-5999af49daf7?ixlib=rb-0.3.5&ixid=eyJhcHBfaWQiOjEyMDd9&s=db366be68b7ffcb315597fcf0c1b6a51&auto=format&fit=crop&w=700&q=60", 0
  ],
  cat6: ['Kitty',
    "https://images.unsplash.com/photo-1417028441456-f283323fe2d6?ixlib=rb-0.3.5&ixid=eyJhcHBfaWQiOjEyMDd9&s=5b2d5f4ce317590a93f0ff7ccc1abd39&auto=format&fit=crop&w=700&q=60", 0
  ]

}

//the following code make sure we have one cat info available as we load the page.
render(cats);

//load cats from drop down.
function showCat() {
  var val = document.getElementById('mySelect').value;
  document.getElementById('catPic').src = '';
  render(cats, val);
}

var currentListener;
function render(cats, val = 'cat1') {
  document.getElementById('catName').innerHTML = cats[val][0];
  document.getElementById('catPic').src = cats[val][1];
  var clickCount = cats[val][2];
  console.log("click1 : " + clickCount);
  document.getElementById('catCount').innerHTML = 'Click Count : ' + clickCount;
  var elemCat = document.getElementById('catPic');
  if (currentListener) elemCat.removeEventListener('click', currentListener);
  currentListener = function() {
    clickCount++;
    cats[val][2] = clickCount;
    console.log("click2 : " + clickCount);
    document.getElementById('catCount').innerHTML = "Click Count : " + clickCount;
  }
  elemCat.addEventListener('click', currentListener);
}
<html>

<head>
  <title>Cat Clicker</title>
</head>

<body>
  <select id="mySelect" onchange="showCat()" name="Select the Cat">
    <option value="cat1">Tiger</option>
    <option value="cat2">Puss</option>
    <option value="cat3">Smokey</option>
    <option value="cat4">Misty</option>
    <option value="cat5">Ashes</option>
    <option value="cat6">Kitty</option>
  </select>
  <h1 id="catName"></h1>
  <img id="catPic" src="#" alt="cat-photo">
  <h3 id="catCount"></h3>
</body>

</html>

Upvotes: 2

Related Questions