An option cannot trigger an onchange event. The onchange event must be associated with the select element instead.
Remove the onchange event from the option, and add a new onchange event to the select element, from where we will choose what to do.
I've also updated the value for the empty option to be empty, so that the keywordHandler can use the value of the selected option to decide what to do.
HTML Code:
<select name="operator[]" onchange="keywordHandler()">
<option value="">Choose Operator</option>
<option value="">empty</option>
<option value="=">equals (=)</option>
</select>
Now create the keywordHandler() function, which looks at the selected value and decides which function to run.
Code javascript:
function keywordHandler(el) {
if (el.value === '=') {
no();
} else {
yes();
}
}
So that now works.
There are some refinements that could be made though. What do the no() or yes() functions do? Can you tell without looking at the functions themself? The functions need to have more descriptive names, such as showKeyword and hideKeyword.
Also - now that we have the keywordHandler function, we can move common behaviour out of the showKeyword and hideKeyword functions so that less duplication occurs, and that each function does one main thing.
Code:
function noshow() {
MMDiv.style.visibility='visible';
mainform.MakeModel.focus();
}
function yeshide() {
MMDiv.style.visibility='hidden';
mainform.MakeModel.focus();
}
function keywordHandler(el) {
if (el.value === '=') {
noshow();
} else {
yeshide();
}
mainform.MakeModel.focus();
}
We can also use a more expressive manner in which to access the element with the MMDiv identifier.
We could also move the element reference out of the function, and pass it in as a function property. So now that the functions can handle hiding and showing different elements, their name can be made more generic now.
Also, instead of using "mainform.MakeModel", we really should use the more expressive form of that instead.
Code:
function show(el) {
MMDivel.style.visibility='visible';
}
function hide(el) {
MMDivel.style.visibility='hidden';
}
function keywordHandler(el) {
var form = mainform,
keyword = document.getElementById('MMDiv');
if (el.value === '=') {
show(keyword);
} else {
hide(keyword);
}
mainform.elements.MakeModel.focus();
}
A final refinement is to remove the inline event attribute from your HTML code, and move it in to your scripting where it belongs instead.
We can easily do that by removing the HTML attribute from your HTML code, and assign the onchange event from your script instead. The most effective way to do that is to move your scripting down to the bottom of the body, just before the </body> tag so that you can easily work with page elements as the page is loading.
Code:
<body>
<script>
...
</script>
<form>
...
</form>
...
<script>
...
</script>
</body>
Now we can streamline things even further, by removing from the HTML code the onchange event and applying it from the scripting instead.
The benefit with that is that the this keyword is then automatically available to the function, so there's no need for handler function to have the function parameter anymore. Instead, the function easily accesses the select element via the this keyword, and can also easily get to the form from the select elements automatic reference that it keeps to the form itself.
We could also rename MMDiv to something more appropriate, such as keyword.
The resulting code is this:
HTML Code:
<select name="operator[]">
<option value="">Choose Operator</option>
<option value="">empty</option>
<option value="=">equals (=)</option>
</select>
<div id="keyword" style="visibility:visible; position:absolute; left:418px; top:-1px;">
<input type="text" name="keyword[]" />
</div>
and at the end of the body, just before the </body> tag is your scripting code:
Code javascript:
function show(el) {
el.style.visibility='visible';
}
function hide(el) {
el.style.visibility='hidden';
}
function keywordHandler() {
var operator = this,
form = operator.form,
keyword = document.getElementById('keyword');
if (operator.value === '=') {
show(keyword);
} else {
hide(keyword);
}
form.elements.MakeModel.focus();
}
var form = document.getElementById('mainform');
form['operator[]'].onchange = keywordHandler;
Reply With Quote
Originally Posted by multichild
Bookmarks