Intro section with dropdown navigation

Community feedback

• 

  lieneil•560

  @NJVSPosted about 2 years ago

  Hi, Abdul. Congrats for completing the challenge.

  Regarding your dropdowns, I've notice that the <li id="features"> and <li id="company"> has the same click event. You can just assign a identical class name for your dropdown like this <li class="dropdown"> then select both of them in javascript then run a forEach() method then add click event(thats a lot of "then" LOL). Also, instead of toggling class for sub-menu, <a> and <i class="fa-solid">, I suggest to just toggle a class on their parent element.

  const dropdowns = document.querySelectorAll("li.dropdown");
  
  dropdowns.forEach(dropdown => {
    dropdown.addEventListener("click", function(event) {
      event.currentTarget.classList.toggle("active");
    });
  });
  

  .dropdown.active > a {
    font-weight: 700;
  }
  .dropdown.active > a i.fa-solid {
    transform: rotate(180deg);
  }
  .dropdown.active > .sub-menu {
    display: block;
  }
  

  Marked as helpful

  1

  ---

  

  Abdul rehman•650

  @abymaniPosted about 2 years ago

  @NJVS Thanks a lot. I have learned something new from you. you are awesome. :)

  1
• 

  Chyunjet•620

  @JetyunPosted about 2 years ago

  1. the Accessibility issue, you can put header tag to contain all the tag that is not in the main tag.
  2. for HTML, i can only comment on the img tag issue, where you no need to put unit on the width and height (the system knew that it is pixel)

  on design:

  3. I think you missed the active state on certain button (career, about, login, register and learn more)

  4. On the greyed out section in the mobile view, i actually used a div with position: absolute with background color of almost black paired with opacity setting to recreate the design. the div, you can just put above the big picture, no need to put anything in the div tag. Then just use JS to amend the display between none and block when you clicked the menu icon. remember to set the width and height of the div so that the grey area actually covered the same area in the design.

  Marked as helpful

  0
• 

  Abhilashpandey•270

  @freakyjonesPosted about 2 years ago

  Hi Abdul,

  congrats on completing the challenge

  I just saw your code, I would like to give some suggestions with your permission

  1. start using data attributes in your code, It will be much more efficient to change open and hide the nav and reduce the use of excessive javascript.
  2. try using a z-index while stacking multiple components in one place . Here is one of my favorite videos regarding z-index (https://www.youtube.com/watch?v=uS8l4YRXbaw).

  hope it helps, Thanks Happy coding :)

  0