6
\$\begingroup\$

I've written this as part of my contact form on my website. It checks to see if required inputs are empty, if so adds a red warning color and text. It also checks the email field for a properly formatted email.

This code works 100%, but somehow I feel like I'm repeating myself and I am not sure how to eliminate the repetition. Most importantly I am not sure how to reference nested "this". What I mean is if my input is jQuery (this) and then I use traversing to get to its parent div.

How can I reference that DIV over and over again, without having to traverse over and over again?

//Check current language and prepare warning text
var current_language = jQuery('html').attr('lang');
if (current_language == 'en-CA') {
 var empty_field_message = 'This field is required.';
 var invalid_email = 'This email address seems invalid.';
}
else {
 var empty_field_message = 'Ce champ est requis.';
 var invalid_email = 'L\'adresse email semble invalide.';
}
//Email validation script
function validateEmail($email) {
 var emailReg = /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/;
 if( !emailReg.test( $email ) ) {
 return false;
 } else {
 return true;
 }
}
//Fire on all input elements of the form
jQuery( ':input' ).on( 'focus', function() {
 //Add a highlight color to div containing the currently active form element
 jQuery(this).closest( '.frm-holder' ).addClass( 'frm-field-focus' );
}).on('blur', function() {
 //Remove the highlight once user moves off the input
 jQuery(this).closest( '.frm-holder' ).removeClass( 'frm-field-focus' );
 //Check if field was required
 if ( jQuery(this).hasClass('wpcf7-validates-as-required') ) {
 //Check if it was empty
 if ( !jQuery.trim(jQuery(this).val()) ) {
 //Add a red background to the wrapping DIV
 jQuery(this).closest( '.frm-holder' ).addClass( 'frm-field-required' );
 //Remove any other warning spans and put in empty field warning
 jQuery(this).siblings( 'span.wpcf7-not-valid-tip' ).remove();
 jQuery(this).after('<span class="wpcf7-not-valid-tip" role="alert">' + empty_field_message + '</span>');
 }
 //If it wasn't empty
 else {
 //Check if it's an email field
 if ( jQuery(this).attr('type') == 'email') {
 //If email is invalid
 if( !validateEmail( jQuery(this).val() ) ) { 
 //Remove any other warning spans
 jQuery(this).siblings( 'span.wpcf7-not-valid-tip' ).remove();
 //Check if wrapping DIV does not have red background
 if ( !jQuery(this).closest( '.frm-holder' ).hasClass( 'frm-field-required' ) ) {
 //Add wrapping DIV red background
 jQuery(this).closest( '.frm-holder' ).addClass( 'frm-field-required' );
 }
 //Add new warning span
 jQuery(this).after('<span class="wpcf7-not-valid-tip" role="alert">' + invalid_email + '</span>');
 }
 //If email is valid
 else {
 //Remove wrapping DIV red background
 jQuery(this).closest( '.frm-holder' ).removeClass( 'frm-field-required' );
 //Remove warning span
 jQuery(this).siblings( 'span.wpcf7-not-valid-tip' ).remove();
 }
 }
 //If not email field
 else {
 //Check if wrapping DIV has red background
 if ( jQuery(this).closest( '.frm-holder' ).hasClass( 'frm-field-required' ) ) {
 //Remove wrapping DIV red background
 jQuery(this).closest( '.frm-holder' ).removeClass( 'frm-field-required' );
 //Check if warning span exists
 if ( jQuery(this).siblings( jQuery('span.wpcf7-not-valid-tip').length ) ) {
 //Remove warning span
 jQuery(this).siblings( 'span.wpcf7-not-valid-tip' ).remove();
 }
 }
 }
 }
 }
});
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Feb 12, 2014 at 13:40
\$\endgroup\$
2
  • \$\begingroup\$ You should probably have a look at stackoverflow.com/questions/201323/… \$\endgroup\$ Commented Feb 12, 2014 at 14:04
  • \$\begingroup\$ @Josay thank you for that link. After reading through it, there still doesn't seem to be 100% agreement on what's best to implemennt. Seems like every solution has some upside and downsides. I decided to change my code to this, what do you think? \$\endgroup\$ Commented Feb 12, 2014 at 14:26

1 Answer 1

5
\$\begingroup\$

From a once over:

I would do the languages like this:

var translations = {
 'en-CA' : {
 emptyFieldMessage: 'This field is required.',
 invalidEmail : 'This email address seems invalid.'
 },
 'fr-CA' : {
 emptyFieldMessage: 'Ce champ est requis.',
 invalidEmail : 'L\'adresse email semble invalide.' 
 }
}
var messages = translations[ jQuery('html').attr('lang') ];

Then you can access emptyFieldMessage as messages.emptyFieldMessage

This:

if( !emailReg.test( $email ) ) {
 return false;
} else {
 return true;
}

can be this because you are evaluating a boolean to return a boolean:

return emailReg.test( $email );

The //If not email field else block can be reduced to:

//If not email field
else {
 //Remove wrapping DIV red background
 jQuery(this).closest( '.frm-holder' ).removeClass( 'frm-field-required' );
 //Remove warning span
 jQuery(this).siblings( 'span.wpcf7-not-valid-tip' ).remove();
}

if you are going to remove a class, dont check for it's existence, just call removeClass, it will silently do nothing if the class was not there in the first place. The same goes for remove(), just call it, dont check prior.

This is most likely wrong as per Josay:

var emailReg = /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/;

Very important, create a var $this = jQuery(this); instead of calling over and over again jQuery(this).

answered Feb 12, 2014 at 14:22
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for your suggestions, I've updated my code. I wasn't aware that removeClass and remove won't do any damage if just fired. That cleaned up a lot of the code. What about addClass, I have one more check in my code to make sure it doesn't keep adding the same class over and over again. Is that necessary? \$\endgroup\$ Commented Feb 12, 2014 at 14:47
  • \$\begingroup\$ @Amir Not needed indeed. \$\endgroup\$ Commented Feb 12, 2014 at 14:48

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.