patternjavascriptMinor
Checking input and email field
Viewed 0 times
fieldemailcheckinginputand
Problem
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
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
Solution
From a once over:
I would do the languages like this:
Then you can access
This:
can be this because you are evaluating a boolean to return a boolean:
The
if you are going to remove a class, dont check for it's existence, just call
This is most likely wrong as per Josay:
Very important, create a
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.emptyFieldMessageThis:
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).Code Snippets
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') ];if( !emailReg.test( $email ) ) {
return false;
} else {
return true;
}return emailReg.test( $email );//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();
}var emailReg = /^([\w-\.]+@([\w-]+\.)+[\w-]{2,4})?$/;Context
StackExchange Code Review Q#41476, answer score: 5
Revisions (0)
No revisions yet.