Huddle-landing-page with flex and grid

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Desktop layout looks fine, just a bit shorter than the design. For the responsive state, it could be better because there are some sizes where the layout gets odd/ugly, for example on size 590px, you will see that the hero-section content is squished, the image, text, also the footer is being hidden. Though mobile state looks fine actually.

  Some suggestions would be:

  • For this one, it would be better to use the header to wrap only the top-most part but not including the hero-section.
  • Website-logo does not need to be inside the nav since it is not being treated as a link.
  • 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="huddle".
  • The hero-section image could use an extra aria-hidden="true" attribute. Decorative images should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if the image is using svg.
  • Always have a main tag to wrap the main-content. For this, use main on the .trio selector instead of section and not the main being used in the very inside of the same selector.
  • Other images being used in here are only decorative so hide them using the method I mentioned above.

  FOOTER

  • The cta should not be included in the footer since footer is typically reused on a site, you don't want every page to have a cta right, it might ruin a design/layout.
  • Since you are using svg for the website-logo, you should have done it this way:

  <svg role="img">
    <title >huddle</title>
    ....
  </svg>
  

  This way, screen-reader will be able to read such svg .

  • Those 3 icons below the logo should be hidden, the email, telephone etc.
  • Those 6 links could be wrapped inside a nav element since those are still your website's navigational links and they could also be wrapped inside ul since those are "list" of links.

  <nav>
    <ul>
      <li>
        <a href=""></a>
      </li>
     .....
    </ul>
  </nav>
  

  You will just need a single ul on this one since they are related list-items, use grid to place each item properly.

  • hose 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.
  • Social-media image should be hidden since it is only a decorative image so use aria-hidden="true".

  Aside from those, great job again on this one.

  0