Planets Facts Site Challenge - ReactJs Mobile First Solution

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Layout is really good in desktop, the mobile as well is good. The desktop layout could have taken a bit more from the media because right now, it goes to tablet layout at point 1230+ px, which I think desktop could be present at that. For the mobile layout, it would be great for the 3 tabs don't take full width. Since right now, it is being justify-content: space-between the 3 tabs are so far away from each other. Would be great to have a max-width on the flex parent.

  I won't look in the javascript since like what you said, you are going to refactor it.

  Still, some suggestions:

  • The fonts are not applying for the planet links, you might want to check that one out.
  • It would be great to have a header element that will wrap the upper section, instead of just using nav.
  • The website logo link should not have been inside the nav since it is not really a primary navigation link. The structure would be great if:

  <header>
    <div>
      <a href="/" aria-label="homepage earth"> The Planets </a>
    </div>
    <nav aria-label="primary">
      different planet  links
    </nav>
  </header>
  

  The planet links are the ones that is inside the nav because those are the navigational links. I added as well a aria-label="homepage earth" on the website logo link because it directs the user in the earth page, better inform user where it would take them.

  • There are multiple div after the h1 tag. Check it out.
  • Do not use tabindex='-1 to an interactive element, this renders it out and won't be accessible at all. Right now, you can't tab on those selections about the current planet.
  • It would be great as well for the selections of information about the planet, is implemented to be a tabbed interface. You might want to search that one out, it might be confusing to implement, but that will be a great accessibility feature on this challenge.
  • If you click on the surface then inspect the image being rendered, you will notice that everyone's alt is being used as alt="earth" which should not be, it should be the current planet name as the value being used. You might want to check that one as well.
  • The bottom part, since those are list of information, it would be great to have them inside a ul element.

  MOBILE

  • Hamburger menu should have been using button and not just div with img inside it. Using button will make it more accessible, right now only by click events are triggering it, you can't use keyboards or other assistive tech.

  Assuming that button is used on the hamburger

  • The button should have aria-label="menu dropdown toggler" so that users will know what this button does, it should also have a aria-expanded being set to it, triggering via javascript, helps user and informing them if it has expanded something.
  • The svg on the button should have aria-hidden="true" so that it won't be pick up by assistive tech.
  • It would be great to have a focus trap on the dropdown.
  • Also, on mobile, the fonts are not being rendered. Check that one.

  Aside from those, really great job.

  Marked as helpful

  1