CODEWITHSUNDEEP
javascripthtmldom
g5wx
g5wx

Reputation: 720

addEventListener() works only for last instance

I'm creating a simple color picker, where the script creates two instances of the picker - one is for changing the text color and second for background color (depending on the data attribute that element has assigned to it).

When looping the elements event listener attaches only to last instance of the color picker that was created. In the example below the background will change but the text color won't as the previously defined event handlers get lost. I've found this answer but can't get mine to work.

var colorPallete = function(id, attr, property, label) {
        var colorsParent = document.getElementById('colorPicker');
        var colorPallete = '<div id="' + id + '" class="pallete"><span>' + label + '</span><div class="colors"></div></div>';
        colorsParent.innerHTML += colorPallete;
        var colors = [
            'blue',
            'red',
            'green'
        ];
        for (var i = 0; i < colors.length; i++) {
            document.getElementById(id).getElementsByClassName('colors')[0].innerHTML += '<div class="color" data-hex="' + colors[i] + '" style="background-color:' + colors[i] + '"></div>';
        }
        var allColors = document.getElementById(id).getElementsByClassName('color');
        for (var i = 0; i < allColors.length; i++) {
            allColors[i].addEventListener('click', function() {
                var colorEl  = document.querySelectorAll('[data-color="' + attr + '"]');
                var newColor = this.getAttribute('data-hex');
                for (var i = 0; i < colorEl.length; i++) {
                    switch(property) {
                        case 'color'      : colorEl[i].style.color = newColor; break;
                        case 'background' : colorEl[i].style.backgroundColor = newColor; break;
                    }
                }
            }, false);
        }
    }
    new colorPallete('txtColor', 'text', 'color', 'Change text color');
    new colorPallete('bgColor', 'background', 'background', 'Change background color');
#colorPicker .current,
#colorPicker .colors .color  {
    display: inline-block;
    content: "";
    width: 20px;
    height: 20px;
    margin-right: 6px;
    cursor: pointer;
}
<div id="colorPicker"></div>
<br /><br /><br />
<div class="bg" data-color="background">Background color</div>
<div class="bg" data-color="text">Text color</div>

Upvotes: 1

Views: 916

Answers (1)

Barmar
Barmar

Reputation: 782785

When you assign to .innerHTML of colorsParent, you're removing all the elements that were in there, and creating new elements by parsing the HTML you assign. So all the event listeners you assigned to those elements are lost.

You can use .insertAdjacentHTML() to add new HTML to an element without removing the old elements.

var colorPallete = function(id, attr, property, label) {
  var colorsParent = document.getElementById('colorPicker');
  var colorPallete = '<div id="' + id + '" class="pallete"><span>' + label + '</span><div class="colors"></div></div>';
  colorsParent.insertAdjacentHTML('beforeend', colorPallete);
  var colors = [
    'blue',
    'red',
    'green'
  ];
  for (var i = 0; i < colors.length; i++) {
    document.getElementById(id).getElementsByClassName('colors')[0].innerHTML += '<div class="color" data-hex="' + colors[i] + '" style="background-color:' + colors[i] + '"></div>';
  }
  var allColors = document.getElementById(id).getElementsByClassName('color');
  for (var i = 0; i < allColors.length; i++) {
    allColors[i].addEventListener('click', function() {
      var colorEl = document.querySelectorAll('[data-color="' + attr + '"]');
      var newColor = this.getAttribute('data-hex');
      for (var i = 0; i < colorEl.length; i++) {
        switch (property) {
          case 'color':
            colorEl[i].style.color = newColor;
            break;
          case 'background':
            colorEl[i].style.backgroundColor = newColor;
            break;
        }
      }
    }, false);
  }
}
new colorPallete('txtColor', 'text', 'color', 'Change text color');
new colorPallete('bgColor', 'background', 'background', 'Change background color');
#colorPicker .current,
#colorPicker .colors .color {
  display: inline-block;
  content: "";
  width: 20px;
  height: 20px;
  margin-right: 6px;
  cursor: pointer;
}
<div id="colorPicker"></div>
<br /><br /><br />
<div class="bg" data-color="background">Background color</div>
<div class="bg" data-color="text">Text color</div>

Upvotes: 8

Related Questions