Javascript average colour of img displayed on mulitple backgrounds

Question

As you can see below I am trying to write a program that will take the average colour of an image and then display it as the background of the parent class where the img is in.

Each image is stored in a separate class: 'sqr1', 'sqr2' e.t.c

The problem is that the average colour of the last image element is being displayed on as the background of all parent class tags ('sqr1', 'sqr2' e.t.c).

Is there a way to separate these two out?

Is there a better of doing this?

<html>

<head>
  <title>getting average color</title>
  <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.1.1/jquery.min.js"></script>
  <script src="js/color-thief.js"></script>
</head>

<body>
  <p id="bob">...</p>

  <div class="sqr1">
    <img src="memes/apple.jpg" width="400" height="400">
  </div>

  <p></p>

  <div class="sqr2">
    <img src="memes/pie.jpg" width="400" height="400">
  </div>

  <script type="text/javascript">
    var x;
    var allimgs = document.getElementsByTagName("img");
    var imglen = 2;

    let change = function(color,img){
      for (var i = 0; i < imglen; i++) {
          if (allimgs[i].src == img){
            allimgs[i].parentElement.style.backgroundColor = 'rgb('+color[0]+','+color[1]+','+color[2]+')';
          }
      }
    let colormix = function(){
      for(x = 0; x<imglen; x++){
        var colorThief = new ColorThief();
        colorThief.getColorFromUrl(allimgs[x].src, change,1);
        console.log();
      }
    }
    colormix();
  </script>
</body>

</html>

score 1 · Answer 1 · edited May 23 '17 at 12:00

Your problem is a classic one in JavaScript programming: You are modifying an object reference in a loop, and inadvertently setting all of your objects to reference the same value. There are a number of solutions to this problem; my preferred solution is to wrap the interior of your for loops in an immediately invoked functional expression (iife) in order to form a closure scope for your variables:

let change = function(color,img){
  for (var i = 0; i < imglen; i++) {
      (function (index) {
        if (allimgs[index].src == img){
          allimgs[index].parentElement.style.backgroundColor = 'rgb('+color[0]+','+color[1]+','+color[2]+')';
        }
      })(i)
  }
}
let colormix = function(){
  for(x = 0; x<imglen; x++){
    (function (index) {
      var colorThief = new ColorThief();
      colorThief.getColorFromUrl(allimgs[index].src, change,1);
      console.log();
    })(x)
  }
}

I find this solution to this problem to be the most convenient, but some people prefer to simply pull the interior of their loops into functions and invoke those in the loop. The result will be the same.

For a more full explanation of what is happening here, see this question: JavaScript closure inside loops – simple practical example

To avoid problems like this in the future, read up on Javascript scoping and closures: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures

I copied your code in this answer and replaced the "index" with i's and x's to no effect. The two colours aren't separate and still display the one of the last. — AtomiCookiez, Feb 23 '17 at 20:16
why would you replace the inner index variable with the outer i and x? the index variable in the closure is what solves the problem. — MaxPRafferty, Feb 23 '17 at 20:46
I thought is corresponded to the variables in for loop. On the other hand this still doesn't solve the problem. — AtomiCookiez, Feb 23 '17 at 21:06

score 1 · Accepted Answer · answered Feb 27 '17 at 17:26

Ah, simple logic error then. You are looping through and setting the colors of all of the images each time. You'll need to remove the loop in change pass an index, or otherwise they will always end up as your final color. One way to do this is by binding the index parameter to the function:

var x;
var allimgs = document.getElementsByTagName("img");
var imglen = allimgs.length;
let change = function(index, color, img){
   if (allimgs[index].src == img){
      allimgs[index].parentElement.style.backgroundColor = 'rgb('+color[0]+','+color[1]+','+color[2]+')';
   }
}
let colormix = function(){
  for(x = 0; x<imglen; x++){
    var colorThief = new ColorThief();
    colorThief.getColorFromUrl(allimgs[x].src, change.bind(null, x),1);
    console.log();
  }
}

I also switched your imglen to get its length dynamically.
Here is the jsfiddle I used to proof it out: https://jsfiddle.net/9L3ofjba/
I'll remove my other answer if this solves your issue.

Javascript average colour of img displayed on mulitple backgrounds

2 Answers2