React | Manage Landing Page

Community feedback

• 

  Travolgi 🍕•31,400

  @denieldenPosted almost 2 years ago

  Hello Steven, You have done a good work! 😁

  Some little tips to improve your code:

  • add header tag and wrap the navbar for improve the Accessibility
  • use article tag to the container a specifies independent, self-contained content like a card, forum post, news... not for the "slider" section of site
  • remove all unnecessary code, the less you write the better as well as being clearer: for example the article container of image with hero-image class
  • you can use picture tag to change image by resolution -> read here
  • add the image directly to the body background with css instead of using img tag with class .pill--lg
  • for floating menu use less useState with this logic: you first hook into the menu button's dom node via a ref and then use a state to define whether the panel is open or closed ref guide - read here

  const refNavBar = useRef(),
  	   [isOpen, setIsOpen] = useState(false),
  	   handleToggle = () => setIsOpen(!isOpen);
  

  Then you restructure the html of the menu like this by adding the nav toggle button and the control over the nav element:

  <header>
      <a>
           <svg logo />
      </a>
  
      <button
  	  className="nav-toggle"
  	  aria-controls={refNavBar}
  	  aria-expanded={isOpen}
  	  onClick={handleToggle}
      />
      
     <nav
  	ref={refNavBar}
  	data-visible={isOpen}
     >
  	<ul>
              ........
          </ul>
  
         <a href="#nogo" className="button">Get Started</a>
      </nav>
  </header>
  

  finally through the css/scss manage the menu icon and whether to make it appear or not:

  button.nav-toggle {
  	background: url(  icon-hamburger  );
  }
  button.nav-toggle[aria-expanded="true"] {
  	background: url(  icon-close    );
  }
  nav {
  	opacity: 0
  }
  nav[data-visible="true"] {
  	opacity: 1
  }
  
  @include u.breakpoint(lg) {
      button.nav-toggle { display: none; }
  }
  

  Keep learning how to code with your amazing solutions to challenges.

  Hope this help 😉 and Happy coding!

  Marked as helpful

  0

  ---

  

  Steven Portas•440

  @satropPosted almost 2 years ago

  @denielden Hey there. Thanks for some pointers but...

  <article> - tag, I originally had my <div class="section"> in section tags and this was throwing ADA warnings and I completely forgot about <articles>, nice easy fix.

  <picture> - didn't use this as it wasn't requested and the image was so small to start but yeah, as a standard your right <picture> - tags are the way to go.

  I disagree with adding the pill image as a background image, if you take a deeper look at my code I remade the pills via CSS, which will lower the page weight. And should there be a color change, it's easy to update.

  Dig what you're saying about the nav and I'll have a deeper dive into this with other builds, thanks for that.

  1

  

  Travolgi 🍕•31,400

  @denieldenPosted almost 2 years ago

  @satrop Pills via CSS is a great optimization, what I meant is that you can handle them in the css via :before or :after without affecting the html of additional nodes.

  What do you want me to dig more about nav ?

  Marked as helpful

  0

  

  Steven Portas•440

  @satropPosted almost 2 years ago

  @denielden oooh! Of course!! I didn’t think about doing that way. I like that approach a lot more. Thanks for that!

  And for sure you can dig into the nav if you have time. That found be great.

  1

  

  Travolgi 🍕•31,400

  @denieldenPosted almost 2 years ago

  @satrop You are welcome and keep it up :)

  0