1
\$\begingroup\$

Recently I have been doing a bit of work with Contact Form 7 and Wordpress. I've begun looking at form validation using jQuery and I've been trying to write a script that is as efficient as possible.

I'm wondering if someone could take a look at my code here: https://jsfiddle.net/javacadabra/6mpo9h9z/4/ and let me know if I'm going about things the right way or if what I am doing is messy and bad practice.

The form operates like this at the moment:

  • If an input fails the regEx check it adds a class called error
  • If an input passes the regEx check it adds a class called success
  • If an input is empty when the form is submitted it adds a class called empty

The form will only submit if the error class is not on any input fields and if at least one checkbox is selected. This is my submission code:

$('#get-started').submit(function(e) {
  var isValid = false;
  //Check at least on option is selected
  var checked = $('input:checked').length > 0;

  //Loop through all text fields
  $("input[type='text']").each(function() {
    if ($(this).hasClass('error') || $(this).val() == "") {
      if ($(this).val() == "") {
        empty($(this));
        isValid = false;
      }
      isValid = false;
    } else {
      isValid = true;
    }
  });

  //If everything is valid submit && checked is true
  if (isValid && checked) {
    return false; //for testing;
  } else { //else show error
    return false;
  }
});

This is my regEx:

function nameCheck(name) { //Checks if a name contains letters only
  var condition = new RegExp(/^[A-Za-z -]+$/);
  return condition.test(name);
}

Whenever a user clicks out of an input field I check to ensure the input is valid like so:

$('input[name="first-name"]').focusout(function() {
  if ($(this).val() != "") { //Only do if user has input text
    var isOk = nameCheck($(this).val());
    if (!isOk) { // Incorrect 
      incorrect($(this));
    } else { // Correct
      correct($(this));
    }
  }
});

I have 3 methods which will set the input according to the state (correct, incorrect, empty)

//Correct input
function correct(input) {
  input.parent().find('.hint').hide();
  input.parent().find('.fa-times').hide();
  input.parent().find('.fa-check').show();
  input.parent().find('.fa-exclamation').hide();
  input.removeClass('error');
  input.addClass('success');
  input.removeClass('empty');
}
//Incorrect input
function incorrect(input) {
  input.parent().find('.hint').show();
  input.parent().find('.fa-times').show();
  input.parent().find('.fa-check').hide();
  input.parent().find('.fa-exclamation').hide();
  input.addClass('error');
  input.removeClass('success');
  input.removeClass('empty');
}

//Empty input
function empty(input) {
  input.parent().find('.hint').show();
  input.parent().find('.fa-exclamation').show();
  input.parent().find('.fa-check').hide();
  input.parent().find('.fa-times').hide();
  input.addClass('empty');
  input.removeClass('error');
  input.removeClass('success');
}

Basically I'd like to continue with this pattern and add more fields (some with slightly different checks on the input). However before I do I just want to be sure that I'm writing clean, maintainable code. Once I am happy with my client-side validation I am going to then implement similar checks on the server-side with PHP.

Please feel free to offer your suggestions on the above. Also this post may be helpful for anyone who is looking to implement their own custom form validation with jQuery.

Also, this is my HTML:

<div id='application-form' class='com col-md-8 form-horizontal'>
  <form id='get-started' action='#' method="post">

    <!-- First and Last Name -->
    <div class="form-group">
      <div class="col-md-6">
        <label>First Name <span class='hint'>(Name can only contain letters)</span></label>
        <input type="text" name="first-name" id="first-name"><i class="fa fa-times"></i><i class="fa fa-check"></i><i class="fa fa-exclamation"></i>
      </div>
      <div class="col-md-6">
        <label>Last Name <span class='hint'>(Name can only contain letters)</span></label>
        <input type="text" name="last-name" id="last-name"><i class="fa fa-times"></i><i class="fa fa-check"></i><i class="fa fa-exclamation"></i>
      </div>
    </div>

    <!-- Options -->
    <div class='form-group'>
      <div class='col-md-12'>
        <label>Choose an option: <span>(You must choose at least one option)</span></label>
        <input type="checkbox" name="option-group[]" value="option 1" /> Option 1
        <input type="checkbox" name="option-group[]" value="option 2" /> Option 2
        <input type="checkbox" name="option-group[]" value="option 3" /> Option 3
      </div>
    </div>

    <!-- Submit -->
    <div class="form-group">
      <div class="col-md-12">
        <input type='submit' name='submit' class='apply btn btn-primary' value="Get Started" />
      </div>
    </div>
  </form>
</div>
\$\endgroup\$
  • \$\begingroup\$ Why would you want to do custom jQuery validation, is there a specific application reason? from what I see here you can customize HTML5 validation without as much hassle. \$\endgroup\$ – Blue Eyed Behemoth Apr 7 '16 at 15:56
  • \$\begingroup\$ Specific application reason \$\endgroup\$ – Javacadabra Apr 8 '16 at 9:13
1
\$\begingroup\$

jQuery lets you select multiple elements with a ,. So you can shorten this quite a bit by doing:

$('input[name="first-name"], input[name="last-name"]').focusout(...

because the code within those focusout handlers is exactly the same.

Another place you could do that:

input.parent().find('.hint, .fa-times').hide();

Also, although it doesn't make much of a difference here, it would be faster (and easier for you) to select the first-name and last-name by their ID:

$('#first-name, #last-name').focusout(...

You might as well, because it's shorter, quicker and easier - you have the ID on the element, so you might as well use it!


jQuery lets you string functions together. When you use addClass(), jQuery returns the element that you added the class to, so you can do:

input.addClass('error').removeClass('success');

 if ($(this).hasClass('error') || $(this).val() == "") {
   isValid = false;
 } else {
   isValid = true;
 }

There's no need to set isValid as false, because you declared it like var isValid = false;, so it's already false! Also, if isValid is false, there's no need to keep on looping through the inputs, because it's going to be false anyway, so you should escape the loop early if is false

Instead, you could do something like:

if ($(this).hasClass('error') || $(this).val() == "") {
   return false;
} else {
  isValid = true;
}

In your correct() and incorrect() functions, you do input.parent() twice. Ideally, you should store this in a variable first, to avoid jQuery looking for the parent twice, eg:

function correct(input) {
  var $parent = input.parent();
  $parent.find('.hint, .fa-times').hide();
  $parent.find('.fa-check').show();
  input.removeClass('error').addClass('success');
}

With all this in mind, your revised code would be something like: (I also moved the functions to the top of the page, but that's just personal preference and makes it easier to see all your functions at once)

function nameCheck(name) { //Checks if a name contains letters only
  var condition = new RegExp(/^[A-Za-z -]+$/);
  return condition.test(name);
}

function correct(input) { //correct input
  var $parent = input.parent();
  $parent.find('.hint, .fa-times').hide();
  $parent.find('.fa-check').show();
  input.removeClass('error').addClass('success');
}

function incorrect(input) { //incorrect input
  var $parent = input.parent();
  $parent.find('.hint, .fa-times').show();
  $parent.find('.fa-check').hide();
  input.addClass('error').removeClass('success');
}

//Check first-name and last-name are valid every time user types a character into the input field
$('#first-name, #last-name').focusout(function() {
  if ($(this).val() != "") { //Only do if user has input text
    var isOk = nameCheck($(this).val());
    if (!isOk) { // Incorrect 
      incorrect($(this));
    } else { // Correct
      correct($(this));
    }
  }
});

//Check everything is valid before the form submits
$('#get-started').submit(function(e) {
  var isValid = false;
  //Check at least on option is selected
  var checked = $('input:checked').length > 0;

  //Loop through all text fields
  $("input[type='text']").each(function() {
    if ($(this).hasClass('error') || $(this).val() == "") {
      return false;
    } else {
      isValid = true;
    }
  });

  //If everything is valid submit
  if (isValid && checked) {
    alert('submitting...')
    return false; //for testing;
  } else { //else show error
    alert('do not submit');
    return false;
  }
});
#application-form label {
  display: block;
}
#application-form label span {
  font-weight: 100;
  font-size: 12px;
}
#application-form input[type='text'] {
  width: 100%;
  position: relative;
  padding: 10px 10px;
}
#application-form i.fa {
  position: absolute;
  right: 25px;
  bottom: 11px;
  font-size: 22px;
}
#application-form i.fa-check {
  color: green;
  display: none;
}
#application-form i.fa-times {
  color: red;
  display: none;
}
#application-form .hint {
  display: none;
}
#application-form .success {
  background: rgba(0, 128, 0, 0.33);
  border: 1px solid green;
}
#application-form .error {
  background: rgba(255, 0, 0, 0.33);
  border: 1px solid red;
}
#application-form input[type="submit"] {
  text-transform: uppercase;
  width: 200px;
  border: none;
  padding: 12px 0px;
  float: right;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/js/bootstrap.min.js"></script>
<link rel="stylesheet" type="text/css" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css">
<link rel="stylesheet" type="text/css" href="https://maxcdn.bootstrapcdn.com/font-awesome/4.5.0/css/font-awesome.min.css">

<div id='application-form' class='com col-md-8 form-horizontal'>
  <form id='get-started' action='#' method="post">
    <!-- First and Last Name -->
    <div class="form-group">
      <div class="col-md-6">
        <label>First Name <span class='hint'>(Name can only contain letters)</span>
        </label>
        <input type="text" name="first-name" id="first-name"><i class="fa fa-times"></i><i class="fa fa-check"></i>
      </div>
      <div class="col-md-6">
        <label>Last Name <span class='hint'>(Name can only contain letters)</span>
        </label>
        <input type="text" name="last-name" id="last-name"><i class="fa fa-times"></i><i class="fa fa-check"></i>
      </div>
    </div>

    <!-- Options -->
    <div class='form-group'>
      <div class='col-md-12'>
        <label>Choose an option: <span>(You must choose at least one option)</span>
        </label>
        <input type="checkbox" name="option-group[]" value="option 1" />Option 1
        <input type="checkbox" name="option-group[]" value="option 2" />Option 2
        <input type="checkbox" name="option-group[]" value="option 3" />Option 3
      </div>
    </div>

    <!-- Submit -->
    <div class="form-group">
      <div class="col-md-12">
        <input type='submit' name='submit' class='apply btn btn-primary' value="Get Started" />
      </div>
    </div>
  </form>
</div>

\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks so much for your feedback, very helpful and informative!!! Greatly appreciated \$\endgroup\$ – Javacadabra Apr 8 '16 at 9:12

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.