Yair
Yair

Reputation: 192

document.click to remove class on page not behaving properly

I'm trying to create a global style of dropdowns which toggle between open and closed when their menu icon is clicked, but also close whenever anywhere else on the page is clicked. The way in which I'm opening or closing this dropdowns is by adding or removing a class called "open" to the parent of the dropdown.

The idea of that (to be more clear) is that the normal class of the dropdown has display: none; set on it, but if it's a descendant of something with the class "open", then it has display: block;

So, without further ado, here's what I have so far: "openable" is a class of 'parent' elements which can be clicked on to add the "open" class.

<script>
$(document).ready(function(){
    $('.openable').click(function(){
        if($(this).hasClass("open")){
            $(this).removeClass("open");
        }
        else{
            $(this).addClass("open");
        }
    });
});

On it's own that actually works fine - it acts as a decent enough toggle for the dropdowns. Of course, clicking anywhere else won't close the dropdowns.

My apparently non-functioning close script is:

<script>
$(document).ready(function(){
    $(document).click(function(event) {
        var clicked = $(event.target);
            if(clicked.hasClass(".open")){
        }
        else{
            if($(".open").length > 0){
                $(".open").each(function(){
                    $(this).removeClass("open");
                });
            }
        }
    });
});
</script>

With that script on the page, dropdowns cease to work altogether, and console isn't throwing up any errors for me to work off of.

Any better way of doing this?

Thanks

edit: html markup is something like

<li class="navItem dropdown openable">
<span><img src="img/settings.png"></span>
  <ul class="subNav hubDrop">
    <li>Nav item 1</li>
    <li>Nav item 2</li>
    <li>Nav item 3</li>
    <li>Nav item 4</li>
</ul>
        </li>

for each one. the li tag there is within another ul (remember, this is for dropdown menu essentially)

Upvotes: 0

Views: 1475

Answers (4)

nbrooks
nbrooks

Reputation: 18233

jsFiddle Demo - Since you haven't provided any HTML I mocked up some elements...

Update: You don't specify if more than one element can be 'open' at once; in your current solution they can be, so I kept that behavior. However, to limit it to one being open you can add $('.open').not(this).removeClass('open'); inside the .openable click handler.


Part One: Why not just use .toggleClass

$(document).ready( function() {
    $('.openable').click( function() {
        $(this).toggleClass("open");
    });
});

Part Two: No need for a second ready handler; in the first, add this:

$(document).click( function(e) {
    var clicked = $(e.target).closest('.openable');
    if ( clicked.length == 0 ) {
        $(".open").removeClass('open');
    }
});

Upvotes: 1

Stone
Stone

Reputation: 2668

How about making everything but the openable classed elements execute your click method?

var openable = $(".openable");
$("div, h2").not(openable).click(function(){
    $('.cont').slideUp().prev('.openable').removeClass('opened');
});

Upvotes: 0

Roko C. Buljan
Roko C. Buljan

Reputation: 205987

jsBin demo

just played around a bit (don't know your markup.)

<div>
<h2 class="openable">ICON 1</h2>
<div class="cont"></div>
</div>
$('.openable').next('.cont').hide();
$('.openable').click(function(e) {
    e.stopPropagation();
    $('.opened').removeClass('opened');
    var d = $(this).next('.cont');
    var visib = (d.is(':visible')) ?
       /*yes*/   d.slideUp() :
       /*no */  ($('.cont').slideUp()) (d.slideDown().prev('.openable').addClass('opened')) ;
});

$(document).click(function(){
    $('.cont:visible').slideUp().prev('.openable').removeClass('opened');
});

Upvotes: 1

KnowHowSolutions
KnowHowSolutions

Reputation: 680

I don't believe you can just do $(document).click(), it's not wrong but you never click the document itself, you click children of.

I have a very similar menu system and I capture an event this way:

 $('.navTab').mouseover(function (event) { navEvent($(this), event.type); });

Then remove all "open" and reapply "open" to the selected item.

I believe you don't want to capture all document click events. jQuery Event.target

Upvotes: 0

Related Questions