3
\$\begingroup\$

I grabbed the form from some random site because I'm only interested writing the javascript at the moment.

I am trying to check that a user has selected or entered text for all fields. I've made it a long if if-else but that can't be the best/most elegant/easiest solution.

Leaving aside the radio button validation for now, what's the better way to check that the text fields, drop down, and checkboxes all have a value/input?

I'm teaching myself javascript so I'm open to being told the proper way and I'll research it and do it on my own, or updating my fiddle would be fine too. (Be gentle with me. I'm sure this code is janky.)

Any thoughts on this would be appreciated.

Fiddle: https://jsfiddle.net/kiddigit/g0rur21a/

document.getElementById("newForm").addEventListener("submit", enterForm);
function enterForm(event) {
 event.preventDefault();
 var dropdown = document.getElementById('dropDown');
 if (document.getElementById('fname').value === ''){ 
 document.getElementById('fname').focus(); 
 alert('Enter text.');
 } else if (document.getElementById('eMail').value === ''){
 document.getElementById('eMail').focus();
 alert('Enter text.');
 } else if (document.getElementById('textArea').value === '') {
 document.getElementById('textArea').focus();
 alert('Enter text.');
 } else if (!dropDown.value) {
 document.getElementById('dropDown').focus();
 alert('Choose an option.');
 } else if ( ( newForm.checkbox[0].checked == false ) && ( newForm.checkbox[1].checked == false ) ) 
 { alert ( "Please choose a checkbox" ); 
 return false;
 }
 var radios = document.getElementsByName("radio");
 var formValid = false;
 var i = 0;
 while (!formValid && i < radios.length) {
 if (radios[i].checked) formValid = true;
 i++; 
 }
 if (!formValid) alert("Please check a radio button.");
 return formValid;
 return false;
};
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Jun 3, 2016 at 14:20
\$\endgroup\$
2
  • 2
    \$\begingroup\$ You might want to just use something like jQuery validate (google it), it provides tried and tested ways of validating forms on the client side and can do lots of clever stuff. Doing it yourself is fine for learning but using a 3rd party lib will be much better in all ways 9 times out of 10. \$\endgroup\$ Commented Jun 3, 2016 at 14:38
  • \$\begingroup\$ I'll take a look. But yeah, was just trying to do it myself for learning purposes. Thanks for the tip though. \$\endgroup\$ Commented Jun 3, 2016 at 14:55

4 Answers 4

1
\$\begingroup\$

Couple of suggestions from my end.

1> There is a better way to check null or empty in javascript,this can be done more efficiently with this

 if (document.getElementById('fname').value)
 {
 //value exists.
 }

Above condition evaluates to true if value is not equal to any of the following:

  • null
  • undefined
  • NaN
  • empty string ("")
  • 0
  • false

2> Try to use switch case instead of so many if else.It will make your code more elegant and readable.

3> Take care of indentation currently it's not very good.

4> Instead of writing ewForm.checkbox[0].checked == false you can use

if(!ewForm.checkbox[0].checked) 
{
//not checked
}
answered Jun 4, 2016 at 15:45
\$\endgroup\$
1
  • \$\begingroup\$ Yeah, my indentation is a joke. I thought about switch case but wasn't 100% sure how to implement that. I'll take a shot at that now. \$\endgroup\$ Commented Jun 4, 2016 at 15:54
1
\$\begingroup\$

Add the required attribute to all your inputs. Then:

// Alias to other names for maximum browser support.
if (!Element.prototype.matches) {
 Element.prototype.matches =
 Element.prototype.webkitMatchesSelector ||
 Element.prototype.mozMatchesSelector ||
 Element.prototype.msMatchesSelector ||
 Element.prototype.oMatchesSelector
}
// Check validity.
if (newForm.matches(':valid')) {
 console.log('form is valid')
} else {
 console.log('form is invalid')
}

This checks if the form matches the CSS pseudo-class :valid using Element.matches().

Note that this checks for validity rather than every input being filled in. It's close, but not quite what you asked.

answered Jun 4, 2016 at 16:35
\$\endgroup\$
1
  • \$\begingroup\$ Very cool. I appreciate this. \$\endgroup\$ Commented Jun 4, 2016 at 17:39
1
\$\begingroup\$

It looks as though you're alerting "Enter text." in case there is a text input, and "Choose an option." in case it is a checkbox/radio input. Besides how bad using alert is, you can still check for all inputs automatically without using super-nested ifs

var form = document.getElementById( 'newForm' );
// other code
var inputs = form.querySelectorAll('input'),
 wasFilled = true,
 i = 0;
while( wasFilled ){
 wasFilled = ( inputs[ i ].type === 'text' ) ?
 inputs[ i ].value : inputs[ i ].checked && inputs[ ++i ].checked; // for the 2 checkboxes, you don't want to iterate over the second checkbox
 ++i;
}
if( !wasFilled ){
 var element = inputs[ i ];
 alert('Enter text.');
 element.focus();
 return false;
}
// rest of code
answered Jun 5, 2016 at 22:34
\$\endgroup\$
1
\$\begingroup\$

In addition to the above mentioned, instead of alert(which is also mentioned already. Think of this as one of the solutions.). You could mention that a field is empty with an adjacent label to that field in addition to bringing focus to that field as well. Alternatively you could also use placeholders.

answered Jun 6, 2016 at 5:44
\$\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.