@mattstuddert
Posted
Awesome work on this project, Brian! I hadn't realised Next.js added the placeholder="blur" attribute now as well, which is nice. We'll need to do some refactoring to add the placeholder attribute to the Frontend Mentor site now as well! 😆
My main piece of feedback would be to not put event listeners on non-interactive elements, like div elements. For example, the element you're using to bring up the gallery modal is a div. This means anyone using a keyboard or other assistive technologies to navigate the site might not be able to access that content.
The best thing to do is use anchors or buttons for interactive content. In this instance, a button would make sense as you're staying on the same page. This will ensure people navigating in other ways beyond a mouse/trackpad will be able to interact with your content.
I hope this helps. Keep up the great work! 👍
@brianlfarmerllc
Posted
@mattstuddert Its sure does. This is a really great project by the way. Even though I've submitted it Im working to improve on this one and accessibility is one thing I want to tackle so I will be updating those div's to buttons here in the near future.
@brianlfarmerllc
Posted
@mattstuddert And fixed....
@mattstuddert
Posted
@brianlfarmerllc nice work! One more small update that could be made is adding descriptive alt attributes to the previous/next slide buttons. Without alt attributes, screen readers will read out the file path. I just noticed this when navigating using a keyboard and VoiceOver.
It's great to hear you enjoyed the project! Taking some time to refine projects after submission is well worth it. Much better than moving onto the next challenge immediately!
