Animating outwards and JS toggle

Community feedback

• 

  

  Grace•32,130

  @grace-snow

  Posted over 4 years ago

  There are a few other suggestions I can make to help you in future projects too ☺

  1. Simplify your js. All your toggle function needs to do is add/remove one class to your nav element. E.g. Add a class like menu-is-open and then control everything visible under that by leveraging that class as a parent selector in your css like:

  .my-menu {
    // css properties for base menu styles and properties to make it hidden  here
  }  
  
  .menu-is-open .my-menu { 
    // css properties to make menu visible here 
  }
  

  2. Use a consistent naming convention for your class names. There's a lot of different hyphens and underlines at the moment. Pick a convention and stick to it (can be bootstrap or tailwind style, can be BEM, can be your own, can be others...)

  3. If you use alternate methods to show / hide that menu instead of the display property, you could animate it with a nice transition. Look up different ways to do this like visibility hidden, height and opacity 0, and/or positioning that menu off screen until its opened.

  4. Try not to set explicit heights (or widths) in css unless absolutely necessary. Use min-height or max- values instead to allow your content to grow and shrink as needed. All that can help you avoid overflow issues in future projects.

  Have fun playing with all that and keep learning. It looks like you're building up a good foundation here 👍

  21
• 

  

  Grace•32,130

  @grace-snow

  Posted over 4 years ago

  Hi Vtykuklys,

  This looks great! You just need a few changes to your html and js to make it accessible:

  • place the nav toggle icons inside a button. That way the nav will work with keyboard fully. I can see you've tried to add an enter trigger to it, but space bar is actually what you'd use for this and assistive tech users need more, like focus. Add a really obvious focus state using css and let the interactive html element <button> handle the rest automatically.

  • then remove that bit of js looking for keyup. Won't be needed.

  • then I would hide the open close buttons with aria-hidden="true" on both os them, and add the following to the button aria-label="Open menu" aria-controls="menu" aria-expanded="false" (with menu as the id on your ul)

  • then in your js function that's toggling the nav open and closed, you also set the attributes to aria-label="Close menu" aria-expanded="true" whenever the nav is open.

  • last accessibility improvement is to swap all your h5s for paragraph tags. It's really important for headings to go in order as assistive tech produces a document outline that is used to move around the page.

  Hope those are helpful suggestions for you ☺

  20
• 

  Abhik•4,820

  @abhik-b

  Posted over 4 years ago

  Hi @vytkuklys , Nice job on this challenge , I liked it very much. It is responsive and Navbar toggling works fine.

  Just a opinion ~ "Login" at the navbar for desktop is unreadable due to its color , giving it a dark color can fix

  Keep contributing these awesome solutions 🚀 & Happy Coding 😇

  10