Sass and Flexbox and Grid

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 and the responsiveness is really great. This is a really good looking layout.

  Some suggestions would be:

  • The website logo link in the header is much better not being inside the nav element, having it's own wrapper. The reason is that, nav should contain those links that will take user in different webpages right since it is navigation links, also, you may think, " the company link is a link to the homepage, why it is not included as well", usually in a nav element, links are inside ul element, this is because you want the user to be informed how many links inside it since screen readers tells users how many items are there in a list. If you included the website logo link in there, a user will be confused especially when they are using your website in a phone. Since navigation links are sometimes used in the dropdown, when they navigate in there, ( talking about your current structure) the screen reader will already say that the website log link is the navigation links, and when they go through the dropdown links, the screen reader won't notify them that those are navigation links, since it already said it at the beginning. So sorry if I may have confused you about my explanation. But you can look in frontendmentor header you will see how it should be structured.
  • Website logo link in the header should have aria-label="homepage", so that users will know where this link would take them.
  • The five links in the header should be inside nav that is inside a ul.
  • Always have a main element. This guides a user to properly navigate your website. The structure should be:

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

  • If an img does not add any content, use alt="". Do not leave it like that, use empty alt="".
  • The four title on the why choose easybank should use heading tags, since those are topics that are inside the why choose easybank section. Better notify a user why they would choose it.
  • The four articles could use an article element, wrapping it whole:

  <a>
    <article>
      { content is here }
    </article>
  
  </a>
  

  • The article titles should use heading tag.

  FOOTER

  • Website logo link a tag should use `aria-label="homepage".
  • Website logo img should use alt="easybank".
  • Social media links should be inside ul since those are list of social media links. Each a tag as well should have used aria-label. For example, the a tag that wraps the facebook should use alt="facebook".
  • The six links should be inside a ul element, that is inside a nav element.

  MOBILE

  • The hamburger menu should use a button and not a tag, since it is not a link, it is a control and button are used for those.
  • The hamburger button should also have aria-label="hamburger dropdown toggler" so that users will know what this button will do. It also should have aria-expanded that is being changed by javascript. This will notify a user if a dropdown has been expanded, you can search the usage of that in google.

  Aside from those, really great work on this.

  Marked as helpful

  0
• 

  ZaraAg•145

  @zaraag92Posted about 3 years ago

  Congrats on finishing this challenge.

  I just finished it too.

  0