Ping coming soon page

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, awesome for another challenge. The desktop layout looks fine, though the hero-image is quite big/long and making the site a bit longer because of some white-spaces as well. The site is responsive but if you go at 340px above, you will notice that the site is hiding the content of the page and creates horizontal scrollbar.

  MordenWebDev already gave a feedback on this one, just going to add some suggestions as well:

  • Avoid using height: 100vh on a large container like the body as this makes the element's height capped based on the viewport/screen's height. Instead use min-height: 100vh so that the element will expand if it needs to.
  • Again, a page needs to have a main tag to wrap the main-content so that user will be able to quickly navigate the content using landmark elements. Use main on the .container.
  • Remember that a website-logo is one of the meaningful images on a site so use proper alt for it. Use the website's name as the value like alt="ping".
  • Your input right now currently lacks associated label to it or an aria-label to which will define the purpose of the input element. Always include it so that user will know what they need to give on each input. Make sure that label is pointing to the id of the input as well.
  • The notify-button should be using a type="submit" attribute. Remember that when a button is placed inside a form element, it defaults to type="submit". So imagine if you have a close-button inside the form without specifying type="button" clicking the close-button will submit the form. Be aware of this kind of scenarios.
  • Right now, the error message or the error in general is only seen visually but not really linked to the input properly. A proper way of adding errors would look something like this pseudocode:

  if ( input is wrong )
    input.setAttribute("aria-invalid", "true");
    input.setAttribute("aria-describedBy", id of the error-message);
  else 
    input.removeAttribute("aria-invalid");
    input.removeAttribute("aria-describedBy");
  

  The error-message element should have an id attribute which is referenced by the aria-describedBy attribute on the input element. By doing that, your user will know that the input is wrong because of aria-invalid and they will know what kind of error they made because of the aria-describedBy. If you like, have a look at this simple accessible form snippet that I have. Let me know if you have any queries about this one.

  • For the hero-image, you could use a more descriptive alt, it could be something like `ping's application user dashboard" or if you could come up with a more descriptive.
  • Those social-media links could be inside a ul element since those are "list" of links.
  • Each a tag that wraps the social-media icon should have either aria-label attribute or sr-only text inside it, defining where the link would take them. For example, you should use facebook as the value if the link would take the user to facebook.
  • Also, I would put the social-medias inside the footer since those aren't really part of the main-content of the page.
  • Lastly, just reducing some sizes perhaps of the image and some white-spaces:>

  Aside from those, great job again on this one.

  0

  ---

  

  Mahnoor Khan•455

  @mahnoork18Posted almost 3 years ago

  @pikapikamart thankyou for making me realise my small mistakes while coding. just wanted to know the code snippet you shared doesn't have emailValidation, I think my js code is working fine but I will definitely look into it. thanks

  1

  

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  @mahnoork18 Hey, glad that you found my review useful^^

  For the snippet, there is a email validation as well on that one, just using a simple regex on the testEmail function.

  Though that whole code snippet is more likely to be seen as adding necessary error attributes:>

  0
• 

  MordenWebDev•500

  @MordenWebDevPosted almost 3 years ago

  media query need to be adjusted because you are not using device screen space properly. even in the 1366px it goes to mobile design.

  0

  ---

  

  Mahnoor Khan•455

  @mahnoork18Posted almost 3 years ago

  @MordenWebDev fixed it thanks for identifying!

  0