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();
}
}
}
}
}
});
-
\$\begingroup\$ You should probably have a look at stackoverflow.com/questions/201323/… \$\endgroup\$SylvainD– SylvainD2014年02月12日 14:04:03 +00:00Commented 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\$Amir– Amir2014年02月12日 14:26:40 +00:00Commented Feb 12, 2014 at 14:26
1 Answer 1
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)
.
-
\$\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\$Amir– Amir2014年02月12日 14:47:02 +00:00Commented Feb 12, 2014 at 14:47
-
\$\begingroup\$ @Amir Not needed indeed. \$\endgroup\$konijn– konijn2014年02月12日 14:48:37 +00:00Commented Feb 12, 2014 at 14:48