responsive landing page - react js

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome job on this one. Layout in desktop looks great, though resizing, the images on the collections section is not really great to be occupying large size, maybe just use a fixed height on those so that it will be maintained.

  Some suggestions would be:

  • Do not use vh unit on height property. Inspect your layout in dev tools at the bottom, you will see that your header section's height got small. Instead, just use a fixed height using rem unit.
  • The alt for the site-logo-image should be only using alt="loopstudios", avoid using words that relates to "graphic" such as "logo, image, icon.." it is already an image, no need to describe it as one.
  • The links on the nav element would be better inside a ul element, since those are "list" of links.
  • IMMERSIVE EXPERIENCES THAT DELIVER should be a heading tag, since it is most likely to be a h1 since it is on the hero-section. Also, you don't need to use multiple span inside that text. You could have just used max-width property to limit the element's width, this will make the text go in the new row like the design.
  • Always have a main element that will wrap the main content of your webpage. A typical and preferred structure markup would look like:

  <header />
  <main />
  <footer />
  

  • The alt for the img after the hero-section should be using alt="" or maybe a meaningful element but not the one that you are using right now.
  • The text after the image should be a heading tag as well.
  • our creations should be a heading, since it gives insight about the current-section, which is the creation sections.
  • see all should be using anchor tag and not input. input are used when there are data that needs to be inserted upon by the user.
  • Each creation card could be inside an anchor tag a tag, since on a real site, this will be a link for each of the creation card.
  • I would use figure as well on each of the card, making the text inside it a figcaption

  FOOTER

  • Site logo-image should be using alt="loopstudios" as well.
  • The links inside the nav element again should be using ul element.
  • The social media links shouldn't be inside a nav element, since those aren't your navigational links for the website. They all should also be using ul element, "list" of social media links.
  • Since those are supposed links, use anchor a tag on them, with an aria-label attribute on the a tag or an sr-only text inside the a tag, which the text corresponds to the name of the social media that they are nesting, example on the facebook link:

  <a href="facebook" > # on aria-label="facebook"
    <span> facebook </span>
  </a>
  

  This way, users will know where this link would take them, same goes for the other links as well.

  MOBILE

  • Hamburger toggle should be using button and not div. When creating an interactive component, always interactive elements. ( supposing that the hamburger menu is not using button )
  • The button should also have a aria-label or an sr-only text, describing what the button does, like "navigation menu dropdown"`.
  • The button should also be using aria-expanded="false" as a default value when the button is not pressed, if it is pressed, it will be set to true using javascript.

  Aside from those, great job again on this one.

  1

  ---

  

  prakash•250

  @mewebbiePosted about 3 years ago

  @pikamart all your valuable points are noted duly.

  I would suggest you to refresh the page after you resize the window to view smaller screen layout. The images in collection are <img/> and not set by background hence I have writen some vannila js in index.js to alter the source of images when screen size is small.

  0
• 

  Michal•665

  @mbart13Posted about 3 years ago

  why are you adding eventListeners to index.js like it was vanilla javascript? :O

  you never do that in react project, it has its own event

  0

  ---

  

  prakash•250

  @mewebbiePosted about 3 years ago

  @mbart13 Thanks for the comment. I'm just getting started with react and didn't know about event listeners in react.

  0