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
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

d11wtq wrote:Of which you can find by:

Code: Select all

for (var prop in theObject)
{
    document.write(prop+'<br />');
}
That is so obvious and cool I can't believe I never did that. Thank you, thank you, thank you....
(#10850)
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Shazaam!

Post by pickle »

Ok, by using ~burrito as a sound-board, I managed to figure it out. fromElement wasn't the property I wanted at all, it was actually srcElement.

The updated code is below. I've moved the page setup into the class to clean things up a bit, & the functions now have comments. The more I look at it though, the more I think it needs a little more work to clean up the code.


What's the speed like for you guys now?

Code: Select all

<?PHP

//------------------------------------
//If we're acting as javascript, do so
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();
}


//----------------------------
//if we're here, dump the grid
$element = $_GET['element'];
$ColourPicker = new ColourPicker($element);
$ColourPicker->go();







//**************************
// Class: ColourPicker
// Author: Dylan Anderson
// Date: July 12, 2006
// License: GPL
// Favorite movie: Tron
//**************************
class ColourPicker
{
  var $step;
  var $colour_max;
  var $dimension;
  var $font_size;
  var $element_id;

  function ColourPicker($p_element_id)
  {
    //the amount to increment the R, G, & B values
    //a higher number here means fewer colours
    $this->step = 2;
    
    //the maximum saturation of the R, G  & B values
    //a lower number here means darker colours
    //don't make it any higher than 15
    $this->colour_max = 15;

    //the dimension of the little squares.  If this value changes,
    //you should fiddle with font_size as well to make everything line up
    $this->dimension = '10px';

    //the font size of the page.  This has to be fiddled with
    //in order to make the <br /> character make everything line up
    $this->font_size = '6pt';

    //the form element in the parent window that the finally chosen colour code
    //gets dumped to
    $this->element_id = $p_element_id;
  }


  //---------------------------------------------------
  //Function: go()
  //Purpose: To mastermind all the work
  function go()
  {
    $this->outputPageHead();

    echo <<<OPEN
<div style = "font-size:$this->font_size;" onmouseover = 'showColour(event);' onclick = 'setColour(event);'>
OPEN;

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


    //the 'start' variables set the saturation
    //of R, G, or B at the beginning of square
    //the 'end' variables set the saturation at the end
    $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>';


    //dump the well so people can see what they've selected
    echo <<<WELL
<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>
WELL;
  }

  //---------------------------------------------------
  //Function: dumpGrid
  //Purpose: to output a grid of colours that start with a mixture
  //         $start_x and $start_y, and end with a mixture of $end_x and $end_y
  //
  //As you may have guessed, the first two variables the range of saturation
  //in the columns of the grid.  $x_rgb sets whether these two variables control
  //the red (r), green (g), or blue (b) saturation.
  //
  //The same applies for $start_y and $end_y
  function dumpGrid($start_x,$end_x,$start_y,$end_y,$x_rgb,$y_rgb)
  {
    //Since we want to be able to do both directions (saturation: 0 to saturation: full
    //& vice versa) in only one loop, we make the values negative if we're decreasing saturation
    $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;

    //loop through each of the 'x' colours
    for($i = $working_start_x; $i <= $working_end_x; $i += $this->step)
    {
      //if we've inverted the value, invert back so it looks like intended
      $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()
  //Purpose: dumpGrid needs colours sent along, but a monochromatic grid
  //         is no where that complex & a little different logic.  This
  //         function does that.
  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()
  //Purpose: simply to display a small div with the passed RGB balues
  function outputColourSquare($r,$g,$b)
  {
    echo <<<SQUARE
      <div class = 'colourCell' style = "background-color:#{$r}{$g}{$b}">&nbsp;</div>
SQUARE;
  }

  //---------------------------------------------------
  //Function: outputPageHead()
  //Purpose: A totally dump function that simply dumps some HTML & Javascript code
  //         It's main purpose is to make the code in go() less cluttered
  function outputPageHead()
  {
    echo <<<OPENPAGE
<html>
 <head>
  <title>
   Colour Picker
  </title>
  <style type = "text/css">
    body{
     margin:0px;
     padding:0px;
    }
    .colourCell{
     cursor:pointer;
     float:left;
     width:10px;
     height:10px;
    }
  </style>
  <script type = "text/javascript">
      function showColour(event)
      {
	target = (event.srcElement) ? event.srcElement : event.target;
	if(target)
	{
	  colour = target.style.backgroundColor;
	  document.getElementById('well').style.backgroundColor = target.style.backgroundColor;
	}
      }

      function setColour(event)
      {
	target = (event.srcElement) ? event.srcElement : event.target;

	if(target)
	{
	  colour = target.style.backgroundColor;
	  properColour = convertToHex(target.style.backgroundColor);
	  window.opener.document.$this->element_id.value = properColour;
	  window.close();
	}
      }

      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>
OPENPAGE;
  }
}
?>
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

one thing I'd recommend is don't directly echo anything within the class, as it should be the user's responsibility to choose what he wants to do with the output.
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 »

Well the point of this whole library is to be a colour picker. You'll notice this isn't just a class, but also there's some Javascript function declaration. I wanted to make this one file all anyone would need to implement a colour picker. I wouldn't want the user (if by user you mean person implementing this file) to have to modify the file to make it display nicely.

Long story short, I want this to be as plug-n-play as possible.
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 »

Yeah I think it's fine to have this echo out. Output is just another operation, so why shouldn't a class do it?

~arborint, there's a snippet in Code Snippets called something like "write_r()" posted by me over a year ago which does the equivalent of print_r() in PHP. One issue it has is that it doesn't see recursion though so it can crash the browser on some objects (e.g. where object has a parentNode property and the parentNode has the childNode property). It's pretty useful for double checking what available on various browsers.

EDIT | Not to take the thread off-topic but if anybody knows the correct way to detect recursion which will never end could you PM me? :) cheers.
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Post by Weirdan »

IMO it's enough to limit recursion depth by 2-3 levels (if you gonna output any part of the DOM tree unlimited recursion would lead you to outputting entire tree because there are usually no 'islands' in there).

Btw, I managed to find my javascript dump() implementation on The Deuce via Google cache... gonna post it to code snippets forum (where it had been stored before the McG incident).
User avatar
Weirdan
Moderator
Posts: 5978
Joined: Mon Nov 03, 2003 6:13 pm
Location: Odessa, Ukraine

Post by Weirdan »

Done that... feel free to use and abuse :) The code is under BSD license.

viewtopic.php?p=281804#281804
User avatar
Nathaniel
Forum Contributor
Posts: 396
Joined: Wed Aug 31, 2005 5:58 pm
Location: Arkansas, USA

Post by Nathaniel »

Perhaps I've been unlucky enough to try it everytime Pickle's fiddling with it, but all I ever get is a white page with the popup. I tried it in Opera 9 and FF 1.5, and in Opera, it gives me an "XML parse error" in the popup.

Hope you can fix it, because I am looking forward to seeing how this works :)

- Nathaniel[/img]
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 »

Try it now. I was trying ~d11wtq's idea of using XML & XSLT, but that's not panning out. I've updated the example to use the old class.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
Nathaniel
Forum Contributor
Posts: 396
Joined: Wed Aug 31, 2005 5:58 pm
Location: Arkansas, USA

Post by Nathaniel »

Yup, sure does. That is slick work, pickle. :)
Post Reply