JS toggle button not working

Question

I have created a Navigation bar and that contain Toggle Button, I'm facing a problem with Space between List Items and The problem has been solved by this https://stackoverflow.com/a/5256640/9947640 method.Unfortunately, its create another big problem, The problem is my Toggle Button not working.
I don't understand what I'm doing wrong and what should I do?

$(document).ready(function(){
  $(".menu-icon").on("click",function(){
    $("nav ul").toggleClass("showing");
  });
});

//Scrolling Effect
$(window).on("scroll", function(){
  if ($(window).scrollTop()) {
    $('nav').addClass('black');
  }else{
    $('nav').removeClass('black');
  }
});

html, body {
  margin: 0;
  padding: 0;
  width: 100%;
}
 
.content {
  width: 90%;
  margin: 4em auto;
  font-size: 20px;
  line-height: 30px;
  text-align: justify;
}
 
.logo {
    position: absolute;
    float: left;
    margin: 16px 46px;
    color: #333;
    font-weight: bold;
    font-size: 20px;
    letter-spacing: 2px;
}
nav{
 position: fixed;
 width: 100%;
}

nav ul{
 list-style-type: none;
 background: rgba(0,0,0,0);
 overflow: hidden;
 color: #fff;
 padding: 0;
 text-align: center;
 margin:0;
 transition: 1.1s;
 font-size:0;
}
nav.black ul{
 background-color: #000000;
}
nav ul li{
 display: inline-block;
 font-size:14px;
 padding: 20px;
 background-color:#ffab00;
}
nav ul li a{
 text-decoration: none;
 color: #333333;
 font-size: 20px;
}

.menu-icon{
 width: 100%;
 background-color: #000000;
 text-align: right;
 box-sizing: border-box;
 padding: 15px 2px;
 cursor: pointer;
 color: #fff;
 display: none;
}

nav #right-menu{
 position: absolute;
 right: 0;
}
nav  #right-menu ul{
 text-align: right;
}
@media(max-width: 768px){
 .logo{
  position: fixed;
  top: 0;
  margin-top: 16px;
 }
 nav ul{
  max-height: 0px;
  background-color: #000000;
 }
 nav.black ul{
  background: #000000;
 }
 .showing {
  max-height: 20em;
 }
 nav ul li{
  box-sizing: border-box;
  width: 100%;
  padding: 15px 0;
  text-align: center;
 }
 .menu-icon{
  display: block;
  padding-right: 30px;
 }

 nav #right-menu{
  position: relative;
 }
}

<link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.0.13/css/all.css" integrity="sha384-DNOHZ68U8hZfKXOrtjWvjxusGo9WQnrNx2sqG0tfsghAvtVlRW3tvkXWZh58N9jp" crossorigin="anonymous">
<script  src="https://code.jquery.com/jquery-3.3.1.js"></script>
<header>
 <nav>
  <div class="menu-icon">
   <i class="fas fa-bars fa-2x"></i>
  </div>
  <div class="logo">
   Logo
  </div>
  <div class="menu" id="right-menu">
 <ul >
   <li><a href="#">HOME</a></li>
   <li><a href="#">ABOUT</a></li>
 </ul>
  </div>
  <div class="menu">
   <ul>
    <li><a href="#">HOME</a></li>
    <li><a href="#">ABOUT</a></li>
    <li><a href="#">SERVICE</a></li>
   </ul>
  </div>
 </nav><!----- nav ------>
</header><!----- header ------>

Chaska · Answer 1 · 2018-07-25T07:44:19.687

nav ul.showing is missing in css?

I added nav ul.showing { display: block; } and display: none; in nav ul to get it works...

Edit: also remove overflow: hidden in nav ul

$(document).ready(function(){
  $(".menu-icon").on("click",function(){
    $("nav ul").toggleClass("showing");
  });
});

//Scrolling Effect
$(window).on("scroll", function(){
  if ($(window).scrollTop()) {
    $('nav').addClass('black');
  }else{
    $('nav').removeClass('black');
  }
});

html, body {
  margin: 0;
  padding: 0;
  width: 100%;
}
 
.content {
  width: 90%;
  margin: 4em auto;
  font-size: 20px;
  line-height: 30px;
  text-align: justify;
}
 
.logo {
    position: absolute;
    float: left;
    margin: 16px 46px;
    color: #333;
    font-weight: bold;
    font-size: 20px;
    letter-spacing: 2px;
}
nav{
 position: fixed;
 width: 100%;
}

nav ul.showing {
  display: block;
}
nav ul{
 list-style-type: none;
 background: rgba(0,0,0,0);
 color: #fff;
 padding: 0;
 text-align: center;
 margin:0;
 transition: 1.1s;
 font-size:0;
}
nav.black ul{
 background-color: #000000;
}
nav ul li{
 display: inline-block;
 font-size:14px;
 padding: 20px;
 background-color:#ffab00;
}
nav ul li a{
 text-decoration: none;
 color: #333333;
 font-size: 20px;
}

.menu-icon{
 width: 100%;
 background-color: #000000;
 text-align: right;
 box-sizing: border-box;
 padding: 15px 2px;
 cursor: pointer;
 color: #fff;
 display: none;
}

nav #right-menu{
 position: absolute;
 right: 0;
}
nav  #right-menu ul{
 text-align: right;
}
@media(max-width: 768px){
 .logo{
  position: fixed;
  top: 0;
  margin-top: 16px;
 }
 nav ul{
  display: none;
  max-height: 0px;
  background-color: #000000;
 }
 nav.black ul{
  background: #000000;
 }
 .showing {
  max-height: 20em;
 }
 nav ul li{
  box-sizing: border-box;
  width: 100%;
  padding: 15px 0;
  text-align: center;
 }
 .menu-icon{
  display: block;
  padding-right: 30px;
 }

 nav #right-menu{
  position: relative;
 }
}

<link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.0.13/css/all.css" integrity="sha384-DNOHZ68U8hZfKXOrtjWvjxusGo9WQnrNx2sqG0tfsghAvtVlRW3tvkXWZh58N9jp" crossorigin="anonymous">
<script  src="https://code.jquery.com/jquery-3.3.1.js"></script>
<header>
 <nav>
  <div class="menu-icon">
   <i class="fas fa-bars fa-2x"></i>
  </div>
  <div class="logo">
   Logo
  </div>
  <div class="menu" id="right-menu">
 <ul >
   <li><a href="#">HOME</a></li>
   <li><a href="#">ABOUT</a></li>
 </ul>
  </div>
  <div class="menu">
   <ul>
    <li><a href="#">HOME</a></li>
    <li><a href="#">ABOUT</a></li>
    <li><a href="#">SERVICE</a></li>
   </ul>
  </div>
 </nav><!----- nav ------>
</header><!----- header ------>

its petty correct but not accurate, see your answer in full screen, `display:none` hide all of menu in above of 768px screen..... — Hasan, Jul 25 '18 at 07:40
Moved the `display: none;` into `@media(max-width: 768px)` to show the menu in desktop screen. Try hiding the mobile nav by yourself? — Chaska, Jul 25 '18 at 07:47

score 1 · Accepted Answer · answered Jul 25 '18 at 07:24

1

hasan, max-height of showing class is wrong. set it in px. for example

@media (max-width: 768px)
.showing {
    max-height: 400px;
}

answered Jul 25 '18 at 07:24

Alpa Bhanderi

41
2

@Hasan Be careful, just an advice: this solution is not adaptive at all! What about if you put another li? And if tomorrow 400px is not enoght? You have to change max-height every time you will change your menu. BTW, good luck! ;) – ReSedano Jul 25 '18 at 08:14
Yes you're right but all answer working almost same, what should I ? – Hasan Jul 25 '18 at 08:37
Be adaptive is not "almost same". For example, removing that 400px allows you to add items without changing CSS every time. This is the reason: do not make the content depending on your design. The contents always command and the design must adapt itself and make the contents always beautiful and legible. This is what I think. :) – ReSedano Jul 25 '18 at 09:23

ReSedano · Answer 3 · 2018-07-25T07:55:22.800

Simply reset max-height on .showing

$(document).ready(function(){
  $(".menu-icon").on("click",function(){
    $("nav ul").toggleClass("showing");
  });
});

//Scrolling Effect
$(window).on("scroll", function(){
  if ($(window).scrollTop()) {
    $('nav').addClass('black');
  }else{
    $('nav').removeClass('black');
  }
});

html, body {
  margin: 0;
  padding: 0;
  width: 100%;
}
 
.content {
  width: 90%;
  margin: 4em auto;
  font-size: 20px;
  line-height: 30px;
  text-align: justify;
}
 
.logo {
    position: absolute;
    float: left;
    margin: 16px 46px;
    color: #333;
    font-weight: bold;
    font-size: 20px;
    letter-spacing: 2px;
}
nav{
    position: fixed;
    width: 100%;
}

nav ul{
    list-style-type: none;
    background: rgba(0,0,0,0);
    overflow: hidden;
    color: #fff;
    padding: 0;
    text-align: center;
    margin:0;
    transition: 1.1s;
    font-size:0;
}
nav.black ul{
    background-color: #000000;
}
nav ul li{
    display: inline-block;
    font-size:14px;
    padding: 20px;
    background-color:#ffab00;
}
nav ul li a{
    text-decoration: none;
    color: #333333;
    font-size: 20px;
}

.menu-icon{
    width: 100%;
    background-color: #000000;
    text-align: right;
    box-sizing: border-box;
    padding: 15px 2px;
    cursor: pointer;
    color: #fff;
    display: none;
}

nav #right-menu{
    position: absolute;
    right: 0;
}
nav  #right-menu ul{
    text-align: right;
}
@media(max-width: 768px){
    .logo{
        position: fixed;
        top: 0;
        margin-top: 16px;
    }
    nav ul{
        max-height: 0px;
        background-color: #000000;
    }
    nav.black ul{
        background: #000000;
    }
    .showing {
        max-height: none; /* Change this */
    }
    nav ul li{
        box-sizing: border-box;
        width: 100%;
        padding: 15px 0;
        text-align: center;
    }
    .menu-icon{
        display: block;
        padding-right: 30px;
    }

    nav #right-menu{
        position: relative;
    }
}

<link rel="stylesheet" href="https://use.fontawesome.com/releases/v5.0.13/css/all.css" integrity="sha384-DNOHZ68U8hZfKXOrtjWvjxusGo9WQnrNx2sqG0tfsghAvtVlRW3tvkXWZh58N9jp" crossorigin="anonymous">
<script  src="https://code.jquery.com/jquery-3.3.1.js"></script>
<header>
    <nav>
        <div class="menu-icon">
            <i class="fas fa-bars fa-2x"></i>
        </div>
        <div class="logo">
            Logo
        </div>
  <div class="menu" id="right-menu">
    <ul >
      <li><a href="#">HOME</a></li>
      <li><a href="#">ABOUT</a></li>
    </ul>
  </div>
        <div class="menu">
            <ul>
                <li><a href="#">HOME</a></li>
                <li><a href="#">ABOUT</a></li>
                <li><a href="#">SERVICE</a></li>
            </ul>
        </div>
    </nav><!----- nav ------>
</header><!----- header ------>

JS toggle button not working

3 Answers3