Forum Moderators: coopster

Message Too Old, No Replies

Array issues

Can this could be done better?

         

Matthew1980

8:07 pm on Jul 12, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Hi all,

This code functions fine:-


function SelectColour(){
$FontColour = array("#FF0000"=>"red","#00FF00"=>"green","#0000FF"=>"blue", "#000000"=>"white", "#FFFFFF"=>"black");

$FontColourSelection = "<select name=\"SelectColour\">";
$FontColourSelection .= "<option value=\"\">Choose font Colour</option>";
foreach($FontColour AS $FontColourKey=>$FontColourValue){
$FontColourSelection .= "<option value=\"".$FontColourKey."\">".$FontColourValue."</option>";
}
$FontColourSelection .= "</select>";

return $FontColourSelection;
}




I am just wondering if I could do this any better or is there another way to do similar that I haven't thought about?

Thanks for any suggestions.

Cheers,
MRb

rocknbil

1:03 am on Jul 13, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Well first, you missspeeeeled color.

(couldn't resist, a little American humor - OOPS - humour - there.)

But seriously, #000 is black, #fff is white. :-)

Yeah I'd take this a step further.


function SelectColour($name,$style=null,$action=null,$value=null) {
$FontColour = array(
"#FF0000"=>"red",
"#00FF00"=>"green",
"#0000FF"=>"blue",
"#ffffff"=>"white",
"#000000"=>"black"
);
//
$FontColourSelection = "<select name=\"$name\" id=\"$name\"";
if ($style) { $FontColourSelection .= " $style"; }
if ($action) { $FontColourSelection .= " onchange=\"$action\"; }
$FontColourSelection .= ">";
$FontColourSelection .= "<option value=\"\">Choose font Colour</option>";
foreach($FontColour AS $FontColourKey=>$FontColourValue){
// Note since you're using double quotes, no need to concatenate
$FontColourSelection .= "<option value=\"$FontColourKey\"";
// or selected="selected" for XML - might even pass that as a param
if ($value) { $FontColourSelection .= ' selected'; }
$FontColourSelection .= ">$FontColourValue</option>";
}
$FontColourSelection .= "</select>";
//
return $FontColourSelection;
}


Call it like

$val = (isset($_POST['SelectColour']))?$_POST['SelectColour']:null;
$color_list = SelectColour('SelectColour','class="toobar"','check_something()',$val);

As for the array itself, you may find it to your advantage to store those in a database and grab them from there, or even a plain text file. If you ever want to add colors it would make it much easier for say, a non-PHP person to do so by editing a text file or database value.

Matthew1980

7:12 am on Jul 13, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Hi there Rocknbil,

>>you missspeeeeled color

I wondered if anyone would pull me up on that ;)

I noticed after I posted that I had the colo(u)r values mixed up, fortunately it's not really a show stopper.

Thanks for the suggestions, I was wanting to put a css class parameter in instead of doing it manually & actually naming the element gives a bit more control.

And I like the idea of the option of using onChange="" in the mix too, so instead of submitting I can just check to see a change that way, but that's borderline ajax isn't it - and I don't know any ajax, I feel a google coming on :)

>>As for the array itself, you may find it to your advantage to store those in a database and grab them from there, or even a plain text file

That's already going into V1.01 of this script, the reason I posted was I wanted to see if I could improve the base function and add anything I had overlooked, and you answered my question quite nicely.

Thank you for the idea's there Rocknbil,

Cheers,
MRb

optik

9:15 am on Jul 13, 2010 (gmt 0)

10+ Year Member



I can't see the point of using an array here when you have fixed data in the function, if you were passing an array to the function then the loop would make sense.

You could just return the menu as straight HTML and the function would have the same functionality.

Matthew1980

9:35 am on Jul 13, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Hi there Optik,

>>I can't see the point of using an array here when you have fixed data in the function, if you were passing an array to the function then the loop would make sense.

This was just an exercise to illustrate something that I am modifying at present, though you are correct, I shall be passing the array into the function - just as rocknbil pointed out :)

I have the loop there so that I don't manually type out each instance that I wanted populated in the option list, saved a few lines of code there that's all :)

Cheers,
MRb

rocknbil

5:50 pm on Jul 13, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



onChange="" in the mix too, so instead of submitting I can just check to see a change that way, but that's borderline ajax isn't it


Not necessarily, but be sure you use lower case, onchange. onChange is html 4+only, lower case will be valid for both HTML and XHTML as well as html 5.

I threw onchange in there as a starting point, it doesn't have to be ajax and there are other event handlers for select lists, you may elect to pass the actual handler like I did with style (or eliminate it entirely, just ideas.) You can write it to say, trigger another function that actually colors a selected item in the text field, or open a hidden div (when color is selected, now select font style, or whatever.)

And agreed, passing an array to it would make it even more expandable, it just all depends on how it's implemented. A completely generic approach would be to just pass all select list parameters to it and use it for all select lists, but it all depends on your context of usage.