My Javascript form validation

Question

Hello today after few weeks of learning I tried some form validation. I am wondering how I could improve my code and what things I missed.

Here is a preview hosted on github + repository.
And here is my code :

HTML

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <link rel="stylesheet" href="main.css">
    <link href="https://fonts.googleapis.com/css2?family=Rubik:wght@300;400;500;600;700&display=swap" rel="stylesheet">
    <title>Form validation</title>
  </head>
  <body>
    <form action="#" class="form">
      <h2>Contact with us</h2>
      <div class="wrapper">
        <div>
          <input type="text" name="name" class='input-name' id='input' placeholder="First name *" required>
          <p id='input-name-p' class='p-hidden'></p>
        </div>

        <div>
          <input type="text" name="surname" class='input-surname' id='input' placeholder="Surname">
          <p  id='input-surname-p' class='p-hidden'></p>
        </div>

        <div>
          <input type="text" name="email" class='input-email' id='input' placeholder="E-mail *" required>
          <p id='input-email-p' class='p-hidden'></p>
        </div>

        <div>
          <input type="text" name="phone" class='input-phone' id='input' placeholder="Phone number">
          <p  id='input-phone-p' class='p-hidden'></p>
        </div>

        <textarea name="message" rows="8" cols="80" class='input-message' id='input' placeholder="Message *" required></textarea>
        <button href="#" class="btn">Submit</button>
      </div>
    </form>

    <script type="text/javascript" src="script.js">
    </script>

  </body>
</html>

JS

'use strict';

const submit = document.querySelector('.btn');
const name = document.querySelector('.input-name');
const surname = document.querySelector('.input-surname');
const email = document.querySelector('.input-email');
const phone = document.querySelector('.input-phone');
const items = document.querySelectorAll('#input');


const isValid = function(item) {
  for (let i = 0; i < item.length; i++) {
    if (item[i].value){ // Check if contains value

      let error = document.getElementById(`${item[i].className}-p`);
      const letters = /^[A-Za-z]+$/;
      const numbers = /^\d+$/;
      let inputLength = item[i].value.length;

      switch (item[i].className){
        case 'input-name':
        case 'input-surname':   
          if(!letters.test(item[i].value)){ // Invalid input 
            error.textContent = 'Invalid data';
            error.classList.remove('p-hidden');
          }else
          error.classList.add('p-hidden');
          break;

        case 'input-email':
          if(!item[i].value.includes('.') || !item[i].value.includes('@') || !letters.test(item[i].value[inputLength-1])  ){ // check if includes (@ or .) and check if the last index is a letter
            error.textContent = 'Invalid data';
            error.classList.remove('p-hidden');
          }else
          error.classList.add('p-hidden');
          break;
        case 'input-phone':
          if(!numbers.test(item[i].value) ||  inputLength<5){  // Invalid input 
            error.textContent = 'Invalid data';
            error.classList.remove('p-hidden');
          }else
          error.classList.add('p-hidden');
          break;
      }

    }
  }
}

for (let i = 0; i < items.length; i++) {
    items[i].addEventListener('click',function(){
      // Check if the input is valid
      isValid(items);
    });
}

CertainPerformance · Accepted Answer · 2020-11-05 18:53:05Z

Duplicate IDs are invalid HTML You have multiple elements of id='input', which is not permitted in HTML. If multiple elements need a particular attribute, use classes instead. IDs should be reserved for elements that are going to be absolutely unique on a page (or, you could also consider not using IDs at all, since they implicitly create global variables, which can lead to hard-to-understand bugs).

Input iteration You put the input collection into a variable named items, which is good: const items = document.querySelectorAll('#input'); but then you pass the collection to a function whose parameter is named item, and you do:

for (let i = 0; i < item.length; i++) {
  if (item[i].value) { // Check if contains value
    // a long block
  }

A reader of the code wouldn't expect an individual item to have a length and a numeric index. How about calling the parameter items instead - or, even better, inputs?
Rather than iterate over the indicies of each element, since you don't actually care about the indicies, but only about the underlying elements, it might be preferable to use for..of instead, so you never have to reference the indicies.
Nested indentation can be difficult to read. Instead of a long block inside an if statement, consider continuing the loop early instead:

for (const input of inputs) {
  if (!input.value) {
    continue;
  }
  // put validation code here

Or put it into a function:

for (const input of inputs) {
  if (input.value) {
    validateInput(input);
  }

Check validity on blur, not on click. Your current implementation will show errors only after the user has inputted something invalid, focused away, then clicked on the input box again. Better to inform them immediately, as soon as a box is de-focused.

Check validity even if input is empty, since errors may be displayed - if the input is empty, you'll want to clear the error. You could also consider clearing errors when an input is focused (so that errors are only displayed when an input currently isn't active).

Error text is always the same, so don't set it via the JS - put it into the HTML, and hide the error on pageload.

Use the case-insensitive flag in regular expressions instead of repeating both the capital and lowercase versions, eg: /^[a-z]$/i

Wording Contact with us would be better as Contact us

DRY input navigation There are a few sources of repetitiveness in the code:

Separate class names for each input, requiring iteration through each class name in the switch
Separate error class names for each input
Separate logic for each class name

You can make this better by:

Use an object indexed by the name attribute of each input, whose values are regular expressions to test the values against
Navigate to the input's adjacent error element with nextElementSibling instead of having separate error classes

For the email, you can use a regular expression so that the validation is of the same shape as for the other inputs.

const validators = {
  name: /^[a-z]$/i,
  surname: /^[a-z]$/i,
  email: /^(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])$/,
  phone: /^\d{5,}$/,
};
for (const input of document.querySelectorAll('#input')) {
  input.addEventListener('blur', () => {
    checkValidity(input);
  });
}
const checkValidity = (input) => {
  const isBad = validators[input.name].test(input.value);
  input.nextElementSibling.classList.toggle('p-hidden', isBad);
};

That's all you need.

'use strict';

const validators = {
  name: /^[a-z]+$/i,
  surname: /^[a-z]+$/i,
  email: /^(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*|"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])$/,
  phone: /^\d{5,}$/,
};
for (const input of document.querySelectorAll('.wrapper input')) {
  input.addEventListener('blur', () => {
    checkValidity(input);
  });
}
const checkValidity = (input) => {
  const isBad = validators[input.name].test(input.value);
  input.nextElementSibling.classList.toggle('p-hidden', isBad);
};

.p-hidden {
  display: none;
}

<form action="#" class="form">
  <h2>Contact us</h2>
  <div class="wrapper">
    <div>
      <input name="name" placeholder="First name *" required>
      <p class='p-hidden'>Invalid data</p>
    </div>

    <div>
      <input name="surname" placeholder="Surname">
      <p class='p-hidden'>Invalid data</p>
    </div>

    <div>
      <input name="email" placeholder="E-mail *" required>
      <p class='p-hidden'>Invalid data</p>
    </div>

    <div>
      <input name="phone" placeholder="Phone number">
      <p class='p-hidden'>Invalid data</p>
    </div>

    <textarea name="message" rows="8" cols="80" placeholder="Message *" required></textarea>
    <button href="#" class="btn">Submit</button>
  </div>
</form>

The email regex is probably more complicated than it needs to be, you could write a much more easy-to-read one with only a handful of characters that works just as well in 99% of situations, if you wanted.

Another option would be to remove all the JavaScript, and use the pattern attribute and type="email" in the HTML instead, letting the browser inform the user of invalid inputs:

<form action="#" class="form">
  <h2>Contact us</h2>
  <div class="wrapper">
    <div>
      <input name="name" placeholder="First name *" required pattern="[a-zA-z]+">
    </div>
    <div>
      <input name="surname" placeholder="Surname" pattern="[a-zA-z]+">
    </div>
    <div>
      <input name="email" placeholder="E-mail *" required type="email">
    </div>
    <div>
      <input name="phone" placeholder="Phone number" pattern="\d+{5,}">
    </div>

    <textarea name="message" rows="8" cols="80" placeholder="Message *" required></textarea>
    <button href="#" class="btn">Submit</button>
  </div>
</form>

Result:

\$\begingroup\$ such useful feedback thanks \$\endgroup\$
– fluffy
Nov 8, 2020 at 11:34 — fluffy, Nov 8, 2020 at 11:34
\$\begingroup\$ also thanks for your time I really appreciate it \$\endgroup\$
– fluffy
Nov 8, 2020 at 11:44 — fluffy, Nov 8, 2020 at 11:44

Stack Exchange Network

current community

your communities

more stack exchange communities

My Javascript form validation

1 Answer 1

Your Answer

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

Hot Network Questions

My Javascript form validation

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions