Insure Landing Page collaboration with HTML and vanilla CSS and JS

Community feedback

• 

  Divine Obeten•2,415

  @Deevyn9

  Posted about 3 years ago

  This is really great... the social media icons at the footer are a bit jam packed on the mobile view, try spacing them a bit.

  Good luck

  11
• 

  PhoenixDev22•16,830

  @PhoenixDev22

  Posted about 3 years ago

  Hello Adam Elitzur,

  First of all, Congratulation on completing this challenge and awesome work on this one. I have few suggestions regarding your solution:

  1- The toggle element is added outside the vav class="nav-hamburger">, it would be better to be placed within the <nav> element. As it is, assistive technology user won’t announce the button related to the <nav>. And this is confusing and not good for the user.

  2- The button needs to have an aria-label attribute or an sr-only text that describes the button purpose.` For example, you can have: aria-label='Mobile Navigation Trigger' or 'Open Menu.’

  3- The <nav> is an important landmark of any website. It’s not recommended to hide the <nav> element. If you want to hide the navigation links in mobile screens , you should only hide the <ul> that wraps them.

  4- Adding aria-expanded attribute on the <button>, the user will be able to know that the button content is expanded or collapsed. At first, it initially has the“false”as a value then you use JavaScript to change the value.

  5- You should use aria-controls attribute on the toggle element, it should reference the id value of the <ul> element.

  6- Remember that a website-logo is one of the most meaningful images on a site so use proper alt for it. Use the website's name as an alternate text alt="insure".(Not empty)

  • Relating to the use of the <hr> above the <h2> , if you wish to draw a horizontal line, you should do so using appropriate CSS.

  • For future use , whenever you include <button> make sure to specify the type of the button to prevent it acting unexpectedly way, as its default type is submit.

  • For social links , the images are decorative, so the alt=”” should be empty with aria-hidden=”true” to be ignored by screen readers and to avoid redundancy and repetition.And the links must have aria-label or sr-only text tells where the link takes.

  • Use the <nav > landmark to wrap the footer navigation with aria-label=”secondary “ or aria-label=”footer”. A brief description of the purpose of the navigation, omitting the term "navigation", as the screen reader will read both the role and the contents of the label. The nav element in the header could use an aria-label="primary" or aria-label=”main” attribute on it. The reason for this is that, You should add the aria-label for a nav element if you are using the nav more than once on the page. This will make it unique.you can read more in MDN

  • If the image is decorative you set an empty alt to it with an aria-hidden=”true”, if the image is informative , you need to set a descriptive alternate text on the image.

  Aside these , Great work. Glad that you are improving and Keep collaborating

  00