Intro component with sign-up form

Community feedback

• 

  Cats-n-coffee•670

  @Cats-n-coffee

  Posted over 2 years ago

  Hi Angelo!

  Nice job! Form validation is never an easy thing, especially when there's an email address in the form (you'll see a lot of people use libraries and validation is usually done in the frontend and backend as well). These are the things that caught my attention:

  • Your submit button is inside a div but the event listener is on the input which is much smaller than your div. At first I thought the button wasn't working because I didn't think of clicking on the text itself. I would suggest removing the div and use only your input (or you could use a <button type="submit"></button>.
  • Your validation is good for basic things such as empty fields, email and password length. You could add a length check for names, and white space trimming for all the fields (except password maybe, I'm not well versed in password validation). Another thing to look into if you're interested is input sanitation, which should be done on both front and backend (most modern frameworks will do this for you).
  • Another fun thing to implement is validation on blur which you'll see used often especially in long forms.

  Js feedback:

  • it seems that you can place most of your selectors at the very top of the file with the ones you already have there (like your errors divs) and they can all be const since you are not re-assigning the values.
  • You should be able to remove the selectors like this one document.forms['myForm']['firstName'].value;, you already have each form input at the top so you can do firstName.value.
  • I think you could place your event listener on the form itself and listen for the submit event, it'll help with code readability.
  • To keep your Js shorter (and slightly improve performance - not that this matters here), you can place a class on each part that changes with validation (error div and input) and add/remove the class in Js. You might be able to integrate the error icon into the class logic as well, so you don't need to loop through them at the beginning and change the background. Your Js isn't as bad as you think :)

  Let me know if you have any questions, hope this helps!

  11

  Marked as helpful