A responsive landing page made with html, css and javscript

Community feedback

• 

  Jeremy Tjahjana•140

  @JeremyTjahjana

  Posted almost 3 years ago

  Hi Zakariyah 👋, Congratulations at completing this challenge! I have a few things to say about your code:

  1. First off all center the whole thing by

  body {
      height: auto;
      display: flex;
      justify-content: center;
      align-items: center;
      font-family: 'Fraunces', serif;
  }
  
  

  2. Rather than doing

              <header>
                  <div class="logo">
                      <img src="img/logo.svg" alt="" srcset="">
                  </div>
                  <div class="navlink">
                      <a href="">About</a>
                      <a href="">Service</a>
                      <a href="">Projects</a>
                      <a href="">contact</a>
                  </div>
              </header>
  
  
  <-- Try this instead -->
  
  
  <header>
      <div class="logo"><img src="img/logo.svg" alt="logo"></div>
      <nav>
          <ul>
              <li><a class="nav-items" href="/">About</a></li>
              <li><a class="nav-items" href="/">Service</a></li>
              <li><a class="nav-items" href="/">Projects</a></li>
              <li><a class="nav-items btn" href="/">Contact</a></li>
          </ul>
      </nav>
  </header>
  

  Usually when creating things such as a navbar, using a list and an anchor tag is much more common then an anchor inside a div. Also usually a header/navbar is outside of the main section/container.

  3.There are too many classes

  <div class="first-container">
  <div class="second-container">
  
  <-- Both of these divs have the same purpose and yet it has different classes, 
  which means there is a lot more CSS you have to write, Such as:-->
  
      .first-container img,
      .first-container .text-container {
          width: 50%;
      }
  
      .second-container img,
      .second-container .text-container {
          width: 50%;
      }
  
  

  4.Why not just put the id on the image

      <div id="hamburger">
        <img src="img/icon-hamburger.svg" alt="" srcset="">
      </div>
  

  5, Play around with the dev tool, also the images are way too big as a result the web is overflowing in the X axis, maybe try to add a max width in the container or the image it self.

  6, Add transition effects to your hover state https://developer.mozilla.org/en-US/docs/Web/CSS/transition.

  7, There are a few more problems that i didn't mention because i myself don't know how to fix it. I recommend watching CSS tutorials. There are many great channels out there but i personally recommend Kevin Powell's Youtube Channel > https://www.youtube.com/kepowob.

  Some more advice:

  • Try to set a time limit for yourself
  • Try to make your code more reusable by adding certain classes and careful not adding too much
  • Try to reduce the amount of code, because 500 line of css is way too much
  • It's ok to copy paste things such as html cards, nav bar, or footer as long as you understand what they do and how to play with them

  I'm sorry that's all from me, correct me if i said something inaccurate :3

  12

  Marked as helpful