dataarts / dat.gui

Lightweight controller library for JavaScript.
Apache License 2.0
7.46k stars 1.08k forks source link

Color Controller & RGB format bugs #117

Closed WillsB3 closed 7 years ago

WillsB3 commented 7 years ago

Hi,

It looks like there's a few issues when using a Color Controller with colors in RGB (or non HEX) format.

Here's a minimal JSBin: http://output.jsbin.com/cokama

As you can see:

  1. rgbArray and rgbObject show in HEX format in the UI, when they should be RGB.
  2. If you click and drag to change the color of the rgbObject control a color in RGB format is logged to the browser console by the onChange callback. This behaviour is as expected. However when you mouseup the onFinishChange callback logs the final color in HEX format (instead of RGB).

I'm mostly interested in using the RGB Object format, but it looks like the other formats are also inconsistent. Here's a table showing the discrepancies between the values passed to the onChange and onFinishChange callbacks.

Input Type onChange Value onFinishChange Value
CSS String #d4a047 #d4a047
RGB Array [86.44301470588235, 155.99318771626287, 225] #569be1
RGB Object {r: 75.87775735294117, g: 136.92735366205298, b: 197.5} #4b88c5
RGBA Array [78.72549019607844, 119.52210688196843, 160, 0.3] rgba(79,120,160,0.3)
RGBA Object {r: 21.113664215686285, g: 107.14288374663585, b: 192.5, a: 0.3} rgba(21,107,193,0.3)
HSL Object {h: 350, s: 0.8805147058823529, v: 0.7156862745098039} #b61530

Am I missing something or is CSS String the only one that works correctly (i.e. the format of the value passed to the onFinishChange callback is the same as the input format?)

customlogic commented 7 years ago

Hey Wills, thanks for the great write up and JSBin. I'll get a fix into the next update

customlogic commented 7 years ago

Thanks again Wills. I've fixed the bugs and added better string formatting for the various color labels

customlogic commented 7 years ago

Here's an example with all the Color formats that dat.gui accepts: https://jsfiddle.net/8cffyzL1/

WillsB3 commented 7 years ago

Excellent, Thanks Jeff!

AlexBezuska commented 7 years ago

I wrote a function that will take in hex or standard rgba strings and convert them to rgba arrays for datgui, I feel like this could smooth over how it is used since I feel like a lot of people would like to just use normal rgba strings like in css:

function hexToRgbA(hex){
    var c;
    if(/^#([A-Fa-f0-9]{3}){1,2}$/.test(hex)){
        c= hex.substring(1).split('');
        if(c.length== 3){
            c= [c[0], c[0], c[1], c[1], c[2], c[2]];
        }
        c= '0x'+c.join('');
        return 'rgba('+[(c>>16)&255, (c>>8)&255, c&255].join(',')+',1)';
    }
    throw new Error('Bad Hex');
}

// datgui wants this format
function parseDatGuiRGBA(color) {
  if (color.indexOf("#") === 0 ) {
    var newColor =  hexToRgbA(color).replace("rgba(", "").replace(")", "");
    return JSON.parse("[" + newColor + "]");
  }
  if (color.indexOf("rgba") === 0 ) {
    var newColor = color.replace("rgba(", "").replace(")", "");
    return JSON.parse("[" + newColor + "]");
  }
  return color;
}

Example:

parseDatGuiRGBA( "#93d5ba" );
// returns array [147, 213, 186, 0.5] 

parseDatGuiRGBA( "rgba(147,213,186,0.5)" );
// returns array [147, 213, 186, 0.5]

parseDatGuiRGBA( [147, 213, 186, 0.5] );
// returns array [147, 213, 186, 0.5]

I hope this is useful, let me know if you think it is appropriate for a PR.