@Sloth247
Posted
Hi Michael,
Nice work! I like the animation effects and the fancy favicon😁
I have the following suggestions;
Issues in Accessibility Report
<div>
should not be inside<a>
tag. You can check from this site- In Navbar.js, you include
<li>
inside<NavLink>
however,<NavLink>
should be inside<li>
and<li>
must be directly under<ul>
. I think these will fix your issues in the report.
I am sorry, I maybe a bit of OCD, I think the font weight of h2 in Destination.js needs to be 400 and give more letter-spacing. Also please give more margins/paddings between the planet choice buttons.
I think the website will be much closer to the design if you utilise the figma file, as this is free+ challenge.
I appreciate if you give some feedback to my solution too!
Marked as helpful
@michagodfrey
Posted
@Sloth247 Thank you Yuko! I appreciate you taking the time to have a look.
Good pick up about the spacing and font weights. I did use the figma but missed those details. Shows why it's great to have a second set of eyes go over these things.
An I'll change those <ul> and <li> elements to <div> elements or even just delete them where possible. I knew it was more conventional to have the <NavLink> inside the <li> but didn't think it would be an accessibility issue. Now I know for next time.
I've seen your submission and I'll have a look again but tbh, I don't think I have much to add other than complementing you on a good job :)