Sunny Landing Page built with - HTML5, CSS(GRID/FLEX), SASS, BEM, JS

Community feedback

• 

  Raymart Pamplona•16,040

  @pikapikamart

  Posted over 3 years ago

  Hey, awesome work on this one. Desktop layout looks great, for the site responsiveness, it responds but if you go like 704px, you will see that contents are being squished along with the images and some text are being hidden by their counterpart images. For mobile state though, it looks great.

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

  • Don't use width: 100vw since this will only add a horizontal scrollbar at the bottom, since this value does not account the vertical scrollbar's width.
  • For this one, I would prefer the header to only contain the topmost part of the site, the website-logo and the navlinks. The reason is that, you want to make the header reusable on different pages of the site, by including the hero-section inside the header it won't be reusable since you don't want to include the hero-section for other pages right. Leave the hero-section to the main tag.
  • Put an aria-label="primary" on the nav on the header so that it will be unique since we are going to use another nav tag on the site.
  • Since the website-logo is not being treated as a link, you can remove it from the nav tag.
  • 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="sunnyside".
  • Those 4 links could be wrapped by a ul tag since those are "list" of links.
  • For the hero-section and for other parts of the site that uses section change those into div since section alone is not informative as a landmark since it doesn't give any extra information unless it is labelled by aria-labelledby attribute.
  • The arrow-down-icon is only decorative. 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.
  • Only use descriptive alt on images that are meaningful and adds content to the site otherwise hide the image for screen-reader users.
  • Those other images, like the egg, glass fruits etc. Those are only decorative images on the site as well so hiding them would be great, use the method I mentioned above.
  • For each of the a tag that contains learn more if you hover on its right side, it is still clickable since it is using the full width of the container. You can use max-width: max-content on it so that it will be capped.
  • For each testimonial person. Person's image should be using the person's name as the alt value like alt="Emily R.". Components like this where a person's information is presented, make sure to use their name as the person's img alt value.
  • Also, you can make use of this markup so that it will be easier for screen-reader users to navigate the testimonials using blockquote:

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

  FOOTER

  • Since you are using svg for the website-logo, use this markup to give it an alternative text:

  <svg role="img">
    <title>Sunnyside</title>
  </svg>
  

  • Those 3 links below the website-logo could be wrapped inside a nav, just make sure to add aria-label="footer" on the nav so that it will be distinct. Also you can again use ul since they are "list" of links:

  <nav>
    <ul> list of links </ul>
  </nav>
  

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

  MOBILE

  • Your hamburger-button should have either aria-label attribute or sr-only text inside it which defines what the button does. It could be something like aria-label="navigation menu dropdown".
  • The hamburger button should have a default attribute of aria-expanded="false" and it will be set to true when the users toggles it and vice-versa.
  • The img inside the hamburger-menu should have been hidden since it is only a decorative image.

  Aside from those, great job again on this one.

  11
• 

  MikevPeeren•2,100

  @MikevPeeren

  Posted over 3 years ago

  Hey 👋

  Good job completing this project 👏

  Try to keep in mind when using alts if for one the image is needed for screenreaders to understand for example the arrow down isn't as its only decorative. And for the hamburger menu a more descriptive text would be better as not everyone knows what a hamburger menu is.

  As for your code I would advise to use BEM or other methodology for sass as it would be more readable. http://getbem.com/

  00