Responsive landing page using SASS and mobile first

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Layout in desktop looks great, just the background-image at the top part should be adjusted. The mobile layout is great, but your breakpoint is too early, right now at 1240px, mobile view already takes place which is too big for mobile layout since desktop layout could use more of those screen-time, adjusting it would be nice.

  Some other suggestions would be:

  • The background-image would be better if you set it as a value in the background property on an element of your choice in here. It could be on the body tag, this way there is no extra element on the markup.
  • A typical layout of a site looks like this:

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

  This way, all element that has content are inside their respective landmark elements.

  • Avoid using height: 100vh on a large element like what you did on the header tag as this limits the element's height based on the remaining screen's height. Instead use min-height: 100vh, this takes full height but lets the element expand if needed. You can use rem as well so that it will be consistent.
  • nav is not needed in the header in this layout since there are no navigational links that are being used as primary. A div would be fine.
  • Website-logo img should only be using the website's name as the alt lose the word logo. When using alt attribute, avoid using words that relates to "graphic" such as "logo" and others. An img is already an image/graphic so no need to describe it as one.
  • Same goes for the other images, avoid improper alt. If the image is meaningful, then make a descriptive alt value, if it is decorative hide it by using alt="" and extra aria-hidden="true" attribute on the img tag.
  • 3 icons before the company logos are decorative only, hide it using the method I mentioned above.
  • aside is not needed as well in wrapping the company logos, section would be fine or a div.
  • You can use I think ul on the companies since it is a list of companies and use a proper alt only, read what I mentioned above. Also you don't need to use figure a lot to wrap an img, you can just use img directly, since when using figure you are typically highlighting an image that is why you use figcaption inside it.

  FOOTER

  • Use proper alt for the website-logo.
  • Those 5 links could be inside a nav element, since those are your website's navigational links only that they are secondary. You can nest as well those inside a ul element since those are "list" of links.

  <nav>
    <ul>
    </ul>
  </nav>
  

  • Social media could be wrapped as well inside ul since again, "list" of links.
  • Since those social media is supposed to be a link, use a tag for each list link.
  • Each a tag that wraps social media, it should have either aria-label attribute or screen-reader element inside it. The value for whatever method you will use should be the name of the social media like aria-label="facebook" on the facebook link a tag. This way, users will know where this link would take them.
  • Each svg inside the social media link should be hidden since they are only decoration so use aria-hidden="true" attribute on them.

  Aside from those, great work again on this one.

  Marked as helpful

  1

  ---

  

  Joaquin•410

  @JoadevyPosted about 3 years ago

  @pikamart Yaass, a pikamart comment it's garantee to a really good review of my work! Thanks a lot, it helps so much. In my previous project I used some of your tips about screen reader to add a hidden h1 element. Then about this I'll considerate all of your suggestions when I'll have time to check again this project. I remember that I had some problems with ARIA tags because I don't know about the correct use of it, I think that exist specific tags for each situation but I didn't find the appropiate. I will investigate more btw for future projects. Thanks for all

  1

  

  Joaquin•410

  @JoadevyPosted about 3 years ago

  @pikamart Hi, I'm writing it to teel you that I changed the code following all of your tips. I learned a lot about semantic HTML with your help. Now the code it's much clearer than before. However it has some design differences but it's okay I think. Maybe it could be better at handling breakpoints, I had issues dealing with this in SASS. Thanks you very much for your help!

  1