0

I'm trying to validate a form, by using a validation function, using JS. When I try to divide the form by using div elements, it doesn't call the validation function anymore.

<form id="myForm" onsubmit="return validateForm();"  method="post" action="form-handler.html">
    <div>
        <label for="firstName">First Name:</label>
        <input name="firstName" type="text" placeholder="Jhon">
    </div>

    <div>
        <label for="submit">Submit</label>
        <input id="submit" type="Submit" value="send">
    </div>
</form>

<script>
    function validateForm(form) {
        //validation failed if first-name input is empty
        if(form.firstName.value == "")
        {
            alert("Error: Input is empty!");
            return false;
        }
        return true;
    }
</script>
advo.net
  • 1
  • 1
  • 3
    try `onsubmit="return validateForm(this);"` You're not passing the form into your function – WillardSolutions Mar 26 '19 at 19:24
  • Also note that the `for` attribute in `` should refer to an `id` of an `` and will not work on the name. You can test the difference by clicking on your label and see whether your input is focused after that. – Webber Mar 26 '19 at 19:31
  • @Webber `label` can never be written as `` and really, in 2019, there's no use for self-terminating tags even where they are allowed. – Scott Marcus Mar 26 '19 at 19:34

2 Answers2

0

Dividing up the form contents with div has no bearing on whether you can invoke a validation function. The problem is that you aren't referring to the element that you want to validate correctly. Your function expects an argument, which you called form, but your code to invoke the function doesn't pass any arguments, so inside your function, form is undefined and therefore you aren't locating the text field you want to validate.

Instead, just reference the element(s) you wish to test:

<form id="myForm" onsubmit="return validateForm();"  method="post" action="form-handler.html">
    <div>
        <label for="firstName">First Name:</label>
        <input name="firstName" type="text" placeholder="Jhon">
    </div>

    <div>
        <label for="submit">Submit</label>
        <input id="submit" type="Submit" value="send">
    </div>
</form>

<script>
    function validateForm(evt) {
        // Reference the field(s) to validate properly:
        if(document.querySelector("[name='firstName']").value == ""){
            alert("Error: Input is empty!");
            return false;
        }
        return true;
    }
</script>

Now, having said that, don't use inline HTML event attributes, like onsubmit in the first place. That technique is 25+ years old and won't die the death it deserves because of how often people just copy it without understanding why it shouldn't be used. Instead, follow modern standards and separate all your JavaScript from your HTML. Also, your first label is not set up correctly. The for attribute value must match the id of the element that the label is "for".

<form id="myForm"  method="post" action="form-handler.html">
    <div>
        <label for="firstName">First Name:</label>
        <input name="firstName" id="firstName" type="text" placeholder="Jhon">
    </div>

    <div>
        <label for="submit">Submit</label>
        <input id="submit" type="Submit" value="send">
    </div>
</form>

<script>

  // Get a reference to the form and set up a submit event handler
  document.getElementById("myForm").addEventListener("submit", validate);
  
  // The argument is automatically passed and represents
  // the event itself
  function validate(evt) {
        //validation failed if first-name input is empty
        
        // Properly reference the element(s) you wish to test
        if(document.querySelector("[name='firstName']").value == ""){
            alert("Error: Input is empty!");
            
            // Tell the event not to proceed.
            evt.preventDefault(); 
        }
    }
</script>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
  • By the way, using `querySelector` in a form validator also introduces spaghetti code and fails to separate concerns, as it makes the JS dependent on the markup. – WillardSolutions Mar 26 '19 at 19:37
  • @WillardSolutions Having JS dependent on markup is not what determines if you are separating your concerns. And, querySelector requires a valid CSS selector to work - - there's no way around that. You confuse reference with combine. – Scott Marcus Mar 26 '19 at 19:40
  • You can't test JS that depends on markup, Just pass the form object into the function – WillardSolutions Mar 26 '19 at 19:41
0

Option 1:

Change

<form id="myForm" onsubmit="return validateForm();"  method="post" action="form-handler.html">

Into

<form id="myForm" onsubmit="return validateForm(this)" method="post" action="form-handler">

To provide the form as an argument to your validateForm function.

Option 2

Change

 <label for="firstName">First Name:</label>
<input name="firstName" type="text" placeholder="Jhon">

into

<label for="firstName">First Name:</label>
<input name="firstName" id="firstName" type="text" placeholder="Jhon">

To fix your form and make the input easily selectable in javascript.

Then:

function validateForm(form) {
  var firstName = document.getElementById('firstName')

  if (firstName.value == "") {
    alert("Error: Input is empty!");

    return false;
  }

  return true;
}
Webber
  • 4,672
  • 4
  • 29
  • 38
  • Then your actual code must be different from your opening post, because this should work. Could you make a runnable code example that shows what doesn't work? – Webber Mar 26 '19 at 20:10