Css Grid Layout with BEM Css, a few of SMACSS Css and basic Javascript

Community feedback

• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  Hey Sebastian! First of all, sorry it's taken me so long to look at your project! It's been a very busy couple of weeks and I haven't been able to look at people's solutions as much as I would have liked.

  You've done a really good job on this challenge. Here are some thoughts after taking a look at your solution:

  • Your styling looks great. One thing I would recommend reviewing is the responsiveness of your layout. Around the tablet sizes, the content is too wide for the screen.
  • Your HTML structure is really good. There's not much I would change. A nice addition for accessibility would be to add the aria-live="polite" attribute to the error messages. This ensures that screen reader software will read out the errors as they get added to the page.
  • You need to add event.preventDefault() inside the validationForm function to prevent the flash when the form is submitted. By default, a form being submitted will refresh the browser, so you lose all of your error states.
  • I would also look at breaking up your functions into specific use cases. For example, you could have smaller validateRequired and validateEmail functions that perform specific tasks.
  • Also, avoid setting styles in your JavaScript. Instead, use JS to toggle classes that will then add/remove styles as needed.

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

  0

  ---

  

  Sebastian Rodriguez Ardila•90

  @sebasrodri237Posted over 4 years ago

  Thanks for the feedback matt, I will do.

  0