Why does my nav slide up and down unexpectedly?

This is only a few lines of code but I don’t see the problem!

Here is a test page:
http://phpby.me/test/

Here are the steps to recreate the bug:

  1. click a main nav link
  2. click a sub nav link to expose the sub sub nav
  3. click the same sub nav link to close the sub sub nav
  4. click the main nav to close the sub nav
  5. start over
  6. this time when you go to click a sub nav to expose a sub sub nav it will open then close by itself.

why does it close by itself? it should stay open.

Here’s the js. it’s a little sloppy, I’ve been hammering away at this issue and changed code so many times:


$(function(){

    $('a.sub').click(function(e){
        e.preventDefault();
        $(this).parent().toggleClass('showSub');


        // this element is display:none; by default, attach event after made display:block; by toggleClass above
        $('li.showSub > ul > li > a').click(function(e){
            e.preventDefault();


            // slide up all potentially open sub navs
            //$('.showSubSub').slideUp();
	
	    // add a class for styles, and slide it down
            //$(this).next().addClass('showSubSub').slideDown();

            if ( $(this).next().is(":visible") ) {
                $(this).next().slideUp();
            }

            if  ( $(this).next().is(":hidden") ){
                $(this).next().slideDown();
            }

        })

    })



})

Well, the only thing I can come up with is since the code to control the sub nav sliding


// this element is display:none; by default, attach event after made display:block; by toggleClass above
        $('li.showSub > ul > li > a').click(function(e){
            e.preventDefault();


            // slide up all potentially open sub navs
            //$('.showSubSub').slideUp();

	    // add a class for styles, and slide it down
            //$(this).next().addClass('showSubSub').slideDown();

            if ( $(this).next().is(":visible") ) {
                $(this).next().slideUp();
            }

            if  ( $(this).next().is(":hidden") ){
                $(this).next().slideDown();
            }

        })

is nested in side a main link, the event is being attached twice and throwing off the behavior i want.

The reason it’s nested is because the sub nav is display:none; by default and I was able to successfully attach an event to a display:none; element.

I think most of my issues with this will be solved if someone knows how i can attach events to an element that is display:none; by default.

Hi,

Can’t you do something simpler like this:


$(function(){
           $('a.sub').click(function(e){
            $(this).next('ul:visible').slideUp();
            $(this).next('ul:hidden').slideDown();
	   e.preventDefault();
        })								  
})



<div id="wrapper">
		<nav>
				<ul class="nav">
						<li class="showSub"><a href="#" class="sub">main nav</a>
								<ul>
										<li><a class="sub" href="#">sub nav</a>
												<ul>
														<li><a href="#">subsub</a></li>
														<li><a href="#">subsub</a></li>
												</ul>
										</li>
										<li><a class="sub" href="#">sub nav</a>
												<ul>
														<li><a href="#">subsub</a></li>
												</ul>
										</li>
										<li><a class="sub" href="#">sub nav</a>
												<ul>
														<li><a href="#">subsub</a></li>
												</ul>
										</li>
										<li><a class="sub" href="#">sub nav</a>
												<ul>
														<li><a href="#">subsub</a></li>
												</ul>
										</li>
								</ul>
						</li>
						<li><a href="#">--</a></li>
						<li><a href="#">--</a></li>
				</ul>
		</nav>
</div>

e.g. Full code:


<!DOCTYPE html>
<html class="no-js">
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
<title></title>
<meta name="description" content="">
<meta name="viewport" content="width=device-width">

<!-- <link rel="stylesheet" href="css/normalize.min.css"> -->

<link rel="stylesheet" href="testcss.css">
<style type="text/css">
/* ==========================================================================
   HTML5 Boilerplate styles - h5bp.com (generated via initializr.com)
   ========================================================================== */

html, button, input, select, textarea { color: #222; }
body {
	font-size: 1em;
	line-height: 1.4;
}

::-moz-selection {
 background: #b3d4fc;
 text-shadow: none;
}
::selection {
	background: #b3d4fc;
	text-shadow: none;
}
hr {
	display: block;
	height: 1px;
	border: 0;
	border-top: 1px solid #ccc;
	margin: 1em 0;
	padding: 0;
}
img { vertical-align: middle; }
fieldset {
	border: 0;
	margin: 0;
	padding: 0;
}
textarea { resize: vertical; }
.chromeframe {
	margin: 0.2em 0;
	background: #ccc;
	color: #000;
	padding: 0.2em 0;
}
/* ==========================================================================
   Author's custom styles
   ========================================================================== */
* {
	margin: 0;
	padding: 0;
}
body {
	background: #fff;
	color: #000;
	font-family: Avenir, 'Century gothic', Arial, sans-serif;
}
body.home {
	background: #000 url(../img/dagger_mobile_bg.jpg) no-repeat center;
	/*background: #000 url(../img/testbg.png) no-repeat center;*/
    color: #fff;
	padding: 10px 0 0 0;
}
a {
	outline: none;
	text-decoration: none;
}
h1 {
	background: #000;
	border-bottom: 1px solid #f00;
	color: #fff;
	font-size: 140%;
	font-weight: normal;
	margin: 0;
	padding: 10px 0;
	text-align: center;
}
h2 {
	text-align: left;
	text-indent: 10px;
}
#wrapper {
	margin: 0 0 45px; /* same height as fixed nav so as to not hide page content */
	text-align: center;
	width: 100%;
}
header a {
	background: #000 url(../img/dagger.png) 8px 3px no-repeat;
	float: left;
	height: 45px;
	width: 100px;
}
/*************************************/
/* main nav */
/*************************************/
nav {
	/*height: 32px;*/
    text-align: center;
	position: fixed;
	bottom: 0px;
	right: 0px;
	width: 100%;
	z-index:9999;
}
nav li {
	float: left;
	width: 33.3%;
}
nav a {
	background: -webkit-linear-gradient(top, #fb0303, #9f0202);
	background:    -moz-linear-gradient(top, #fb0303, #9f0202);
	border-right: 1px solid #fb0303;
	color: #fff;
	display: block;
	font-size: 14px;
	font-weight: bold;
	height: 45px;
	line-height: 45px;
	/*padding: 8px 0;*/
    text-transform: uppercase;
}
nav ul li:last-child a, nav > ul > li ul a { /* turn off right border on last nav link AND all sub/subsub nav links*/
    border: none; }
/*nav ul:first-child a:hover,*/
/*nav ul:first-child a:active{*/
/*    background: #9f0202;*/
/*}*/
/* turn off hover bg for sub nav */
nav > ul > li a:hover { background: #9f0202; }
nav > ul > li ul { text-align: left; }
/*************************************/
/* sub nav */
/* js applies showSub and showSubSub class on nav and nested nav click/touch */
/*************************************/

/* this rule hides the sub and sub-sub nav */
nav li ul { display: none; }
li.showSub > ul {
	position: absolute;
	/*top: -153px;*/
    bottom: 45px;
	left: 0;
	width: 100%;
}
li.showSub > ul li { width: 100%; }
li.showSub > ul li a {
	background: #c50202 url(../img/navArwDown.png) 96% 20px no-repeat;
	font-size: 18px;
	font-weight: bold;
	height: 50px;
	line-height: 50px;
	text-indent: 30px;
}
/*************************************/
/* sub sub nav */
/*************************************/
li.showSubSub > ul { /*display: block; ---------------*/
    /*position: absolute;*/
    /*top: -153px;*/
    /*left: 0;*/
    /*width: 100%; ------------------*/
}
/*li.showSubSub > ul li{*/
/*    width: 100%;*/
/*}*/
li.showSubSub > ul li a {
	background: #ad0101 url(../img/navArwRight.png) 96% 16px no-repeat;
	height: 45px;
	line-height: 45px;
	text-indent: 60px;
}
li.showSub > ul li ul li a {
	background: #ad0101 url(../img/navArwRight.png) 96% 16px no-repeat;
	font-size: 16px;
	height: 45px;
	line-height: 45px;
	text-indent: 60px;
}

</style>
</head>
<body class="home">
<div id="wrapper">
		<nav>
				<ul class="nav">
						<li class="showSub"><a href="#" class="sub">main nav</a>
								<ul>
										<li><a class="sub" href="#">sub nav</a>
												<ul>
														<li><a href="#">subsub</a></li>
														<li><a href="#">subsub</a></li>
												</ul>
										</li>
										<li><a class="sub" href="#">sub nav</a>
												<ul>
														<li><a href="#">subsub</a></li>
												</ul>
										</li>
										<li><a class="sub" href="#">sub nav</a>
												<ul>
														<li><a href="#">subsub</a></li>
												</ul>
										</li>
										<li><a class="sub" href="#">sub nav</a>
												<ul>
														<li><a href="#">subsub</a></li>
												</ul>
										</li>
								</ul>
						</li>
						<li><a href="#">--</a></li>
						<li><a href="#">--</a></li>
				</ul>
		</nav>
</div>
<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.10.1/jquery.min.js"></script> 
<script>window.jQuery || document.write('<script src="js/vendor/jquery-1.10.1.min.js"><\\/script>')</script> 

<!-- <script src="js/vendor/retina.js"></script> --> 
<script>
$(function(){
         	$('a.sub').click(function(e){
            $(this).next('ul:visible').slideUp();
            $(this).next('ul:hidden').slideDown();
												e.preventDefault();
        })								  
})

</script>
</body>
</html>


Bear in mind I’m a beginner at Jquery also. :slight_smile:

Every time you open your main sub navigation you’re recreating the click event for it, see the below which will fix this.

$(function() {

    $('a.sub').on('click', function(e) {
        e.preventDefault();
        $(this).parent().toggleClass('showSub');
    });

    // This element is hidden by default so we need to delegate a click event that watches for elements
    // that have a class of "showSub"
    $(document).on('click', '.showSub li > ul > li > a', function(e) {
        e.preventDefault();
        $(this).next().slideToggle();
    });

});

Hi Chris,

I think you’ve over-cooked the selector :slight_smile: Shouldn’t it be this:


  $(document).on('click', '.[B]showSub > ul > li > a[/B]', function(e) {

Eager to learn, is there something wrong with the approach I took here?


<script>
$(function(){
         $('a.sub').click(function(e){
         $(this).next('ul').slideToggle();
	e.preventDefault();
        })								  
})
</script>

It seems to work and allows for all elements to slide as required without having two different routines and works for the hidden elements. (I did add classes the the sub navs to identify them but could be done without the classes of course.).

I simply took his code and re-worked it into a delegated event, I noticed it was overdone but it been so warm here in Melbourne I just got it done as quick as possible :), as for your code the only thing I would change is the preventDefault line so it comes before the slide toggle otherwise if that fails the default action will take place.

Also drop down menu’s aren’t typically coded like this any more using classes and what not, from the trend I’ve seen going around data attributes are becoming a thing when it comes to managing DOM elements, of course this shouldn’t rule out classes all together but they are much easier to manage.

Ah ok thanks Chris that’s a good thing to remember:)

(Mind you if the slide failed then you may want the link to be actioned to allow navigation to a sub page as mentioned here..)

Also drop down menu’s aren’t typically coded like this any more using classes and what not, from the trend I’ve seen going around data attributes are becoming a thing when it comes to managing DOM elements, of course this shouldn’t rule out classes all together but they are much easier to manage.

Yes, I’ve seen data attributes used a lot (especially in bootstrap) but of course only valid in html5 (although I gather they will do no harm elsewhere).

I understand what that guy is getting at but not checking if for instance the variable my is not set is bad practice, allowing your code to fail on purpose is the wrong way to write an event as you have bound a custom action to it that should be carried out without any errors.

Sorry for stealing your thread @sessions; :slight_smile:

Thanks for the explanation Chris :slight_smile:

no problem! I learn a ton from you both! you can steal my thread anytime :slight_smile:

why aren’t drop down coded with classes anymore and what replaced classes?

Chris was talking about data attributes. It removes the dependence from a class being present and less likelihood that an author would remove or change a class that the JS needs to work.