@mattstuddert
Posted
Hey Elliot, congrats on completing this challenge...it's a big project!! Especially considering this was your first ever multi-page site, you've done an amazing job 🙂
Your solution matches up to the design really well. One small area I'd look to review the responsiveness would be the large phone / small tablet range. This is always a tricky area, so it's worth playing around with it a bit. At the moment, the content on large phones sizes before it breaks to the tablet layout looks a little too narrow for the device size. But that's definitely me nitpicking!
Your BEM looks good in general. As you mentioned, you strayed in a couple of places. But more practice will help to stay on track and refine your process.
Here are a couple of areas related to accessibility where you could make improvements:
- For the hamburger menu, instead of having
alt
text as "icon-hamburger" and "hamburger-close", you could have "Open menu" and "Close menu". Whenever you write alt text, think to yourself what would best describe the image or action you expect someone to take. Also, if you were to write words normally, you wouldn't typically use hyphens. So I'd avoid it in the alt text as well. - It's great to see you using a
button
element to trigger the mobile navigation. A lot of people placeclick
listeners on non-interactive elements likediv
elements, which is an inaccessible pattern. If you want to take it up a notch, you could look intoaria-*
attributes likearia-expanded
andaria-controls
to further help screen reader users. Here's a great article on the Inclusive Components blog with different strategies for creating accessible menus and buttons.
I hope those tips help. Like I said, awesome work on this challenge. I'm really happy you're enjoying PRO! 🎊