1
\$\begingroup\$

The reason I asked about standing the test of time is that I previously used PHP echo to create a string and placed it into a div with .innerHTML, but modern browsers were not passing the value of the select box. (Went back and tested wit IE9 and the innerHTML version worked.)

file:js_selecttest.php

<form method="post" action='js_selecttest.php'>
<table>
  <tr>
    <td><input type="text" name="text_input" onClick="create_sel(document.getElementById('select_holder'),'class_date',op_array)">
    <div id="select_holder">Filled by JS</div>
    <br><input type="submit"></td>
  </tr>
</table>
</form>

<script type="text/javascript">
function create_sel(container,sel_name,op_array)
  {
    var sel = document.createElement('select');
    sel.name=sel_name;
    populateList(op_array,sel);
    container.innerHTML='';
    container.appendChild(sel);
  }

function populateList(op_array,sel_obj) // from http://www.webdeveloper.com/forum/showthread.php?206250#4
  {
    var arLen=op_array.length;
    for(var i=0; i<arLen; i++)
      {
        sel_obj.options[i]=new Option(op_array[i], i);
        sel_obj.options[i].value=op_array[i];
      }
  }
<?php
$op_array=array('Please Select','One','Two','Three','Four','Five','Six',);
?>
var op_array = <?php echo json_encode($op_array) ?>;

</script>
<?php
if($_POST)
  {
    var_dump($_POST);
  }
?>

Would you have any suggestions or improvements?

\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

To answer your question in the title: no For multiple reasons. One being that the things we use today could be out-dated in a few years. who knows

One question/remark:

sel_obj.options[i]=new Option(op_array[i], i);
sel_obj.options[i].value=op_array[i];

is there a reason why you are setting the value twice? (once in the new Option call and once on the next rule)

My suggestions are:

Separation of concerns

You have one file with html, javascript and php all interweaved with each other. So debugging won't make it any easier. Alsog reusing a part of that script is nearly impossible since everything is hard coded.

Overuse of javascript

Is there a reason why you are building the html with javascript? Because this has some drawbacks: Screenreaders will not be able to see the selectlist items since they don't parse javascript. Serach engines will not know about the select-options. Instead you should use PHP to create the html for the select items.

Hard to read code

The code is very hard to read, the function and variable names don't clearly tell what they are or have typehinting in the names. For instance op_array variable. Words as Array, String, int,.. should be banned from variable names. Simply call it options. Now by simply reading it I allready know that is has multiple values and that it are the options. Where as op_array tells me that it is an array of 'op'.

Also for future enhancements. If for some reason you are going to change the way options are passed in, you will also have to change the name of the variable...

Also be more consistent in your naming. populateList() needs a select and options. But no list. This is confusing.

Personally I never use the onClick attributes inhtml. Not because they are bad. But because if you do use them, javascript will be hidden in the html and tightly coupled with it whereas if you EvntListners it is very easy to change the html without having to bother about the javascript. this is also better for sepperation of concerns

\$\endgroup\$
3
  • \$\begingroup\$ The HTML with onClick is just 1 way to implement this & the PHP json is just 1 way to fill the array. It could be ajax as well. I'll test again, but the redundant code was because I think I was getting value="" in those elements even though the inner text was filled. The re-naming of vars and funcs make sense and I thank you for pointing those out to me. In the case for which I wrote this, I ended up not using it. I put hidden divs in with various selects and used js to unhide them. Then the handler script has to deal with which ones are filled or empty \$\endgroup\$
    – TecBrat
    Jul 27, 2013 at 14:45
  • \$\begingroup\$ @TecBrat the PHP json way is a weird way to do what you are doing. You are rendering a part of the html in php on serverside and another part on clientside. This is simply bad practice \$\endgroup\$
    – Pinoniq
    Jul 28, 2013 at 10:53
  • \$\begingroup\$ The use case is where there is a class schedule. The site owner manages the available class dates. The end-user selects between the full class, or just the last day with the exam. The PHP will gather the available dates for the selected class. Then, when the end-user chooses the full class or just the end date, the select box appears with the correct options. A <noscript> is used to offer all options. "bad practice" is not helpful. Can you finish this sentence? "That practice will cause a problem when..." \$\endgroup\$
    – TecBrat
    Jul 28, 2013 at 22:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.