Insure Landing Page - SCSS, Grid, Flexbox, JS

Community feedback

• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted almost 4 years ago

  It's awesome to see you post another solution, Anna. Welcome back! 👋

  Your code looks great, scales up/down nicely, and it's awesome to see you using min-width media queries! The only suggestions I could find were in your HTML:

  • You've duplicated the navigation code when you don't need to. Instead of duplicating the HTML, you could style the nav differently for mobile or desktop layout inside a media query.
  • It's great to see you using a button element to trigger the mobile menu. If you want to take it up a notch and make it more accessible, I'd recommend checking out this Inclusive components article on menus & menu buttons. Add aria-* attributes will really enhance the accessibility of the navigation.
  • You've got 3x img elements with alt text of "illustration". These add no context to the content, so I'd recommend leaving the alt attributes blank, making screen readers skip them. As it is, screen readers would read out the word "illustration" before each of those 3 blocks, which doesn't add anything and actually makes the experience worse.

  I hope that helps. Let me know if you have any questions! 👍

  1

  ---

  

  Anna Leigh•5,135

  @brasspetalsPosted almost 4 years ago

  @mattstuddert Hi, Matt!

  I really appreciate you taking the time to look over the code! I knew having two separate navigations was likely…not the best solution 😅 - thanks for calling me out on it. I’ve gone back in and combined the navigations, and also changed the alt attributes as you suggested.

  Thank you for sharing the article on menu accessibility! I’m always uncertain on best practice when it comes to accessibility (and, honestly, just accessibility practices in general), and this is really helpful. I haven’t implemented the aria attributes yet, as I’ll need to give the article a few more reads, but I’m excited to start adding proper accessibility into my projects.

  Thanks again for your suggestions! 😄

  0

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted almost 4 years ago

  @brasspetals you're welcome! Accessibility can be tricky, but it's well worth learning about as it's such a critical piece to being a good front-end developer. The Inclusive Components site is an incredible resource for that, so you'll get a lot out of reading that 🙂

  1
• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted almost 4 years ago

  Hello, Anna! 👋

  I'm so happy to see you submit another solution! 😆 (I was wondering when you'd come back!) 😉

  Excellent job, as always, of course. Everything looks good and responds nicely! I like the fade in/out transition you added to the mobile navigation. 🤩

  Your code looks clean and organized, too. 👍

  Well, keep coding (and happy coding), as always! 😁

  1

  ---

  

  Anna Leigh•5,135

  @brasspetalsPosted almost 4 years ago

  @ApplePieGiraffe Hey, APG 👋 - it's definitely been a minute!

  Thanks for the welcome back. Hopefully today I'll have time to really get back into it and start providing feedback again. Hope you're well!

  Yeah, nothing crazy with this one (that's your department 😄 - I haven't commented yet, but I saw that entire extra Xmas theme on the to-do list challenge. Only you!). Thanks for the feedback again as always! I'm forever paranoid there'll be some major issue I completely blanked on.

  Happy coding to you too!

  1

  

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted almost 4 years ago

  @brasspetals

  Haha, I feel you! I'm often scared I'll spend so much time trying to add some cool feature but then I'll totally forget something else that's basic but very important! 😅 (Yet it seems to me that when I'm most worried about something going wrong, not much does and when I'm totally not thinking about it, a bunch of things go awry). Anyway! 😂

  👍

  1