3
\$\begingroup\$

In my HTML, I have a large form (id ="myform") that asks users to fill in 5 fields for Date of Birth (DOB), Sex, Weight, Hair Color and Eye Color and then several other fields which don't relate to this question. Each of those 5 category fields start with a question mark in it. When the users enters something, the question mark goes away. So my JS (which you'll see in a few seconds) looks for a question mark. If it's there, user missed the field. If it's not, the user entered something.

Next to each field is code such as this example for Date of Birth (this is set to display:none by default). It's just the word "Required" in red:

<label id='dob-display-error' class='error' for='dob-display'>Required</label>

I am validating each field using this JavaScript:

$('#myform').submit(function(){
 //check to make sure all categories have been guessed 
 var $dobdisplaytext=$('#dob-display').text();
 var $sexdisplaytext=$('#sex-display').text();
 var $weightdisplaytext=$('#weight-display').text();
 var $hairdisplaytext=$('#hair-display').text();
 var $eyedisplaytext=$('#eye-display').text();
 if ( $dobdisplaytext.indexOf('?') !== -1 ) {
 $('#dob-display-error').show();
 $('#dob').addClass('error-border');
 $('html,body').animate({scrollTop:0},0);
 return false;
 } 
 if ( $sexdisplaytext.indexOf('?') !== -1 ) {
 $('#sex-display-error').show();
 $('#sex').addClass('error-border');
 $('html,body').animate({scrollTop:0},0);
 return false;
 } 
 if ( $weightdisplaytext.indexOf('?') !== -1 ) {
 $('#weight-display-error').show();
 $('#weight').addClass('error-border');
 $('html,body').animate({scrollTop:0},0);
 return false;
 } 
 if ( $hairdisplaytext.indexOf('?') !== -1 ) {
 $('#hair-display-error').show();
 $('#hair').addClass('error-border');
 $('html,body').animate({scrollTop:0},0);
 return false;
 } 
 if ( $eyedisplaytext.indexOf('?') !== -1 ) {
 $('#eye-display-error').show();
 $('#eye').addClass('error-border');
 $('html,body').animate({scrollTop:0},0);
 return false;
 } 
});

Quick rundown of what each thing's purpose is:

$('#dob-display-error').show();

This sets the "Required" error to display:block to alert the user there's an issue

$('#dob').addClass('error-border');

This simply adds a class which puts a red border around the dob field, to further visually alert the user to an issue

$('html,body').animate({scrollTop:0},0);

The form is very long and these fields are toward the top. So when the user is at the bottom of the form and clicks submit, if there's an issue, this sends the user up to the top of the page so they can see the visual alerts.

This code works, however, here are a few things I'd like help with:

  1. I am not even close to a JS expert as I'm sure you can all tell, but there has to be a more efficient way to write this, right? Like in a loop of some sort? With the key being that if four fields are missed, I'd like all four fields to show their respective alerts.

  2. I believe this will be answered by being written better and I alluded to it in #1 but just to be clear and give more details. If I remove return false; from each one, ALL missed categories show the required error/border at once (which I prefer) but of course the form then immediately submits because I have nothing stopping it.

The way it is above, if dob was missed, ONLY the dob error displays (even if they missed others). So that's not a good UX. For example, if the user missed the dob AND sex fields, they'd click "Submit" and get an alert about dob and would choose a dob, then scroll all the way to bottom and click "Submit" again and it would send them back up to fill in Sex. Again, not a good UX.

So I know return false; has got to go SOMEWHERE (and likely only once, once the code is written more efficiently), just not sure where.

200_success
146k22 gold badges190 silver badges479 bronze badges
asked May 2, 2017 at 1:31
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

One thing that pops up in my mind when I see code containing repetitions of almost exact copies of structure is how can we achieve reusability?

DRY principle is a very powerful thing that helps identifying and fixing code issues, until gets to the point where the other principles are abused.


This can be solved with code like one below.

  1. First we declare fields to iterate on.
  2. Clear validation error indicators set on previous runs.
  3. filter the fields which fail validation. I am not sure what is the '?' that original code relies on, but I think you know better ;)
  4. If there list of filtered fields is empty, we know there are no validation errors and can return from here.
  5. Otherwise animate to the top of the screen, display UI errors for failed fields, and return false.

$('#myform').submit(function() {
 const allFields = [ 'dob', 'sex', 'weight', 'hair', 'eye' ];
 allFields
 .forEach(nameOfField => {
 $(displayError(nameOfField)).hide();
 $(formField(nameOfField)).removeClass('error-border');
 });
 const listOfFieldsFailedValidation =
 allFields
 .filter(field => {
 const fieldText = $('#' + field + '-display').text();
 return fieldText.indexOf('?') !== -1;
 });
 if (listOfFieldsFailedValidation.length <= 0)
 return true;
 $('html,body').animate({scrollTop:0}, 0);
 listOfFieldsFailedValidation
 .forEach(nameOfFieldFailedValidation => {
 $(displayError(nameOfFieldFailedValidation)).show();
 $(formField(nameOfFieldFailedValidation)).addClass('error-border');
 });
 return false;
});
function displayError(field) {
 return '#' + field + '-display-error';
}
function formField(field) {
 return '#' + field;
}

P.S. I assume ECMAScript 6 version of JS is okay.

answered May 2, 2017 at 4:32
\$\endgroup\$

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.