Insure landing page

Community feedback

• 

  P

  Emmilie Estabillo•5,540

  @emestabilloPosted almost 4 years ago

  Hi Marta, great job! Site seems responsive. The roles are not necessary in this case because they are redundant. You already used the proper html tags to define landmarks in your document. Check out this [article] (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Navigation_Role) which clearly explains the purpose of the roles.

  A few more thoughts:

  • On larger screens, the entire .wrapper is left-aligned

  • The social media icons in the footer could use a tags. Hover states are also missing there and in the mobile nav.

  • Minor changes in font styling should get the page closer to the design

  Hope this helps :-)

  2

  ---

  

  Marta•630

  @martam90Posted almost 4 years ago

  Thank you, your feedback is very helpful. :) On that page there is a lot of different patterns that change whether it is mobile or desktop version. I checked your solution and I noticed that you put them as background-images. In my solution I have put them as img to html. Is there any best practice in similar cases? Thanks

  1

  

  P

  Emmilie Estabillo•5,540

  @emestabilloPosted almost 4 years ago

  @martam90 Yes. If the asset is purely decorative, like those patterns, best to place them as background images. If it adds meaning to the page, like a diagram or a photo that is directly related to the content, use the img tag.

  1
• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted almost 4 years ago

  Hello, Marta! 👋

  Always nice to see you complete another challenge! 😀 Well done on this one! Everything looks pretty good and responds well! 👍

  Two minor things I suggest are,

  • Adding a hover state to the social media icons near the bottom of the page.
  • Perhaps adding a max-width to the bottom-most button in the mobile navigation so that it isn't too wide when the layout first changes from desktop to tablet.

  Keep coding (and happy coding, too)! 😁

  1