fylo-landing-page with html and CSS flexbox

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, awesome work on this one. The layout in desktop looks fine, although there is an extra space on the right side thus creating a horizontal scroll. Resizing the screen, the layout responds but not that well since if you go to at like 1010px upwards, the content are stuck on the left side and not really responding. For mobile state, the layout is fine but there is a huge horizontal scrollbar as well.

  Here are some suggestions for the site:

  • Adding a max-width on the body tag would be nice. If you try zooming out on your screen, you will see that the overall content are just stretching as the screen changes. The max-width will prevent it since it will limit the content's width and also add a margin: 0 auto so that it will be centered.
  • On this, the main should be outside the header tag so that it will be treated as a primary landmark:

  <header />
  <main />
  <footer />
  

  • The header should only contain the topmost part of the site, including the site-logo, navlinks and some other controls.
  • The site-logo could be left out from the nav since it is not being treated as a link. If you insist keeping it, then wrap the img by a tag with a proper aria-label or sr-only text.
  • Add a aria-label="primary" on the nav , this will make sure that it stays unique since I will be suggesting another usage of nav on this layout.
  • 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="fylo".
  • When using img tag, you don't need to add words that relates to "graphic" such as "logo, image" and others, since img is already an image so no need to describe it as one.
  • Whenever there is input on the site that submits data, always wrap it in a form tag to make it more semantic.
  • Also, you can have this snippet of mine to reference for accessible form and validation.
  • When there is a button that submits the form, always use type="submit". 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.
  • Almost all the images being used on the site are just decorative except for the site-logo and the testimonial person-image. 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 you are using svg instead of img tag.
  • On the .curv selector, this is the curve background-image, this is the one causing the horizontals scrollbar. To fix, just add width: 100% on it so that it will scale well.
  • For the testimonial card, you can use this markup below to make it more easier for screen-reader users to navigate through it:

  <figure>
    <blockquote>
      <p> {qoute in here}</p>
    </blockquote>
    <img src="" alt={person name}>
    <figcaption>
      person name
      <p>
        person role
      </p>
    </figcaption>
  </figure>
  

  • Person's img should have used their name as the alt value since in a testimonial, it is all about the person so it makes sense to make their image visible for all users by using their name as the alt value.

  FOOTER

  • The footer tag should be wrapping the bottom-most part of the site and not the .attribution, you can include the .attribution inside the footer but make sure that the footer wraps the correct content.
  • For the site-logo, you could have used the svg code of the logo and use the fill attribute to set it to color white so that it will be visible.
  • The site-logo as well should not be included in the ul tag that you used since that site-logo is a content of its own and not a list item.
  • Those 7 links could have live inside a nav since those are still your navigational links. Also, since they are all related, using a single ul tag to wrap those will be really great:

  <nav aria-label="footer"> 
    <ul>
      <li>
        <a href=""></a>
      </li>
    </ul>
  </nav>
  

  Use a tag for each since they are all acting as 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.
  • Lastly, addressing the styling issue of the site. From the breakpoint to the zooming out of the screen.

  Aside from those, great job again on this one.

  0