Mobile first, using Scss and css grid for the first time

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. The layout both desktop and mobile is really good, the responsiveness as well is great.

  Some suggestions would be:

  • The website logo on the header tag, the a tag that wraps it needs a aria-label="Homepage" since usually, company logo leads to the homepage link.
  • The alt value for the header logo should only be alt="easybank". Avoid including words like "icon, image, logo" or any word that relates to "graphic" as a value in alt text. Assistive tech will handle those for you.
  • On the header personally, I would separate the company logo link from the nav. I would wrap the other 5 links inside the nav so that it will be more distinct to a user, especially in mobile state where the dropdown only contains the 5 links and not the company logo, that would create a confusion.
  • Nesting as well the 5 links inside ul element since it is a list of links, also this would guide a user on how many links there are , since screen readers will say how many items are there inside a list element.
  • Have a main element that will wrap the whole section, starting from the hero-section, going down to the articles section. This would create more structuring in your html.
  • On the article section, the 4 cards could be wrapped as well inside an article tag since it is an article right, that would make sense for other user. It would be:

  <a>
    <article>
      { contents inside here} 
    </article>
  </a>
  

  • The section element for the footer you might want to replace that with actual footer element.
  • The website logo in the footer, inside the svg you need to have a title element. This acts as a text for screen reader user, since the svg doesn't have any text inside it, a screen reader can't tell what it is. By giving a title it would know what it should say to the user. Adding like:

  <title>easybank </title>
  

  • Social media links as well could be wrapped inside ul since those are list of links.

  • Each a tag that wraps the social media should have aria-label on them, like aria-label="facebook" to the a tag that wraps the facebook icon. Also, it would be great to add aria-hidden="true" on the svg for the social media links.

  • The other 6 links as well could be wrapped by the ul element.

  • On mobile state, the hamburger menu toggler should have used button element instead of a tag since it is not really a link right, it is just a control, that is why button should be used. It should also have aria-label="hamburger dropdown toggler" so that user will know what this control is.

  • Add as well aria-expanded attribute on the hamburger toggler, this way, you will inform a screen reader user that it expanded something, hence it is the dropdown.

  Overall, this is great especially the layout. Great job.

  Marked as helpful

  0

  ---

  

  Jan•135

  @trostjanPosted about 3 years ago

  @pikamart wow, thanks for the comprehensive and well constructed feedback! It's so easy to forget that there is already the common markup built in html such as article and footer when you are used to just creating div for everything... I will need to be more carefull in my future projects!

  0
• 

  Conrad•950

  @ConradMcGrifterPosted about 3 years ago

  looks really nice, good job!

  as far as the mobile nav remaining opened when scaling up to desktop size, I think you could just write a media query to hide it

  Marked as helpful

  0

  ---

  

  Jan•135

  @trostjanPosted about 3 years ago

  @ConradMcGrifter that worked as a charm, thanks! :)

  0