Welcome to WebmasterWorld Guest from 18.207.136.184

Forum Moderators: open

Message Too Old, No Replies

optimize my first javascript validation code

     
11:28 am on Mar 9, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


Hi,
Thanks by everything you have showned me I have done my first validation script.
It uses ids, I know there are smarter ways but I like it as it is mine and I keep learning.

I am validating the inputs are empty and using a focus to jump up to the input as the form is very long.
I wonder what should I use to delete the innerhtml message and focus when a value is inerted into the box?
Suppose maybe onchange then innerthml = ''?

This is my script so far, I wanted to only write the css class ones, but as the id does not exist yet I did not manage.
This is what I have,
Any opinion, something I should improve?:
<script type="text/javascript"> 
function OnSubmitBooking()
{

if(document.Booking.name.value == '')
{
divresult = document.getElementById('namebox')
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in name';
document.Booking.name.focus();
return false;
}
if(document.Booking.passport.value == '')
{
divresult = document.getElementById('passportbox')
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in your identification';
document.Booking.passport.focus();
return false;
}
if(document.Booking.emailtrue.value == '')
{
divresult = document.getElementById('emailtruebox');
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in email';
document.Booking.emailtrue.focus();
return false;
}
if(document.Booking.repeat_email.value == '')
{
divresult = document.getElementById('repeat_emailbox');
divresult.className = 'validation'
divresult.innerHTML = 'Please repeat email';
document.Booking.repeat_email.focus();
return false;
}
if(document.Booking.telmobile.value == '')
{
divresult = document.getElementById('telmobilebox');
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in mobile to bring on holiday';
document.Booking.telmobile.focus();
return false;
}
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
{
divresult = document.getElementById('repeat_emailbox');
divresult.className = 'validation'
divresult.innerHTML = 'Email and repeat email are not equal';
return false;
}
if(document.Booking.test.value =='')
{
divresult = document.getElementById('resultbooking');
divresult.className = 'validation'
divresult.innerHTML = 'Please answer the security question';
document.Booking.test.focus();
return false;
}
if (isNaN(document.Booking.test.value))
{
divresult.className = 'validation'
divresult.innerHTML = 'The answer is invalid, use only numbers in security question';
document.Booking.test.focus();
return false;
}

else{
divresult.style.color="blue";
divresult.innerHTML = 'Please Wait...';}
}
</script>
12:40 pm on Mar 9, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


Tried to add the onchange function inside the other function in the first input but does not work:

function OnSubmitBooking()
{

if(document.Booking.name.value == '')
{
divresult = document.getElementById('namebox')
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in name';
document.Booking.name.focus();
return false;
function quitarfocus()
{
divresult.innerHTML.style.display = "none"}
}
html:
<input id="name" name="name" title="Please enter your name" onchange="quitarfocus()" type="text" size="50">
<div id="namebox"></div>
7:46 pm on Mar 9, 2014 (gmt 0)

Senior Member from US 

WebmasterWorld Senior Member fotiman is a WebmasterWorld Top Contributor of All Time 10+ Year Member Top Contributors Of The Month

joined:Oct 17, 2005
posts: 5021
votes: 26


Some suggestions.

1. Look for repetition. If you're using document.Booking over and over, consider putting it into a shorter variable:

var f = document.Booking; // the form

2. Always declare variables with "var", otherwise you're creating globals, which can have unintended consequences.

var divresult;

3. document.Booking.name.value
Having a form element named "name" can be problematic, because the form element itself can have a name attribute. If you can, consider giving the "name" element a different name. If you can't, then do this instead:

document.Booking.elements['name'].value

4. Each of those "if" bodies looks similar. You could probably make a function that takes the name and message, and reuse it for all of those to prevent duplication.
9:06 pm on Mar 9, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


Thanks for that,
think most are covered now.
I have added an ajax at the end, and it works but always return false.
I need that if there is no responsetext to validate true:

function objetoAjax(){
var xmlhttp=false;
try {
xmlhttp = new ActiveXObject("Msxml2.XMLHTTP");
} catch (e) {

try {
xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
} catch (E) {
xmlhttp = false;
}
}

if (!xmlhttp && typeof XMLHttpRequest!='undefined') {
xmlhttp = new XMLHttpRequest();
}
return xmlhttp;
}

//Función para recoger los datos del formulario y enviarlos por post
function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
{
divresult = document.getElementById('repeat_emailbox');
divresult.className = 'validation'
divresult.innerHTML = 'Email and repeat email are not equal';
document.Booking.repeat_email.focus();
return false;
}
var info = {
names : ['namebox' , 'Please fill in name' ],
passport : ['passportbox' , 'Please fill in your identification' ],
emailtrue : ['emailtruebox' , 'Please fill in email' ],
repeat_email: ['repeat_emailbox', 'Please repeat email' ],
telmobile : ['telmobilebox' , 'Please fill in mobile to bring on holiday' ],
test : ['resultbooking' , 'Please answer the security question' ]
}
for( key in info ) {
if(document.Booking[key].value == '')
{
divresult = document.getElementById(info[key][0])
divresult.className = 'validation'
divresult.innerHTML = info[key][1];
document.Booking[key].focus();
return false;
}
}
divresult = document.getElementById('resultbooking');
//recogemos los valores de los inputs

enviar = document.solicitud.enviar.value;
question = document.solicitud.question.value;
answer = document.solicitud.answer.value;
test = document.solicitud.test.value;
var ajaxcaptcha = objetoAjax();
ajaxcaptcha.open("POST", "captchaajax.php", true);
ajaxcaptcha.onreadystatechange = function () {
if (ajaxcaptcha.readyState == 4) {
divresult.innerHTML = ajaxcaptcha.responseText
<!-- LimpiarCampos();-->
}
}
ajaxcaptcha.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
ajaxcaptcha.send("test=" + test + "&enviar=" + enviar + "&answer=" + answer +
"&question=" + question + "");
return false;
}


This is the php page:
if (isset($_POST['enviar'])){
$question = $_POST['question'];
$answer = $_POST['answer'];
$test = $_POST['test'];
$enviar = $_POST['enviar'];
$result2 = $dbh->query("SELECT question, answer FROM captcha ORDER BY RAND() LIMIT 1");
if($row = $result2->fetch()) {
$question = $row["question"];
$answer = $row["answer"];
if ($test != $answer) {
echo "The answer of the security question is not correct";
}
}
}
11:30 am on Mar 10, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


Puf,
try to return true if valid,
any idea how to do this?
I want to return true (submit the form if it validates)
enviar = document.solicitud.enviar.value;
question = document.solicitud.question.value;
answer = document.solicitud.answer.value;
test = document.solicitud.test.value;
var ajaxcaptcha = objetoAjax();
ajaxcaptcha.open("POST", "/pasarela/captchaajax.php", true);
ajaxcaptcha.onreadystatechange = function () {
if (ajaxcaptcha.readyState == 4) {
if (ajaxcaptcha.responseText === "ok") {
return true;
} else {
divresult.innerHTML = "The answer to the security question was not correct"
<!-- LimpiarCampos();-->
}
}
}

ajaxcaptcha.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
ajaxcaptcha.send("test=" + test + "&enviar=" + enviar + "&answer=" + answer +
"&question=" + question + "");
return false;
}


I have simplified the php page as no need to do a query again and it works when I dont try to get a true valid, but I cant manage the ajax part:
if (isset($_POST['enviar'])){
$answer = $_POST['answer'];
$test = $_POST['test'];
$enviar = $_POST['enviar'];


if ($test = $answer) {
echo "ok";
}
}
7:05 pm on Mar 10, 2014 (gmt 0)

Senior Member from US 

WebmasterWorld Senior Member fotiman is a WebmasterWorld Top Contributor of All Time 10+ Year Member Top Contributors Of The Month

joined:Oct 17, 2005
posts: 5021
votes: 26



Tried to add the onchange function inside the other function in the first input but does not work:

You wouldn't want that defined within the other function, you'd want it to be a sibling function. Like this:

function OnSubmitBooking()
{
...
}
function quitarfocus()
{
...
}

Also, ideally you should not use inline event handlers. Instead, it's better to assign the event handlers from the JavaScript. So instead of this:
<input id="name" name="name" title="Please enter your name" onchange="quitarfocus()" type="text" size="50">

You could do something like this within your script:
var n = document.getElementById('name');
n.onchange = quitarfocus;

Even better, though, you might attach a single onchange event handler to a parent element of all your inputs, so you only need 1 handler total instead of 1 for each element. Your handler would then need to inspect the event to see which input field triggered it.
8:45 pm on Mar 10, 2014 (gmt 0)

Senior Member from US 

WebmasterWorld Senior Member fotiman is a WebmasterWorld Top Contributor of All Time 10+ Year Member Top Contributors Of The Month

joined:Oct 17, 2005
posts: 5021
votes: 26


AJAX requests are typically "Asynchronous". As such, you can't expect a synchronous method to be able to return the value from an AJAX request. Now, you *could* make the request synchronous by doing something like this:

ajaxcaptcha.open("POST", "/pasarela/captchaajax.php", false);
if (ajaxcaptcha.readyState == 4) {
if (ajaxcaptcha.responseText === "ok") {
return true;
} else {
divresult.innerHTML = "The answer to the security question was not correct"
<!-- LimpiarCampos();-->
}
}


In general, you should rarely if ever use synchronous requests.
8:47 pm on Mar 10, 2014 (gmt 0)

Senior Member from US 

WebmasterWorld Senior Member fotiman is a WebmasterWorld Top Contributor of All Time 10+ Year Member Top Contributors Of The Month

joined:Oct 17, 2005
posts: 5021
votes: 26


Alternatively, you could always return false from your onsubmit method, then in the AJAX request, call form.submit() if the request is successful.
8:55 pm on Mar 10, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


Alternatively, you could always return false from your onsubmit method, then in the AJAX request, call form.submit() if the request is successful.


This is been trying, been reeeding and if I understood correctly instead of true I declare false here:
ajaxcaptcha.open("POST", "captchaajax.php", false);
Is this correct?

Will try the onchange when I finished the Ajax return true
9:20 pm on Mar 10, 2014 (gmt 0)

Senior Member from US 

WebmasterWorld Senior Member fotiman is a WebmasterWorld Top Contributor of All Time 10+ Year Member Top Contributors Of The Month

joined:Oct 17, 2005
posts: 5021
votes: 26



This is been trying, been reeeding and if I understood correctly instead of true I declare false here:
ajaxcaptcha.open("POST", "captchaajax.php", false);

That makes it synchronous (my first example). But instead you could do something like this:

function OnSubmitBooking() {
...
ajaxcaptcha.open("POST", "captchaajax.php", false);
ajaxcaptcha.onreadystatechange = function () {
if (ajaxcaptcha.readyState == 4) {
if (ajaxcaptcha.responseText === "ok") {
document.Booking.submit();
} else {
divresult.innerHTML = "The answer to the security question was not correct"
<!-- LimpiarCampos();-->
}
}
};
return false; // always return false here to prevent default form submit
}


Though you may want to populate a hidden form with the values from the form the user sees and submit that instead. That way, if there is any time between your AJAX call and the response, any changes the user makes to the form could be ignored.
9:33 pm on Mar 10, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


document.Booking.submit();

This is ever seen, thanks hope that will do it.
Will let you know, late dinnertime ;)
9:36 pm on Mar 10, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


That makes it synchronous (my first example). But instead you could do something like this:

We answered at same time lol :)
9:39 pm on Mar 10, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


In general, you should rarely if ever use synchronous requests.


Then I should pass a big amount of vars by the Ajax doing it with php the normal way, redirection.
Was trying to avoid passing all the vars.
Better to be on the safe side
4:03 pm on Mar 11, 2014 (gmt 0)

Senior Member from US 

WebmasterWorld Senior Member fotiman is a WebmasterWorld Top Contributor of All Time 10+ Year Member Top Contributors Of The Month

joined:Oct 17, 2005
posts: 5021
votes: 26


Ooops, I meant this:

function OnSubmitBooking() {
...
ajaxcaptcha.open("POST", "captchaajax.php", true); // async
ajaxcaptcha.onreadystatechange = function () {
if (ajaxcaptcha.readyState == 4) {
if (ajaxcaptcha.responseText === "ok") {
document.Booking.submit();
} else {
divresult.innerHTML = "The answer to the security question was not correct"
<!-- LimpiarCampos();-->
}
}
};
return false; // always return false here to prevent default form submit
}
8:39 pm on Mar 11, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


Thanks, been struggling all day not able to understand why end did not work,.....wrong form name puf.
Anyway it works now, however not sure I can do it using this: document.Booking.submit();
That way it submits through Ajax and the form submits to itself and if the var in the submit button exists a php redirection is done.

Is it possible to send a var to php in this or some other way?
document.Booking.submit();

Looks like I will have to pass all vars by ajax, its a very long form so I tried to be smarter and only send the vars I needed to validate and let the rest for php to do.
9:08 pm on Mar 11, 2014 (gmt 0)

Senior Member from US 

WebmasterWorld Senior Member fotiman is a WebmasterWorld Top Contributor of All Time 10+ Year Member Top Contributors Of The Month

joined:Oct 17, 2005
posts: 5021
votes: 26


I'm not sure I understand what you're asking or what you're trying to achieve.
9:23 pm on Mar 11, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


Same as this:
ajaxcaptcha.send("test=" + test + "&enviar=" + enviar + "&answer=" + answer +
"&question=" + question + "");
however the contrary, send back to the page where the form is.
The form is executed using: document.Booking.submit();
however the form point to the page where the form is,
in order to do the php header redirection with its sessions I need to get at least the var enviar back.
I suppose as the form is not normally submitted as I do this: document.Booking.submit();
the var in the form input:
<input class="boton roundedcorner border" type="submit" name="enviar" value="Confirm booking and pay deposit now">
is being lost.
So the php on the page where the form is: if (isset($_POST['enviar'])){
is not executed.
10:50 pm on Mar 11, 2014 (gmt 0)

Senior Member from US 

WebmasterWorld Senior Member fotiman is a WebmasterWorld Top Contributor of All Time 10+ Year Member Top Contributors Of The Month

joined:Oct 17, 2005
posts: 5021
votes: 26


What if you just insert a hidden input with the name "enviar" into the form in your JavaScript? So before you call document.Booking.submit(), do something like this:

var enviar = document.createElement('input');
enviar.type = 'hidden';
enviar.name = 'enviar';
enviar.value = true;
document.Booking.appendChild(enviar);
document.Booking.submit();
9:07 am on Mar 12, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


What if you just insert a hidden input with the name "enviar" into the form in your JavaScript? So before you call document.Booking.submit(), do something like this:

That works perfect, thanks, maybe I could have found that in my friend google.
1:18 pm on Mar 12, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


Also, ideally you should not use inline event handlers. Instead, it's better to assign the event handlers from the JavaScript. So instead of this:
<input id="name" name="name" title="Please enter your name" onchange="quitarfocus()" type="text" size="50">

You could do something like this within your script:
var n = document.getElementById('name');
n.onchange = quitarfocus;

Even better, though, you might attach a single onchange event handler to a parent element of all your inputs, so you only need 1 handler total instead of 1 for each element. Your handler would then need to inspect the event to see which input field triggered it.

Thanks for all, on to next step, cleaning up the error messages, manage to get it work my way (at least the first).

This does not work:

var n = document.getElementById('repeat_email');
n.onchange = quitarfocus();
function quitarfocus()
{

document.getElementById("repeat_emailbox").style.display = "none";

}

function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
{


This does not work either:
 function quitarfocus()
{
var n = document.getElementById('repeat_email');
n.onchange = quitarfocus();
document.getElementById("repeat_emailbox").style.display = "none";

}

function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
{
document.getElementById('repeat_emailbox').style.display = "block";


However this works (at least in firefox):

<input id="repeat_email" onchange="quitarfocus()" name="repeat_email" title="Please repeat your email" size="61" type="text">
<div id="repeat_emailbox"></div>

function quitarfocus()
{

var n = document.getElementById('repeat_email');
if (n != '')
{
document.getElementById("repeat_emailbox").style.display = "none";
}
}

function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
{
document.getElementById('repeat_emailbox').style.display = "block";


This is what I have so fare:
function objetoAjax(){
var xmlhttp=false;
try {
xmlhttp = new ActiveXObject("Msxml2.XMLHTTP");
} catch (e) {

try {
xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
} catch (E) {
xmlhttp = false;
}
}

if (!xmlhttp && typeof XMLHttpRequest!='undefined') {
xmlhttp = new XMLHttpRequest();
}
return xmlhttp;
}
function quitarfocus()
{

var n = document.getElementById('repeat_email');
if (n != '')
{
document.getElementById("repeat_emailbox").style.display = "none";
}
}

function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
{
document.getElementById('repeat_emailbox').style.display = "block";
divresult = document.getElementById('repeat_emailbox');
divresult.className = 'validation'
divresult.innerHTML = 'Email and repeat email are not equal';
document.Booking.repeat_email.focus();
return false;
}
var info = {
names : ['namebox' , 'Please fill in name' ],
passport : ['passportbox' , 'Please fill in your identification' ],
emailtrue : ['emailtruebox' , 'Please fill in email' ],
repeat_email: ['repeat_emailbox', 'Please repeat email' ],
telmobile : ['telmobilebox' , 'Please fill in mobile to bring on holiday' ],
test : ['resultbooking' , 'Please answer the security question' ]
}
for( key in info ) {
if(document.Booking[key].value == '')
{
divresult = document.getElementById(info[key][0])
divresult.className = 'validation'
divresult.innerHTML = info[key][1];
document.Booking[key].focus();
return false;
}
}
divresult = document.getElementById('resultbooking');

var enviar = document.Booking.enviar.value,
question = document.Booking.question.value,
answer = document.Booking.answer.value,
test = document.Booking.test.value;
var ajaxcaptcha = objetoAjax();
ajaxcaptcha.open("POST", "captchaajax.php", true);
ajaxcaptcha.onreadystatechange = function () {
if (ajaxcaptcha.readyState==1 || ajaxcaptcha.readyState==2 || ajaxcaptcha.readyState==3)
{
divresult.className = 'validationok'
divresult.innerHTML = 'Please wait...';
}
if (ajaxcaptcha.readyState == 4) {
if (ajaxcaptcha.responseText === "Loading, please wait") {
var enviar = document.createElement('input');
enviar.type = 'hidden';
enviar.name = 'enviar';
enviar.value = true;
document.Booking.appendChild(enviar);
document.Booking.submit();
} else {
divresult.className = 'validation'
divresult.innerHTML = ajaxcaptcha.responseText
}
}
}

ajaxcaptcha.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
ajaxcaptcha.send("test=" + test + "&enviar=" + enviar + "&answer=" + answer +
"&question=" + question + "");
return false;
}
10:36 pm on Mar 13, 2014 (gmt 0)

Senior Member from US 

WebmasterWorld Senior Member fotiman is a WebmasterWorld Top Contributor of All Time 10+ Year Member Top Contributors Of The Month

joined:Oct 17, 2005
posts: 5021
votes: 26



n.onchange = quitarfocus();

You don't want to include the parenthesis at the end. Otherwise you're executing quitarfocus() immediately and assigning the return value to n.onchange instead of assigning the function itself to onchange. So try this instead:
n.onchange = quitarfocus;
8:28 am on Mar 14, 2014 (gmt 0)

Senior Member

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month

joined:Nov 27, 2003
posts: 1318
votes: 0


n.onchange = quitarfocus;

Yes I managed to get it to work with one of the vars.
However I just decided to leave it, and just change the css so it looks ok.
Thanks for everything I learnt a lot.
 

Join The Conversation

Moderators and Top Contributors

Hot Threads This Week

Featured Threads

Free SEO Tools

Hire Expert Members