@mattstuddert
Posted
Nice work, Natnael! Congrats on submitting this solution. Here are some thoughts after taking a look at your code:
- I'd say the solution needs a bit more work to get it looking close to the design. You can see in the design comparison that the boxes are a bit small, and the gaps a quite big vs the design. It would be a great exercise to try refining it to get it matching up more. Attention to detail is a key trait in good front-end developers, so it's good to practice.
- Your theme toggle uses a click listener on a
div
to trigger the action. I'd recommend avoiding setting click listeners on non-interactive elements, like thediv
element. These can't be accessed by anyone not using a mouse/trackpad to navigate the content, which is a bad practice. Instead, add click listeners to interactive elements likea
orbutton
. This will ensure the element is focusable and accessible by people not using a mouse/trackpad. As it's a toggle, I'd use abutton
in this instance. - You've left the
alt
attributes blank for allimg
elements, so screen reader users won't know what social media platform each stat is for. When you build interfaces, it's essential to think through how all users will consume the content as you could easily create inaccessible interfaces otherwise.
I hope these tips help! Keep up the great work! π
Marked as helpful