0

I have a function which works only once. I'm sure that's because it says "return" and I haven't got any loops, etc. Could anyone help me un-noob this function? (It's all in a jQuery container, although I don't think jQuery probably has much to do with the question.)

The HSL color returned from Farbtastic is in a format like (hue,saturation,lightness). I'm adjusting the 3rd value, lightness, and that part works. All the repetitions don't work though. It makes all the new swatches the color of the first swatch.

    function onColorChange(color) {

        // retrieve HSL color value
        var hslcolor = $.farbtastic('#main_color_picker').hsl;

        // create 4 new colors of varying brightness, and variable names to save them to db.
                var newcolor1 = hslcolor;
                var newcolor2 = hslcolor;
                var newcolor3 = hslcolor;
                var newcolor4 = hslcolor;
                    newcolor1[2] = 0.10 * (Math.round(hslcolor[2]*10000)/10000);
                    newcolor2[2] = 0.85 * (Math.round(hslcolor[2]*10000)/10000);
                    newcolor3[2] = 1.15 * (Math.round(hslcolor[2]*10000)/10000);
                    newcolor4[2] = 1.50 * (Math.round(hslcolor[2]*10000)/10000);
                var rgb1 = hsl2rgb(newcolor1);
                var rgb2 = hsl2rgb(newcolor2);
                var rgb3 = hsl2rgb(newcolor3);
                var rgb4 = hsl2rgb(newcolor4);

        //apply the color to swatches and show original swatch in the middle.
        var firstSwatch = $('#section-main_color').find('.square1');
        firstSwatch.css( 'background-color', 'rgb('+rgb1.r+','+rgb1.g+','+rgb1.b+')' );
        var secondSwatch = $('#section-main_color').find('.square2');
        secondSwatch.css('background-color', 'rgb('+rgb2.r+','+rgb2.g+','+rgb2.b+')');
                    // original color is in square 3 
        var fourthSwatch = $('#section-main_color').find('.square4');
        fourthSwatch.css('background-color', 'rgb('+rgb3.r+','+rgb3.g+','+rgb3.b+')');
        var fifthSwatch = $('#section-main_color').find('.square5');
        fifthSwatch.css('background-color', 'rgb('+rgb4.r+','+rgb4.g+','+rgb4.b+')');

    }

function hsl2rgb(hsl) {
    var h = hsl[0], s = hsl[1], l = hsl[2];
    var m1, m2, hue;
    var r, g, b
    h = (Math.round( 360*h )/1);
    if (s == 0)
        r = g = b = (l * 255);
    else {
        if (l <= 0.5)
            m2 = l * (s + 1);
        else
            m2 = l + s - l * s;
        m1 = l * 2 - m2;
        hue = h / 360;
        r = Math.round(HueToRgb(m1, m2, hue + 1/3));
        g = Math.round(HueToRgb(m1, m2, hue));
        b = Math.round(HueToRgb(m1, m2, hue - 1/3));
    }
    return {r: r, g: g, b: b};
}

For part 1 of this question, about HSL using Farbtastic in Theme-Options-Panel, see: Farbtastic convert HSL back to RGB or Hex

Followup: If you need an actual copy of an array, I found this bit:

        var newcolor = hslcolor.slice(0);

It's probably a good idea to avoid using this in most cases. I found I needed to retain the original array as-is, for use in another set of calculations, so I did my brightness on a copy.

Community
  • 1
  • 1
Jen
  • 1,663
  • 1
  • 17
  • 34
  • 1
    Could you try to reproduce the error in http://jsfiddle.net/ ? – nicosantangelo Aug 06 '12 at 00:25
  • That has nothing to do with using `return` or not using loops. Most functions return different values for different inputs and I'm sure this function does as well. The error must be somewhere else. As already said, please create a jsfiddle demo, it would help us to help you. – Felix Kling Aug 06 '12 at 00:33
  • Not working: http://jsfiddle.net/uttqX/5/ And here's the example working, but only iif I do just one color: http://jsfiddle.net/MdN7X/ – Jen Aug 06 '12 at 00:59

1 Answers1

0

Is this what you are looking for: http://jsfiddle.net/uttqX/6/ The reason why it didn't work, is because you were updating the same variable over and over again, because you assigned all variables to the same initial variable in the begging. So any update to any one of those variables was propagating to the initial variable (hslcolor) and then back to all the other variables.

Ilya Volodin
  • 10,929
  • 2
  • 45
  • 48
  • 1
    A better explanation is that an assignment like `newcolor1 = hslcolor` does NOT make a new copy of the array. Both variables refer to the same array, so a change to element [2] of one affects the other. See http://my.opera.com/GreyWyvern/blog/show.dml/1725165. – Barmar Aug 06 '12 at 02:25
  • Agree on the comment from Barmar. Was trying to figure out how to explain ByRef vs. ByVal better in a short paragraph but failed. – Ilya Volodin Aug 06 '12 at 02:26
  • I see that it's working, and I also think I understood the explanation, but... what did you change in the code to make it work? It looks exactly the same to me. – Jen Aug 06 '12 at 02:58
  • I moved the call to hsl2rgb function right under each assignment. This way the call to the function is made before any other updates to the variable. In general, you don't need all those newcolor variables, you can just change hslcolor and make a call to hsl2rgb right after. – Ilya Volodin Aug 06 '12 at 03:06