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;
};
\$\endgroup\$
  • 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\$ – John Hunt Jun 3 '16 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\$ – NJeffries Jun 3 '16 at 14:55
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
}
\$\endgroup\$
  • \$\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\$ – NJeffries Jun 4 '16 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.

\$\endgroup\$
  • \$\begingroup\$ Very cool. I appreciate this. \$\endgroup\$ – NJeffries Jun 4 '16 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
\$\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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.