Help with a generated select list being rendered where it should be

[Darkmatter5]

Here's the dropdown menu code I wrote.

 

<?php
function dropdown($name,$select,$default,$onchange) {
  $id=$name. "_id";
  echo "<select name='$name' class='text_boxes'";
  if(isset($onchange)) { echo " onchange='$onchange(this.value)'"; }
    echo "><option value='0'>--SELECT--</option>";
    $query=mysql_query($select) or die(mysql_error());
    while($row=mysql_fetch_array($query)) {
      $r1=$row[$id];
      $r2=$row[$name];
      if($r1==$default) { echo "<option value='$r1' selected='selected'>$r2</option>"; }
      else { echo "<option value='$r1'>$r2</option>"; }
    }
  echo "</select>";
}
?>

 

Here's the code that calls this function.

 

<table width='100%' cellpadding='3' style='border-collapse: collapse; border: 2px solid black;' border='0'>
<tr><th colspan='2' style='font-size: 16px; font-weight: bold;'>Profile</th></tr>
  <form method='post' action='$url/library/scripts/siteadmin_t1.php'>
  <input type='hidden' name='id' value='$row[member_id]'>
  <tr><td width='100' align='right'><b>Username:</b></td><td><input type='text' name='username' value='$row[mem_username]'></td></tr>
  <tr><td align='right'><b>Firstname:</b></td><td><input type='text' name='firstname' value='$row[mem_firstname]'></td></tr>
  <tr><td align='right'><b>Lastname:</b></td><td><input type='text' name='lastname' value='$row[mem_lastname]'></td></tr>
  <tr><td align='right'><b>Email:</b></td><td><input type='text' name='email' value='$row[mem_email]'></td></tr>
  <tr><td align='right'><b>Activation code:</b></td><td>$row[activation_code]</td></tr>
  <tr><th colspan='2'><hr></th></tr>
  <tr><th colspan='2'>Privileges</th></tr>
  <tr><th colspan='2'>Site Admin:<input type='checkbox' name='siteadm' value='" .($row['priv_admin']==1?"1' checked='checked'":"0'"). "> News Admin:<input type='checkbox' name='newsadm' value='" .($row['priv_news']==1?"1' checked='checked'":"0'"). "><br>Forums Admin:</b><input type='checkbox' name='forumsadm' value='" .($row['priv_forums']==1?"1' checked='checked'":"0'"). "> Game Master:</b><input type='checkbox' name='gm' value='" .($row['priv_gm']==1?"1' checked='checked'":"0'"). "></th></tr>
  <tr><th colspan='2'><hr></th></tr>
  <tr><th colspan='2'>Preferences</th></tr>
  <tr><td align='right'><b>Theme:</b></td><td>" .$rpgbuilder->dropdown(theme,'SELECT * FROM themes ORDER BY theme ASC',4,''). "</td></tr>
  <tr><td align='right'><b>Rows per page:</b></td><td><input type='text' name='rows' size='2' value='$row[rowsperpage]'></td></tr>
  <tr><th colspan='2'><input type='submit' value='Submit'></th></tr>
  </form>
</table>

 

Attached is a screenshot of how it's being rendered.

Why's it rendering outside the table?

 

[attachment deleted by admin]

[Psycho]

Look at the rendered HTML to see where the problem is. I suspect the problem may be due to this line in the function

if(isset($onchange)) { echo " onchange='$onchange(this.value)'"; }

 

The PHP is going to try and parse the "$onchange" as a PHP variable since you are using double quotes to define the string. But, ity may also be picking up some of the text in parens.

[Psycho]

Ok, I just realized the the second block of code is apparently within an echo statement. That's the problem. You have an echo calling an echo. So, when the PHP is getting ready to process the echo in the lower block of code it has to call the function first. So, that function will perform it's echo before the echo which called it. The function need to "return" the output instead of echo'ing it.

 

Here's an example of what I mean:

function echoBar()
{
    echo "bar";
}

echo "foo" . echoBar();
//Output barfoo

 

I have rewritten the function (below) to fix that problem and make it flow better. However, I would recommend some changes in the process itself. For example, you create an ID variable that is not used as the ID of the select list, instead it is used to determine the value from the query to be used in the options. I think that it would be much easier to pass queries to the function that will dynamically pull the needed data. For example if you have a table of colors and used color and color_id you could ensure that the values returned were in a generic format for this function

SELECT color as name, color_id as label FROM colors

Just prepare the queries for each table so the values are always returned as name and label (or whatever you want to use)

 

New Function

<?php
function dropdown($name, $selectQuery, $default, $onchangeFunc)
{
  $id = $name. '_id';
  $onchange = (isset($onchangeFunc)) ? ' onchange="{$onchangeFunc}(this.value);" : '';
  
  $options = "<option value=\"0\">--SELECT--</option>\n";
  $query = mysql_query($selectQuery) or die(mysql_error());
  while($row = mysql_fetch_array($query))
  {
      $value = $row[$id];
      $label = $row[$name];
      $selected = ($value==$default) ? ' selected="selected"' : '';
      $options .= "<option value=\"{$value}\"{$selected}>{$label}</option>\n";
  }
  return "<select name=\"{$name}\" class=\"text_boxes\"{$onchange}>{$options}</select>\n";
}
?>

[haku]

Generally in functions like that, rather than using echo statements, you want to assign all the output to a variable, then return the variable. This will prevent problems like you had.

[Darkmatter5]

Thank you for all your suggestions I got it working.