6
\$\begingroup\$

I've got an assignment to do pure JS validation as well as submit with AJAX. Here is the code I've got so far. I'm wondering if I can do away with the whole "reason" bit. That was because it was originally meant to display a list of all the errors, but I've tweaked the code to instead display the errors right by the field.

Basically I want to call up the validateFormOnSubmit and just have it go through all the if statements instead of all the separate functions, then if it all passes execute submitFormAjax.

The AJAX is currently not working but don't want to troubleshoot that until I've cleaned up the JS code.

JSfiddle

HTML

<form id="contact" name="contact" onsubmit="return validateFormOnSubmit(this)" action="" method="GET">
    <div>
        <label>First Name</label>
        <input placeholder="First Name" type="text" name="name" id="name" tabindex="1" autofocus />
        <div id="name-error" class="error"></div>
    </div>
    <div>
        <label>Nickname</label>
        <input placeholder="Nickname" type="text" name="nickname" id="nickname" tabindex="2" autofocus />
    </div>
    <div>
        <label>Email</label>
        <input placeholder="Email" type="email" name="email" id="email" tabindex="3" autofocus />
        <div id="email-error" class="error"></div>
    </div>
    <div>
        <label>Phone</label>
        <input placeholder="Phone" type="tel" name="phone" id="phone" tabindex="4" autofocus />
        <div id="phone-error" class="error"></div>
    </div>
    <div>
        <label>I prefer</label>
        <input type="radio" name="pet" id="Dogs" tabindex="5" autofocus />Dogs
        <input type="radio" name="pet" id="Cats" tabindex="6" autofocus />Cats
        <input type="radio" name="pet" id="Neither" tabindex="7" autofocus />Neither
        <div id="pet-error" class="error"></div>
    </div>
    <div>
        <label>My favorite number between 1 and 50</label>
        <input placeholder="Favorite number between 1 and 50" type="text" name="number" id="number" tabindex="8" autofocus />
        <div id="number-error" class="error"></div>
    </div>
    <div>
        <label>Disclaimer</label>
        <input type="checkbox" name="disclaimer" id="disclaimer" tabindex="9" autofocus />I confirm that all the above information is true.
        <div id="disclaimer-error" class="error"></div>
    </div>
    <div>
        <button type="submit" name="submit" id="submit" tabindex="10">Send</button>
    </div>
</form>

JS

function validateFormOnSubmit(contact) {
    reason = "";
    reason += validateName(contact.name);
    reason += validateEmail(contact.email);
    reason += validatePhone(contact.phone);
    reason += validatePet(contact.pet);
    reason += validateNumber(contact.number);
    reason += validateDisclaimer(contact.disclaimer);

    console.log(reason);
    if (reason.length > 0) {

        return false;
    } else {
        // Show some loading image and submit form
        submitFormAjax();
    }
}

// validate required fields
function validateName(name) {
    var error = "";

    if (name.value.length == 0) {
        name.style.background = 'Red';
        document.getElementById('name-error').innerHTML = "The required field has not been filled in";
        var error = "1";
    } else {
        name.style.background = 'White';
        document.getElementById('name-error').innerHTML = '';
    }
    return error;
}

// validate email as required field and format
function trim(s) {
    return s.replace(/^\s+|\s+$/, '');
}

function validateEmail(email) {
    var error = "";
    var temail = trim(email.value); // value of field with whitespace trimmed off
    var emailFilter = /^[^@]+@[^@.]+\.[^@]*\w\w$/;
    var illegalChars = /[\(\)\<\>\,\;\:\\\"\[\]]/;

    if (email.value == "") {
        email.style.background = 'Red';
        document.getElementById('email-error').innerHTML = "Please enter an email address.";
        var error = "2";
    } else if (!emailFilter.test(temail)) { //test email for illegal characters
        email.style.background = 'Red';
        document.getElementById('email-error').innerHTML = "Please enter a valid email address.";
        var error = "3";
    } else if (email.value.match(illegalChars)) {
        email.style.background = 'Red';
        var error = "4";
        document.getElementById('email-error').innerHTML = "Email contains invalid characters.";
    } else {
        email.style.background = 'White';
        document.getElementById('email-error').innerHTML = '';
    }
    return error;
}

// validate phone for required and format
function validatePhone(phone) {
    var error = "";
    var stripped = phone.value.replace(/[\(\)\.\-\ ]/g, '');

    if (phone.value == "") {
        document.getElementById('phone-error').innerHTML = "Please enter a phone number";
        phone.style.background = 'Red';
        var error = '6';
    } else if (isNaN(parseInt(stripped))) {
        var error = "5";
        document.getElementById('phone-error').innerHTML = "The phone number contains illegal characters.";
        phone.style.background = 'Red';
    } else if (stripped.length < 10) {
        var error = "6";
        document.getElementById('phone-error').innerHTML = "The phone number is too short.";
        phone.style.background = 'Red';
    } else {
        phone.style.background = 'White';
        document.getElementById('phone-error').innerHTML = '';
    }
    return error;
}

function validatePet(pet) {
    if ((contact.pet[0].checked == false) && (contact.pet[1].checked == false) && (contact.pet[2].checked == false)) {
        document.getElementById('pet-error').innerHTML = "Pet required";
        var error = "2";
    } else {
        document.getElementById('pet-error').innerHTML = '';
    }
    return error;
}

function validateNumber(number) {
    var num = document.forms["contact"]["number"];
    var y = num.value;
    if (!isNaN(y)) {

        //alert('va');

        if (y < 0 || y > 50) {
            //Wrong
            number.style.background = 'Red';
            document.getElementById("number-error").innerHTML = "Must be between 0 and 50.";
            var error = "10";
        } else {
            //Correct
            number.style.background = 'White';
            document.getElementById("number-error").innerHTML = "";
        }
        return error;
    } else {
        document.getElementById("number-error").innerHTML = "Must be a number.";
        var error = "3";
    }
    return error;
}

function validateDisclaimer(disclaimer) {
    var error = "";

    if (document.getElementById("disclaimer").checked === false) {
        document.getElementById('disclaimer-error').innerHTML = "Required";
        var error = "4";
    } else {
        document.getElementById('disclaimer-error').innerHTML = '';
    }
    return error;
}

function submitFormAjax() {
    var xmlhttp= window.XMLHttpRequest ?
        new XMLHttpRequest() : new ActiveXObject("Microsoft.XMLHTTP");

    xmlhttp.onreadystatechange = function() {
        if (xmlhttp.readyState == 4 && xmlhttp.status == 200)
            alert("Test Ready State"); // Here is the response
    }

    var name = document.getElementById('name').innerHTML;
    var nickname = document.getElementById('nickname').innerHTML;
    var email = document.getElementById('email').innerHTML;
    var phone = document.getElementById('phone').innerHTML;
    var pet = document.getElementById('pet').innerHTML;
    var number = document.getElementById('number').innerHTML;
    var disclaimer = document.getElementById('disclaimer').innerHTML;

    xmlhttp.open("GET","form.php?name=" + name + "&nickname" + nickname + "&email=" + email, + "&phone" + phone + "&pet" + pet + "&number" + number + "&disclaimer" + disclaimer, true);
    xmlhttp.send();
}

PHP

<?php
$name = $_GET['name'];
$nickname = $_GET['nickname'];
$email = $_GET['email'];
$phone = $_GET['phone'];
$pet = $_GET['pet'];
$number = $_GET['number'];
$disclaimer = $_GET['disclaimer'];
$from = 'From: Test From'; 
$to = 'euteneier@gmail.com'; 
$subject = 'Hello';
$message = "This is a message.";

if ($_GET['submit']) {               
    if (mail ($to, $subject, $message)) { 
        echo '<p>Your message has been sent!</p>';
} else { 
    echo '<p>Something went wrong, go back and try again!</p>'; 
} 
}
?>
\$\endgroup\$
2
  • 1
    \$\begingroup\$ If you put 0 in the number field, it does not validate. The if statement says "y < 0 || y > 50". \$\endgroup\$
    – Gonz
    Feb 19, 2014 at 19:15
  • \$\begingroup\$ Good catch @GonzaloNaveira. Thanks. I upvoted the comment. \$\endgroup\$
    – justinae
    Feb 19, 2014 at 19:42

3 Answers 3

4
\$\begingroup\$

From a once over:

  • You are repeating .style.background = 'Red'; and .style.background = 'White'; a ton, what if suddenly all these fields also have to get bold. I would suggest a function like highlightField that does the setting of the style:

    function setHighlight( element , isHighlighted )
    {
      element.style.background = isHighlighted ? 'Red' : 'White';
    }
    
  • The reason being a number is silly, since you do not do anything with the number, I would simply return booleans and AND these booleans.
  • It is considered a better style to have one single var statements, with comma separated variables.
  • Then, you could change this:

    function validateEmail(email) {
        var error = "";
        var temail = trim(email.value); // value of field with whitespace trimmed off
        var emailFilter = /^[^@]+@[^@.]+\.[^@]*\w\w$/;
        var illegalChars = /[\(\)\<\>\,\;\:\\\"\[\]]/;
    
        if (email.value == "") {
            email.style.background = 'Red';
            document.getElementById('email-error').innerHTML = "Please enter an email address.";
            var error = "2";
        } else if (!emailFilter.test(temail)) { //test email for illegal characters
            email.style.background = 'Red';
            document.getElementById('email-error').innerHTML = "Please enter a valid email address.";
            var error = "3";
        } else if (email.value.match(illegalChars)) {
            email.style.background = 'Red';
            var error = "4";
            document.getElementById('email-error').innerHTML = "Email contains invalid characters.";
        } else {
            email.style.background = 'White';
            document.getElementById('email-error').innerHTML = '';
        }
        return error;
    }
    

    to

    function validateEmail(email) {
        var emailFilter  = /^[^@]+@[^@.]+\.[^@]*\w\w$/,
            illegalChars = /[\(\)\<\>\,\;\:\\\"\[\]]/,
            errorMessage = '',
            isValid;
    
        //Do we have an error ?
        if (email.value == "") {
            errorMessage = "Please enter an email address.";
        } else if (!emailFilter.test(trim(email.value))) { //test for illegal chars
            errorMessage = "Please enter a valid email address.";
        } else if (email.value.match(illegalChars)) {
            errorMessage = "Email contains invalid characters.";
        }
    
        document.getElementById('email-error').innerHTML = errorMessage;
        isValid = !errorMessage.length;
        setHighlight( email , !isValid );
        return isValid;
    }
    

    You could do this for every validation rule.

  • You must declare all your variables with var so that you do not pollute the global namespace ( I am looking at you, reason = "" )
  • If all validation functions will return a boolean, than you could rename reason to isValid:

    var isValid = validateName(contact.name)     &&
                  validateEmail(contact.email)   &&
                  validatePhone(contact.phone)   &&
                  validatePet(contact.pet)       &&
                  validateNumber(contact.number) &&
                  validateDisclaimer(contact.disclaimer);
    

    obviously the && does not have to be aligned, but it makes my heart beat faster :)

  • There would still be the concern that the name validateXxx lies a little because it also returns a boolean, but since it also updates the UI and does the validation it would be hard to name this correctly without having a really long name.
\$\endgroup\$
3
  • \$\begingroup\$ so basically the email functions returns "false" if any of those conditions are met. Where is it returning "true" or valid? \$\endgroup\$
    – justinae
    Feb 20, 2014 at 0:01
  • \$\begingroup\$ || is a short-circuit operator. If the name is false then it will check the email, if false, then the phone, if false, then the pet, etc... \$\endgroup\$
    – elclanrs
    Feb 20, 2014 at 0:07
  • \$\begingroup\$ @elclanrs Actually I needed more coffee when I wrote that, it should be && since validateXxx returns true if the value is valid. Fixed it now. \$\endgroup\$
    – konijn
    Feb 20, 2014 at 13:57
2
\$\begingroup\$

If you don't have a strong reason for using the onsubmit attribute on your <form> element, I would suggest attaching the submit action to the form without it. This would work:

document.getElementById('contact').onsubmit(function () { return validateFormOnSubmit(this); });

Other than that, it might not be a good idea to send the user's email in plain text. You can have an AJAX request use the POST method as well as GET, which would improve (slightly) the security of personal information.

\$\endgroup\$
2
  • \$\begingroup\$ This doesn't look right, I think you missed an anonymous function there. \$\endgroup\$
    – elclanrs
    Feb 20, 2014 at 0:00
  • \$\begingroup\$ Whoops! Good catch. \$\endgroup\$ Feb 20, 2014 at 0:12
1
\$\begingroup\$

For your JS email validation, I'd consider taking a look at this regular expression.

Also, I'd make sure you have a bit more security on that GET to form.php. That looks like an invitation for spammers!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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