-1

I have this code there find the dominated color of an image (using RGBaster.js) then convert it to a flat color, and give the information box beside the image the color. At the same time, it makes the text color white or black after the YIQ scala.

function getContrastYIQ(hexcolor){
    var r = parseInt(hexcolor.substr(1,2),16);
    var g = parseInt(hexcolor.substr(3,2),16);
    var b = parseInt(hexcolor.substr(4,2),16);
    var yiq = ((r*299)+(g*587)+(b*114))/1000;
    return (yiq >= 128) ? 'black' : 'white';
}



var img = $('.category').attr('data-background');
var colors = RGBaster.colors(img, {
    paletteSize: 5,
    success: function(colors){

        console.log("-----------------------------------------------");
        console.log("Dominant color:", colors.dominant);
        console.log("-----------------------------------------------");

        var rgb = colors.dominant;
        rgb = rgb.substring(4, rgb.length-1).replace(/ /g, '').split(',');

        var flatColors = FlatColors(rgb);
        var flatColor = flatColors[3];

        var textColor = getContrastYIQ(flatColor);
        console.log(textColor);

        $(".category").css('background-image', 'url(' + img + ')');
        $(".category .info").css('background-color', flatColor);
        $(".text").css('color', textColor);
        $(".text").text(flatColors[4]);
    }
});

Here comes the problem


I have multiple divs named like follow:

<div class="category" data-background="images/7.jpg">
    <div class="info">
        <p class="text">Hello World</p>
    </div>
</div>

And the code are need to find the dominated color for each category div and paint the info div with it.

I have tried to do this:

function colorPick(img) {
    RGBaster.colors(img, {
    paletteSize: 5,
        success: function(colors){

            var rgb = colors.secondary;
            rgb = rgb.substring(4, rgb.length-1).replace(/ /g, '').split(',');

            var flatColors = FlatColors(rgb);
            console.log("Return:", flatColors[3]);

            return flatColors[3];                       
        }
    });
}



$('.category').each(function(){
    var img = $(this).attr('data-background');

    $(this).css('background-image', 'url(' + img + ')');

    var color = colorPick(img);
    console.log(color);
});

But that didn't work. so now I need some help.

TheCrazyProfessor
  • 919
  • 1
  • 15
  • 31

1 Answers1

0

You need to use a promise.

You have a callback action - something that fires later than the rest of your code:

function colorPick(img) {
    RGBaster.colors(img, {
    paletteSize: 5,
        success: function(colors){ 

            var rgb = colors.secondary;
            rgb = rgb.substring(4, rgb.length-1).replace(/ /g, '').split(',');

            var flatColors = FlatColors(rgb);
            console.log("Return:", flatColors[3]);

            return flatColors[3]; // returns the parent function, success                       
        }
    });

    // colorPick itself doesn't return at all, which is why you get undefined
}

You need to return a Promise - an object that holds the asynchronous action and resolves when it completes:

function colorPick(img) {
    // Now colorPick returns the promise
    return new Promise(function(resolve, reject) {
        RGBaster.colors(img, {
            paletteSize: 5,
            success: function(colors){ 

                var rgb = colors.secondary;
                rgb = rgb.substring(4, rgb.length-1).replace(/ /g, '').split(',');

                // Check this shouldn't be "new FlatColors", as CapitalCase usually means it needs "new"
                var flatColors = FlatColors(rgb);
                console.log("Return:", flatColors[3]);

                // Resolve the promise
                // I've passed it the FlatColors object so the callback can access all of it
                resolve(flatColors); 
            }
        });
    });
}

Then, to access this you either use .then:

$('.category').each(function(){
    var $this = $(this); // avoid calling multiple times
    var img = $this.attr('data-background');

    $this.css('background-image', 'url(' + img + ')');

    // .then fires the function when the promise completes.
    colorPick(img).then(function(colors) {
        var flatColor = colors[3];

        // Set the background colour
        $this.find('.info').css('background-color', flatColor);

        var textColor = getContrastYIQ(flatColor);
        var $text = $this.find('.text');

        $text.css('color', textColor);
        $text.text(colors[4]);

        return colors; 
    }).then(console.log); // You can chain promises
});

Or you use async/await syntax:

$('.category').each(async function(){
    const $this = $(this); // avoid calling multiple times
    const img = $this.attr('data-background');

    // If we can use async/await we can also use const and iterpolated strings
    $this.css('background-image', `url(${img})`);

    // await continues this function after the promise completes
    const color = await colorPick(img);
    const flatColor = colors[3];

    // Set the background colour
    $this.find('.info').css('background-color', flatColor);

    const textColor = getContrastYIQ(flatColor);
    const $text = $this.find('.text');

    $text.css('color', textColor);
    $text.text(colors[4]);

    console.log(colors);
});

I recommend await, but if you support older browsers you may need to transpile your JS as it's fairly new.

That method resolves one colour at a time, but you may want to compare the results of multiple colours. The way to do that is with Promise.all, which waits for a collection of promises to finish and then returns an array of their results:

// Get the dominant colours in all the images on one array
const colors = await Promise.all(
    images.map(i => colorPick(i)));

That maps all the images to an array of promises, all waiting for the colorPick function to finish, and then waits for all of them to finish.

Finally, your question has been marked as a duplicate of this one. The top answer for that question goes into much more extensive detail on using promises and async/await than I have here.

Keith
  • 150,284
  • 78
  • 298
  • 434
  • Have a +1 to mitigate the -1's from the incomplete answer earlier. Or you could just link to the duplicate: https://stackoverflow.com/questions/14220321/how-do-i-return-the-response-from-an-asynchronous-call/14220323 – freedomn-m Sep 08 '17 at 15:15
  • @freedomn-m I don't think this is a duplicate. The solutions are the same, but I don't think the OP even realised that was what their problem was or knew to search for _asynchronous_ calls in the first place. – Keith Sep 08 '17 at 15:18
  • 1
    I agree that it's not always obvious what the cause is (just see some of the question comments (now deleted)) and this question gets asked about 10 times a day for that very reason. The problem is the same and the solution is the same, therefore duplicate. The duplicate provides *lots* of information including a version using promises and `await` and it makes no sense to reproduce *part* of that information every time this issue is asked (in whatever guise). – freedomn-m Sep 08 '17 at 15:24
  • @freedomn-m The problem isn't the same - at least not to the person we're trying to help. They're asking why this function doesn't work not about AJAX calls. – Keith Sep 08 '17 at 15:30
  • 4
    The problem is **exactly** the same. The question here is why a `return` from within a callback doesn't work as the OP expects. They don't know enough about how asynchronous callbacks work to ask the right question, but that **is** the question they need to ask. – user229044 Sep 08 '17 at 15:44
  • I see why you think they're different. However, they're using RGBaster with a success callback, which is a callback from... wait for it... an asynchronous javascript `onload` event handler (the AJ part of ajax). Again, not always obvious what the cause is. – freedomn-m Sep 08 '17 at 15:53
  • @meagar exactly: they don't know the right question to ask. I'm not sure closing their question as a dupe is helpful in that context. – Keith Sep 08 '17 at 16:08
  • @freedomn-m yes, so after picking apart what the component does you're able to show what I already agree with - the ultimate solution is the same. That doesn't actually help the OP though, unless you put that detail in an answer and teach them why it's the case. If a question requires any explanation as to why the answer to a different question helps here too then it deserves an answer. – Keith Sep 08 '17 at 16:12
  • 2
    @Keith That isn't how this site works. The OP needs to go to the linked duplicate and read the question. The code samples are practically identical to theirs, even if the question they asked wasn't quite the same. Once they've read the question, they should have a better understanding of why the answers are correct. – user229044 Sep 08 '17 at 16:49
  • 1
    @meagar er. No. That's exactly how this site works: ask a question, get help. That's what I'm here to do, anyway. OP doesn't even know what Promises are or how ask about async callbacks - I'm here to help educate them. Duplicates should mean the same question, not the same ultimate answer once the component being used has been dissected. I could close 80% of the questions on this site with that rule. – Keith Sep 08 '17 at 19:23
  • 2
    @Keith You *should* close 80% of the questions on this site by that rule. Questions like this one get asked a dozen times a day, with our without the words "promises" or "async" in them. They all should be closed as duplicates of the linked question. The point of this site is provide **canonical answers**, not to provide *unique* answers to every variation of the same question that people can come up with. You're not helping OP by providing **worse** answers than the extremely comprehensive ones that [already exist](https://stackoverflow.com/a/14220323/229044), and you're not helping the site. – user229044 Sep 08 '17 at 19:25
  • @meagar nah mate. Like I said, I'm here to help, not demonstrate my superiority to beginners. I only close actual duplicates. Questions like this I'll go on trying to help educate and explain. – Keith Sep 08 '17 at 19:27
  • 3
    It appears you are **only** interested in demonstrating your knowledge, otherwise you'd be happier linking people to the existing **better** answers. You're welcome to continue answering questions instead of linking to the existing duplicates, but don't be surprised when your answers wind up deleted along with the duplicate questions. – user229044 Sep 08 '17 at 19:31
  • @meagar yes. There should be only one authoritative answer to all promise and async related questions on this entire site. Let's go close all the rest as dupes. Look, I've no problem linking to better or more complete answers - I do it all the time. I'm not disputing that answer to that other question was a very fine and complete explanation. Ultimately this OP needs to learn that they need a promise in the first place - your way to do it is to close their question and hope they figure out the step they're missing, my way is to answer their question. – Keith Sep 08 '17 at 19:40
  • @meagar answer updated, I now cite that duplicate answer directly. I still answer this question as it's own thing - I think we'll have to agree to disagree on that one. – Keith Sep 11 '17 at 07:03