ColourPicker class

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

ColourPicker class

Post by pickle »

Hi all,

I just finished whipping up a quick colour picker and I was wondering if you folks thought it was worth releasing. There are 1001 other colour pickers out there, but this one is way simpler to implement IMHO.

Implementation:

Code: Select all

<html>
 <head>
  <script src = "path/to/ColourPicker.php?mode=js"></script>

  ...


  <form name = "myForm">
   <input type = "text" name = "myCode" />
   <input type = "submit" onClick = "newColourPicker('myForm.myCode');return false;" />
  </form>

example This URL will change if this gets approved

And now the code:

Code: Select all

<?PHP

if($_GET['mode'] == 'js')
{
  header('Content-type: text/javascript');
  $window_url = $_SERVER['SCRIPT_NAME'];
  

  echo <<<JS
    function newColourPicker(id)
    {
      var myWindow = window.open('$window_url?element=' + id,'colourPicker','height=200, width=160, menubar=no, resizable=yes, scrollbars=no, status=no, titlebar=no, toolbar=no');
    }
JS;
exit();
}

?>


<html>
 <head>
  <style type = "text/css">
    body{
     margin:0px;
     padding:0px;
   }
  </style>
  <title>
   Colour Picker
  </title>
  <script type = "text/javascript">
  function setWellColour(colour)
  {
    document.getElementById('well').style.backgroundColor = colour;
  }
  function convertToHex(colour)
  {
    var hex_r;
    var hex_g;
    var hex_b;

    pattern = /rgb\((.*)\)/;
    var match = pattern.exec(colour);

    //if the colour was in the format rgb(RRR,GGG,BBB) (we're in Firefox)
    if(match)
    {
      var rgb = match[1].split(", ");
      var r = rgb[0];
      var g = rgb[1];
      var b = rgb[2];
  
      hex_r = (r - 0).toString(16);
      hex_g = (g - 0).toString(16);
      hex_b = (b - 0).toString(16);
  
      final_code = (hex_r.length == 2) ? hex_r : hex_r + hex_r;
      final_code += (hex_g.length == 2) ? hex_g : hex_g + hex_g;
      final_code += (hex_b.length == 2) ? hex_b : hex_b + hex_b;
  
      return(final_code);
    }
    
    //if it didn't match, we must be in IE
    else
    {
      hex_r = colour.substring(1,2);
      hex_g = colour.substring(2,3);
      hex_b = colour.substring(3,4);
      return hex_r + hex_r + hex_g + hex_g + hex_b + hex_b;
    }
  }
  </script>
 </head>
 <body>

<?PHP
$element = $_GET['element'];
$ColourPicker = new ColourPicker($element);
$ColourPicker->go();
?>

<div style = "border:1px solid #333;height:32px;width:152px;padding:3px;">
 <div id = "well" style = "font-family:Verdana,sans-serif;font-size:8pt;height:32px;width:152px;">
 </div>
</div>

<?PHP
class ColourPicker
{
  var $step;
  var $colour_max;
  var $dimension;
  var $font_size;
  var $element_id;

  function ColourPicker($p_element_id)
  {
    $this->step = 1;
    $this->colour_max = 15;
    $this->dimension = '5px';
    $this->font_size = '3pt';
    $this->element_id = $p_element_id;
  }

  function go()
  {
    echo "<div style = 'font-size:$this->font_size;'>";

    //----------------
    //dump green & blue
    //Top left: turqoise
    //Bottom right: black


    $start_g = $this->colour_max;
    $end_g = 0;
    $start_b = $this->colour_max;
    $end_b = 0;

    echo '<div style = "float:left;">';
    $this->dumpGrid($start_g,$end_g,$start_b,$end_b,'g','b');
    echo '</div>';

    //-----------------
    //dump green & red
    //Top left: green
    //Bottom right: red

    $start_g = $this->colour_max;
    $end_g = 0;
    $start_r = 0;
    $end_r = $this->colour_max;

    echo '<div>';
    $this->dumpGrid($start_g,$end_g,$start_r,$end_r,'g','r');
    echo '</div>';

    //-----------------
    //dump blue & red
    //Top left: blue
    //Bottom right: red

    $start_b = $this->colour_max;
    $end_b = 0;
    $start_r = 0;
    $end_r = $this->colour_max;


    echo '<div style = "float:left;">';
    $this->dumpGrid($start_r,$end_r,$start_b,$end_b,'r','b');
    echo '</div>';

    echo '<div>';
    $this->dumpMonochromaticGrid();
    echo '</div>';
  }

  function dumpGrid($start_x,$end_x,$start_y,$end_y,$x_rgb,$y_rgb)
  {
    $working_start_x = ($start_x > $end_x) ? $start_x * -1 : $start_x;
    $working_end_x = ($start_x > $end_x) ? $end_x * -1 : $end_x;
    $invert_x = ($start_x > $end_x) ? true : false;

    $working_start_y = ($start_y > $end_y) ? $start_y * -1 : $start_y;
    $working_end_y = ($start_y > $end_y) ? $end_y * -1 : $end_y;
    $invert_y = ($start_y > $end_y) ? true : false;

    for($i = $working_start_x; $i <= $working_end_x; $i += $this->step)
    {
      $x = ($invert_x) ? dechex($i * -1) : dechex($i);
      
      for($j = $working_start_y; $j <= $working_end_y; $j += $this->step)
      {
	$y = ($invert_y) ? dechex($j * -1) : dechex($j);

	
	$r = ($x_rgb == 'r') ? $x : 0;
	$r = ($y_rgb == 'r') ? $y : $r;

	$g = ($x_rgb == 'g') ? $x : 0;
	$g = ($y_rgb == 'g') ? $y : $g;

	$b = ($x_rgb == 'b') ? $x : 0;
	$b = ($y_rgb == 'b') ? $y : $b;

	$this->outputColourSquare($r,$g,$b);
      }
      echo '<br />';
    }
  }


  function dumpMonochromaticGrid()
  {
    for($row = 0; $row <= $this->colour_max;$row += $this->step)
    {
      for($value = 0; $value <= $this->colour_max;$value += $this->step)
      {
	$hex_value = dechex($value);

	$this->outputColourSquare($hex_value,$hex_value,$hex_value);
      }
      echo '<br />';
    }
  }

  function outputColourSquare($r,$g,$b)
  {
    echo <<<SQUARE
<div style = "cursor:pointer;float:left;background-color:#{$r}{$g}{$b};width:$this->dimension;height:$this->dimension;" 
     onMouseOver = "setWellColour(this.style.backgroundColor);"
     onClick = "window.opener.document.$this->element_id.value = convertToHex(this.style.backgroundColor);window.close();">&nbsp;</div>
SQUARE;
  }
}
?>


If anyone can come up with any way to optimize this further, please don't hesitate to let me know!
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Post by Weirdan »

hmm... perhaps it would be nice to factor out the formatting of the color picker to separate css.
Other than that... why not to assemble the color picker entirely in javascript?
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Post by Weirdan »

heck, it's sooo slooowww.... :?
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

Weirdan wrote:heck, it's sooo slooowww.... :?
But very cool :) I like the way it's laid out but I think there are too many colors on your example which is slowing it down a heck of a lot (markup size maybe as much as anything).

I wrote one recently that's used in our form class at work. Doesn't do the greys like yours though although you can specify the number of colors to display. Doubt I'll be allowed to show the code though :(
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

It is nice -- an a little slow.

One thing I don't like is that the red is split. I would rather see the gray scale at the top/bottom/side than in a quadrant.
(#10850)
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Post by pickle »

Weirdan wrote:heck, it's sooo slooowww.... :?
Only in IE ;) That's a problem with rendering, not creating the code. Not sure how to increase that speed. Ideas?


arborint wrote:One thing I don't like is that the red is split. I would rather see the gray scale at the top/bottom/side than in a quadrant.
There are only 3 primary colours - either the Red, Blue or Green would need to be split
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

pickle wrote:
Weirdan wrote:heck, it's sooo slooowww.... :?
Only in IE ;) That's a problem with rendering, not creating the code. Not sure how to increase that speed. Ideas?
Hmmm.... was slow in FF/Linux for me. Downloading the markup takes about 5 seconds. The JS is fast enough in FF though.

IE has issues with it's DOM implementation from what someone told me at work. If there's lots of markup on the page all JS which uses DOM starts to run slowly since IE (so I'm told) scans the entire DOM tree each time something happens which uses up memory and takes time.
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Post by pickle »

d11wtq wrote:Hmmm.... was slow in FF/Linux for me. Downloading the markup takes about 5 seconds. The JS is fast enough in FF though.
Hmm, it loads almost instantly for me. Guess I'll have to think about that. Each square is it's own DIV, so there's tons of markup on the page.

Increasing the '$step' variable to 2 and the '$font_size' variable to 6pt cuts the number of colours in half. I've done so in the example - does it make it any quicker?
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

pickle wrote:Only in IE ;) That's a problem with rendering, not creating the code. Not sure how to increase that speed. Ideas?
Linux/Gnome/Firefox.

pickle wrote:There are only 3 primary colours - either the Red, Blue or Green would need to be split
Maybe do it as a band with the gray scale on the bottom or side -- instead of all the duplicated grays. If you did a band I would do red->violet rather than violet->red.
(#10850)
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Post by pickle »

I'm not sure what you mean.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

Instead of a 16x16 square, do a 24x8 rectangle with an extra 1x8 on the end for gray scale. I think vertical bands of color arranged horizontally with light to dark being top to bottom would be clearer for people.
(#10850)
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Post by pickle »

Oh ok. I understand now.

No. ;)

I like it layed out the way it is so I'm not planning on changing it unless I can get some hella speed increases.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

pickle wrote:
d11wtq wrote:Hmmm.... was slow in FF/Linux for me. Downloading the markup takes about 5 seconds. The JS is fast enough in FF though.
Hmm, it loads almost instantly for me. Guess I'll have to think about that. Each square is it's own DIV, so there's tons of markup on the page.
Considered using XML and client side XSLT to massively reduce the markup size? ;)
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Post by pickle »

Not even a little bit. Not that I've ruled it out, just that it never occurred to me. I guess I could look at it a bit closer when I get a chance.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

pickle wrote:Not even a little bit. Not that I've ruled it out, just that it never occurred to me. I guess I could look at it a bit closer when I get a chance.
I reckon you could get that markup down to about 20% of what it is now... possibly less.

You could end up with sexy looking XML like:

Code: Select all

<grid>
    <row>
        <color=aaaaaa />
        <color=bbbbbb />
        ...... snip .....
    </row>
    <row>
        ..... snip .....
    </row>
</grid>
Post Reply