1
\$\begingroup\$

I've been working on a simple form validation code in JavaScript coupled with an HTML form. The form asks for user's basic information, including name, email, country, gender, and skills. There's a form validation function that checks if all required fields are filled in before submission.

While the current code works as expected, I believe there might be some room for optimization, better practices, or even a more efficient way to validate the form.

Here's the Code:

let nom = document.querySelector("#nom")
let prenom = document.querySelector("#prenom")
let mail = document.querySelector("#mail")
let select = document.querySelector("#select")
let radios = [... document.querySelectorAll("input[type=radio]")]
let checkboxs = document.querySelectorAll("input[type=checkbox]")
function verif() {
    
    if (prenom.value == "") 
        errpre.innerHTML = "Veuillez saisir le prénom"
    else
        errpre.innerHTML = ""
    
    if (nom.value == "") {
        errnom.innerHTML = "Veuillez saisir le nom"
    }else{
        errnom.innerHTML = ""
    }
    if (mail.value == "") {
        errmail.innerHTML = "Veuillez saisir le mail"
    }else{
        errmail.innerHTML = ""
    }
    if(radios[0].checked== false && radios[1].checked   ==false){
        errgenre.innerHTML = "Choisir le genre"
    }else{
        errgenre.innerHTML = ""
    }
    let isChecked = false
    for (let i = 0; i < checkboxs.length; i++) {
        if (checkboxs[i].checked) {
            isChecked = true
        }
        
    }
    if (isChecked == false) {
        errcomp.innerHTML ="Choisir au moins une langage"
    }else{
        errcomp.innerHTML =""
    }
    if(select.selectedIndex == 0){
        errpays.innerHTML="Choisir une pays"
    }else{
        errpays.innerHTML=""
    }
}
btn.addEventListener('click',()=>{
    verif()
})
        .err{
            color: red;
        }
  <body>
    <fieldset>
      <legend>Vos Cordonnés</legend>
      Prénom : <input type="text" id="prenom" /><span id="errpre" class="err"></span><br /><br />
      Nom : <input type="text" id="nom" /><span id="errnom" class="err"></span><br /><br />
      Email : <input type="email" id="mail" /><span id="errmail" class="err"></span><br /><br />
      Pays : <span id="errpays" class="err"></span><br />
      <select id="select"><span id="errpays" class="err"></span>
        <option></option>
        <option>Maroc</option>
        </select><br /><br />
      Genre:
      <input type="radio" name="sex" />Homme
      <input type="radio" name="sex" />Femme <span id="errgenre" class="err"></span>
    </fieldset>
    <fieldset>
      <legend>Vos Compétences</legend>
      <input type="checkbox" />HTML <input type="checkbox" />CSS
      <input type="checkbox" />PHP <input type="checkbox" />MySQL
      <input type="checkbox" />Javascript<span id="errcomp" class="err"></span>
      <br />
      Commentaires <br />
      <textarea cols="20" rows="3"></textarea>
    </fieldset>
    <br />
    <button id="btn">Envoyer</button>
    <script src="sc.js"></script>
  </body>

I'd appreciate any suggestions to make the code cleaner, more readable, and efficient. Please let me know if there are any modern JavaScript features or methodologies that could be used to improve this code.

Thank you in advance for your help!"

\$\endgroup\$
2
  • \$\begingroup\$ it's champ verifed le error message delete \$\endgroup\$
    – XMehdi01
    May 15, 2023 at 20:35
  • \$\begingroup\$ no check the code \$\endgroup\$
    – XMehdi01
    May 15, 2023 at 20:36

1 Answer 1

1
\$\begingroup\$

You can simplify this a lot with proper markup and use of existing features.


General observations

Starting with general observations to the code, I noticed that you're writing everything in French.
Please, avoid doing that, as it makes it harder for someone else to read and/or fix or improve your code.


HTML

HTML includes some validation facilities, like the required attribute.

You're also using a <button> as a submit button, but without a form.

You have almost everything, except the <form> element is missing, which is the most important.


Something else you're doing is to use <br><br> to separate elements.
You're not writing HTML code for emails to be read in modern Microsoft Office.

Instead, just use a <div> on your elements.


Additionally, the id value errpays is repeated: that's invalid HTML and you must use a different id for both elements.


The indentation is fine, but you have really long lines, and you aren't using <label> tags properly.

You also don't have values and names on almost all elements, which is kinda required to make use of them.


Here's a simple re-write of the HTML you have, but with better markup:

<form id="form" action="#">
  <fieldset>
    <legend>Vos Cordonnés</legend>
    <div>
      <label for="prenom">Prénom:</label>
      <input type="text" id="prenom" name="prenom" required />
    </div>

    <div>
      <label for="nom">Nom:</label>
      <input type="text" id="nom" name="nom" required />
    </div>
    
    <div>
      <label for="email">Email:</label>
      <input type="email" id="mail" name="mail" required />
    </div>
    
    <div>
      <label for="select">Pays:</label>
      <select id="select" required >
        <option value=""></option>
        <option>Maroc</option>
      </select>
    </div>
    
    <div>
      Genre:
      
      <label>
        <input type="radio" name="sex" value="h" required />
        Homme
      </label>
      <label>
        <input type="radio" name="sex" value="f" />
        Femme
      </label>
    </div>
  </fieldset>
  
  <fieldset>
    <legend>Vos Compétences</legend>
    <div>
      <label>
        <input type="checkbox" name="competences[]" value="html" />
        HTML
      </label>
      
      <label>
        <input type="checkbox" name="competences[]" value="css" />
        CSS
      </label>
      
      <label>
        <input type="checkbox" name="competences[]" value="php" />
        PHP
      </label>
      
      <label>
        <input type="checkbox" name="competences[]" value="mysql" />
        MySQL
      </label>
      
      <label>
        <input type="checkbox" name="competences[]" value="js" />
        JavaScript
      </label>
    </div>
    
    <div>
      <label for="commentaires">Commentaires</label>
      <textarea id="commentaires" name="commentaires" cols="20" rows="3"></textarea>
    </div>
  </fieldset>
  
  <button type="submit">Envoyer</button>
</form>

Please note that the checkboxes aren't marked as required.
You need to validate it in JavaScript, because HTML doesn't offer an option to validate it with the existing features.


Additionally, you could also defer the script, to reduce render blocking.

It isn't a requirement for your small HTML file, but it's something to keep in mind.



CSS

You can use some simple CSS to make the whole thing look better, doing all the separation in it.
This keeps the presentation separated from the markup, which is a good idea to do.

You can do something like this:

div, fieldset {
  margin-bottom: 0.5rem;
}

div:last-child,
fieldset:last-child {
  margin-bottom: 0;
}

label:not(:has(input, textarea)) {
  cursor: pointer;
  
  display: inline-block;
  width: 8ch;
}

label[for="commentaires"] {
  display: block;
  width: auto;
}

Yes, this isn't the prettiest, but it works well enough.

This is just to give you an idea, not to be an actual implementation.



JavaScript

All you really need to do is to handle the form submission, check if there's any error and stop if there is.

You can use the reportValidity() for method to detect if the form has no validation issues.

You can do something like this:

const form = document.getElementById('form');

form.addEventListener('submit', (event) => {
  if(event.target.reportValidity()) {
    // Stops the browser from submitting
    event.preventDefault();
  }
});

This is just an example of how you can use the validation features.


You also should wrap the entire code in a load event handler, but you've put the file at the end of the document, so, it isn't too important, but still important.

I won't do it in my examples because ... lazy...

And yes, even for a small script, like yours, there's tons of benefits, like scoping your constants away from the global scope.
This has performance and reliability improvements.




An example...

Here's a full example of how you can everything.

This uses a <dialog> with a basic message.

const form = document.getElementById('form');
const dialog = document.getElementById('ok-dialog');

form.addEventListener('submit', (event) => {
    // Stops the browser from submitting
  event.preventDefault();
    
    if(event.target.reportValidity()) {
    // Shows the <dialog> element
    dialog.showModal();
    
    // Clears the form from all values
    form.reset();
  }
});
div, fieldset {
  margin-bottom: 0.5rem;
}

div:last-child,
fieldset:last-child {
  margin-bottom: 0;
}

label:not(:has(input, textarea)) {
  cursor: pointer;
  
  display: inline-block;
  width: 8ch;
}

label[for="commentaires"] {
  display: block;
  width: auto;
}
<form id="form" action="javascript:void(0);">
  <fieldset>
    <legend>Vos Cordonnés</legend>
    <div>
      <label for="prenom">Prénom:</label>
      <input type="text" id="prenom" name="prenom" required />
    </div>

    <div>
      <label for="nom">Nom:</label>
      <input type="text" id="nom" name="nom" required />
    </div>
    
    <div>
      <label for="email">Email:</label>
      <input type="email" id="mail" name="mail" required />
    </div>
    
    <div>
      <label for="select">Pays:</label>
      <select id="select" required >
        <option value=""></option>
        <option>Maroc</option>
      </select>
    </div>
    
    <div>
      Genre:
      
      <label>
        <input type="radio" name="sex" value="h" required />
        Homme
      </label>
      <label>
        <input type="radio" name="sex" value="f" />
        Femme
      </label>
    </div>
  </fieldset>
  
  <fieldset>
    <legend>Vos Compétences</legend>
    <div>
      <label>
        <input type="checkbox" name="competences[]" value="html" />
        HTML
      </label>
      
      <label>
        <input type="checkbox" name="competences[]" value="css" />
        CSS
      </label>
      
      <label>
        <input type="checkbox" name="competences[]" value="php" />
        PHP
      </label>
      
      <label>
        <input type="checkbox" name="competences[]" value="mysql" />
        MySQL
      </label>
      
      <label>
        <input type="checkbox" name="competences[]" value="js" />
        JavaScript
      </label>
    </div>
    
    <div>
      <label for="commentaires">Commentaires</label>
      <textarea id="commentaires" name="commentaires" cols="20" rows="3"></textarea>
    </div>
  </fieldset>
  
  <button type="submit">Envoyer</button>
</form>

<dialog id="ok-dialog">
  <p>Merci de nous contacter.</p>
  <form method="dialog">
    <button>OK</button>
  </form>
</dialog>

\$\endgroup\$
3
  • \$\begingroup\$ No, that's not what I want. I want the same functionality as the code you provided, with some improvements \$\endgroup\$
    – XMehdi01
    May 15, 2023 at 21:50
  • 1
    \$\begingroup\$ You can implement the improvements yourself. If you want a focus on more specific areas, I can review specific areas a little deeper. In fact, I did everything you asked in "While the current code works as expected, I believe there might be some room for optimization, better practices, or even a more efficient way to validate the form." and even implemented an example that shows that it works, and you have an idea of how you can handle it as well. \$\endgroup\$ May 15, 2023 at 21:58
  • \$\begingroup\$ I apologize, but you did not understand very well what I want, but in general your answer is useful \$\endgroup\$
    – XMehdi01
    May 15, 2023 at 22:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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