The News Letter Sign Up

Community feedback

• 

  P

  schindlerdumagat•290

  @schindlerdumagat

  Posted 22 days ago

  Here are the issues I saw:

  HTML

  • You should enclose all the html code in the body element within a landmark role. In this case, you can put it in a main tag.
  • I recommend that you use a picture element for the illustration image to be able to download only what is needed based on the device width. Your solution currently download all of the illustration even though you only need the mobile version if you are on mobile view for example.
  • I recommend that you use h2 tag instead of h1 for the form title because the form will only be a part of the page and it will not probably hold the main title of the page.
  • The icon list are just decorative. They don't provide context to the page so an alt attribute with the value of empty string is enough to make it decorative. Do the same thing with the icon image in the success page.

  CSS

  • There are still some inconsistencies with the UI. It seems like the tablet view was not implemented. You also need to fix some parts of UI to closely match the design provided.

  JS

  • As you have mentioned, you have added styling logic to your JavaScript code directly. For you to fix that, you can create classes instead and style them in your CSS. To apply those classes in your elements, you can use methods on your elements such as element.classList.add("className"), element.classList.remove("className"), element.classList.toggle("className") just to name a few. You can learn more about how this code works on the internet.

  12

  Marked as helpful