Space Tourism Website built with React

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, really really nice work on this one. Desktop layout looks nice though I had to zoom out since you are using 1440px as the desktop breakpoint which is too big for lots of users including me who uses a 1366x768 monitor. The 1440px on the design is not related to the breakpoint, lowering it down would be really nice. Tablet layout looks fine and the mobile state as well but could use more bigger images for the planets, crew and the technology page.

  Some other suggestion on the site would be: GENERAL It would be great to have this markup:

  <header /> # contains the navbar
  <main /> # main content
  

  This way, all content of your page will be inside their own respective landmark element. Always separate related content into different containers or landmark.

  • Since you are not treating the website-logo as a link, you don't need it inside the nav element. nav usually contains navlinks and controls for the links as well.
  • Remember that a website-logo is one of the meaningful images on a site so using an img tag on this would be nice and maybe using something like alt="space tourism" on the img tag.
  • Those 4 links could have used a ul since those are "list" of links.
  • For each of the links, it would be nice to include a aria-hidden="true" on the div that contains the number like 00 so that when screen-reader reads the link name, it won't include those numbers to read.

  On the homepage of desktop layout:

  • Remove the height styling since it just make the layout not consistent. Try inspecting the layout in devtools at the bottom in the home page. You will see that the layout is destroyed.
  • SO, YOU WANT TO TRAVEL TO SPACEis only a single phrase so use a singleh1` to wrap it.
  • Also, using background-position: fixed hurts a site's performance since it create lags.

  DESTINATION

  • 01 PICK YOUR DESTINATION`` and other like this on the different section of the page could have been h1` since it gives information about what the user would expect from the section.
  • In general. When wrapping up a text-content, make sure that it is inside a meaningful element like p tag or heading tag and not using like div, span to wrap the text.
  • On those selections, you can wrap those 4 buttons inside of a ul so that user will get extra information on how many selections are in there.
  • button alone is not informative when they are selected. You can use an aria-live element that will announce something like, moon selected. Have a search about aria-live. If you like, you can implement a tabbed interface on that section since it suits it. Have a look at my solution on this one and check the tabs.
  • Every destination name could have used like an h2 since information below it is all about it.
  • Those 2 information at the very bottom could have been wrapped in a ul since those are "list" of information about the destination.
  • Also, a padding would be nice at the bottom of the layout so that user can scroll a bit more and prevents the content from touching the flooring of the screen.
  • It would be great if you use img tag rather than div and using the image as background-image since those destination images are meaningful since they are being described by the right-side content.

  CREW

  • Opening dev tools at bottom in desktop view, the person's image is pushed on the top of the page.
  • Person's name as well their position could be wrapped in a single h2 so that it will announce like "commaned douglas harley".
  • Those selections below should be using something like a button since those are interactive. Interactive components needs to use interactive elements, always. You can nest those as well in a ul since they are "list" of selections.
  • Again, you can use aria-live to announce what button has been selected.
  • Person's image should be using an img tag since they are meaningful and make sure to use the person's name as the alt for it.

  TECHNOLOGY AND CREW IS ALMOST THE SAME AS WELL SO SKIPPING IT:>

  MOBILE

  • Hamburger menu should be using a button since it is an interactive component.

  SUPPOSING BUTTON IS USED

  • The hamburger button should have a default attribute of aria-expanded="false" and it will be set to true when the users toggles it and vice-versa.
  • The hamburger button should have either aria-label attribute or sr-only text inside it which defines what the button does. You could use aria-label="navigation dropdown menu"

  Aside from those, great job again on this one.

  Marked as helpful

  2

  ---

  

  Carl Wicker•1,055

  @carlwickerPosted almost 3 years ago

  @martpika Hey ya Raymart. Thanks for taking the time to go through my code and make suggestions, it's really appreciated. I'll do the fixes today.

  1

  

  Carl Wicker•1,055

  @carlwickerPosted almost 3 years ago

  @pikapikamart just made the majority of the changes, I still need to change the media queries.

  0

  

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  @carlwicker Hey, great that you refactor some of the codes! Just looked at the site and looks great and the interactive components are now accessible for other users as well which is really nice.

  Just to point some things out on the updated site:

  • For the crew page, each person's images are still pushed up top. You can see this by opening dev tools at bottom.
  • For the button on the selections in the crew and technology page, have a visual indicator on the button :focus-visible state. You can do something like:

  .button:focus-visible {
      outline: 2px dashed white;
      outline-offset: 3px;
  }
  

  Marked as helpful

  0