AtomiCookiez
AtomiCookiez

Reputation: 61

Javascript average colour of img displayed on mulitple backgrounds

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>

Upvotes: 2

Views: 180

Answers (2)

MaxPRafferty
MaxPRafferty

Reputation: 4977

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.

Upvotes: 1

MaxPRafferty
MaxPRafferty

Reputation: 4977

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

Upvotes: 1

Related Questions