Responsive intro section with dropdowns

Community feedback

• 

  Cats-n-coffee•670

  @Cats-n-coffeePosted almost 2 years ago

  Hi Kamil!

  Nice work! I'll give as much feedback as I can:

  • the button hover state makes the content jump because you are adding a border, which ads to the box (you're essentially adding pixels around the button). I can think of two easy solutions to get around that: 1- add the border to the button (before the hover), that way the border is already there when hovering and you only need to change the background-color, 2- use outline instead of border on hover since it will add this line inside the button and not outside. Word of caution with this second one, since outline is there by default on focus, it's actually an important state that should be modified carefully/be a distinct state in most cases (in your case you'll modify the color, which might make it the same as hover and that's not a good solution, ... but it's good to know it's possible).
  • your font doesn't seem to have loaded correctly, or you not have it imported anywhere, I can't find it in your HTML or Css.
  • around 1194px width your title is stuck at the very top of the page, which might be because of your layout and the height you're calculating. If you're using flexbox, you should be able maintain all elements within their space by using wrappers (divs) and setting the flex-direction and flex properties.
  • your chevrons (near the menu items) appear broken on my machine (using FF), and the dropdown menus are missing styles.
  • adding an click listener to the document and matching the event target is a valid way, but might not be the simplest way in your case. You could just declare all your elements at the top and place an event listener on each. Since your logic is the same across (adding the active class) you can create a single function for this. If you want to keep track of the current dropdown, take the variable out in the global scope, that way it can accessed everywhere, and you can make a separate function to remove the class. You could also have one event listener per menu item and loop through their child elements to place more event listeners if you'd like. That way you don't need to grab every single submenu item in a separate variable (so you'll do a forEach like you did).
  • in general, try avoiding having callbacks inside of callbacks like you're doing with document.querySelectorAll, Js can easily get you down this "callback hell" and your code will get difficult to deal with.

  Good job! Hope this helps, let me know if your have any questions!

  Marked as helpful

  0