Add and remove extra class on click

Hi all

I have a snippet that works good, adding active to a sort of dropdown as shown below.

$('#nav').on('click', '#nav > li > a', function(event){
              event.stopPropagation();
              event.preventDefault();
              var openMenu = $('#nav .active').next('ul');
              if(event.handled !== true) {
                 openMenu.velocity("slideUp",  200, function() {});
                 if ($(this).hasClass('active')){
                  $(this).removeClass('active');
                } else{
                  $(this).next('ul').velocity("slideDown", 500, function() {});
                  $('#nav li a').removeClass('active');
                  $(this).addClass('active');
                }
                  event.handled = true;
              } else {
                  return false;
              }
      });

Now within the html, I have a span which has a class for plus which I’d like to change to minus once clicked.

<ul id="nav">
  <li><span class="plus"></span>
    <a href="#company-1">What we say</a>
    <ul>...

I generally need to add something like this into the mix:

$('#nav li span').removeClass('plus');
...
add minus

Could I add this to the existing if else or would I need a seperate if else statement?
Can anybody advise how I would add this extra snippet to the code?

Thanks, Barry

Immediately after the removeClass line, do the same thing with addClass(‘minus’).

$('#nav li span').addClass('minus').removeClass('plus')

This should do the trick. I am always adding a class first and than removing the classes I don’t need anymore, but it works as well as WolfShade has said.

Ok, nothing happens when I click once, then the minus gets added on close, but to all the links?

Code updated:

if ($(this).hasClass('active')){
                  $(this).removeClass('active');
                  $('#nav li span').addClass('minus');
                } else{
                  $(this).next('ul').velocity("slideDown", 500, function() {});
                  $('#nav li a').removeClass('active');
                  $('#nav li span').removeClass('plus');
                  $(this).addClass('active');
                }

Should I add $(this) to prevent applying to all links?
And what about the plus class already on the span in the html, do I remove this now?

Thanks, Barry

I’ve managed to get things working to a certain degree - http://jsfiddle.net/sn68s0te/1/

As you’ll see, the classes are adding to all the links instead of just one?

Just a couple of issues if anybody can help?

  1. Where do I add ($(this)) to prevent all the spans changing on a single click?
  2. If I open another tab after one is already open, the classes don’t get updated?
$('#nav').on('click', '#nav > li > a', function(event){
              event.stopPropagation();
              event.preventDefault();
              var openMenu = $('#nav .active').next('ul');
              if(event.handled !== true) {
                 openMenu.velocity("slideUp",  200, function() {});
                 if ($(this).hasClass('active')){
                  $(this).removeClass('active');
                  $('#nav li span').addClass('plus').removeClass('minus');
                } else{
                  $(this).next('ul').velocity("slideDown", 400, function() {});
                  $('#nav li a').removeClass('active');
                  $('#nav li span').addClass('minus');
                  $(this).addClass('active');
                }
                  event.handled = true;
              } else {
                  return false;
              }
      });

Please see fiddle link above.

Thanks, Barry

Hi,

I would approach this slightly differently.
This should do what you want:

  <ul id="nav">
    <li>
      <span class="plus">1</span>
      <a href="#company">What we say</a>
      <ul>
        <li>
          <p>This is what we say</p>
          <p>This is what we say</p>
          <p>This is what we say</p>
        </li>
      </ul>
    </li>
    <li>
      <span class="plus">2</span>
      <a href="#reviews">Reviews</a>
      <ul>
        <li class="box">
          <p>These are our reviews</p>
          <p>These are our reviews</p>
          <p>These are our reviews</p>
        </li>
      </ul>
    </li>
  </ul>
$.fn.slideUp = function(){
    this.velocity("slideUp",  400, function() {
        $(this).parent().removeClass("active");
        $(this).prev().prev().removeClass("minus").addClass("plus");
    });
};

$.fn.slideDown = function(){
    this.velocity("slideDown",  400, function() {
        $(this).parent().addClass("active");
        $(this).prev().prev().removeClass("plus").addClass("minus");
    });
};

$('#nav li').on('click', function (event) {
    var $panel = $(this),
        $panelContents = $panel.find("ul"),
        $activePanelContents = $(".active").find("ul");
    
    if($panel.hasClass("active")){
        $panelContents.slideUp();
    } else {
        $activePanelContents.slideUp();        
        $panelContents.slideDown();
    }
    
    event.stopPropagation();
});

Demo.

This is perfect :smiley:

I just needed to add ‘preventDefault’ below to stop the anchors showing in the url.

event.stopPropagation();
event.preventDefault();

Thank you!

Came across a small issue using this functionality with some slightly different code.
Been trying, though can’t get anything to work.

Ok, the script above we have:

$(this).prev().prev().removeClass("plus").addClass("minus");

Which targets targets the span:

<li>
<span class="plus">2</span>
      <a href="#reviews">Reviews</a>
      <ul>

The new code I have has a slightly different nesting:

<li>
        <h3>
            <a href="#">
                <span class="plus"></span>
                Today</a>
        </h3>
        <ul>

I did try.closest though didn’t seem to work for some reason:

$(this).closest("span").removeClass("plus").addClass("minus");

How do I target the span now nested inside other elements as above?
And is there a way to have the first ul open by default? (I did try this with CSS but the ul container didn’t close when I clicked another).

Thanks again, Barry

Can you post a fiddle of what you currently have?

Thanks Pullo

I’ve updated your last fiddle.
Everything is generally the same besides the updated html as I mentioned above.

http://jsfiddle.net/whskrhL6/6/

Barry

Ah ok, thanks.
Change the slideUp/slideDown functions like so:

$.fn.slideUp = function () {
    this.velocity("slideUp", 400, function () {
        $(this).parent().removeClass("active");
        $(this).prev().find("span").removeClass("minus").addClass("plus");
    });
};

$.fn.slideDown = function () {
        console.log(this);
    this.velocity("slideDown", 400, function () {
        $(this).parent().addClass("active");
        $(this).prev().find("span").removeClass("plus").addClass("minus");
    });
};

Magic :cool:
I was trying all different combinations, I didn’t realise we needed to keep .prev() (:

Good catch!

Ok, if you have a minute Pullo two other problems, though enhancements might be a better word.

Within our CSS also shown on the fiddle, we have:

ul ul {
    display: none;
}

This works good, though if I turn javascript off, nobody can see the content.
Is there a way we can add this declaration into the js so if people are not using javascript, display: none; won’t run and everybody can still see all the open tabs?

And the other enhancement, using the same approach above coding inside the javascript.
Can we leave one of the tabs open, for example the first tab?

Something like:

ul ul:first-child {
    display: block;
}

Maybe we could also add the class via javascript, so the minus and plus won’t show if again, javascript is turned off?
Meaning, our javascript code adds the class to the span, not the static css we added ourselves in the html

<span class="plus">1</span>

Thanks, Barry

I’ve managed to fix it.
It works, not sure if this is the best way?

ul ul {
    display: none;
}
$("ul ul").css("display","none");
ul ul:first-child {
    display: block;
}
$("ul > li:first-child > ul").css("display","block");
<span class="plus">1</span>
$("h3 a span").addClass("plus");

I also needed to add extra so the first block level worked with plus and minus and synced up with the others:

$("ul li:first-child h3 a span").removeClass("plus").addClass("minus");
$("ul li:first-child").addClass("active");

Note that all this code is above our existing code, out of the functions sitting just above.
Works ok, is this the right, best way to do it?

I’m now also getting a flash of content because we’re adding the css display: none via js.

Thanks, Barry

Hi Barry,

Could you post a fiddle of what you currently have?
That will make it much easier to offer an opinion.

Cool.

Updated fiddle: http://jsfiddle.net/whskrhL6/7/

Barry

Hi Barry,

What you posted seems ok, although I prefer .hide() and .show() to .css("display", "whatever");

I get the fouc, too.
Here is an article on how to minimize/prevent it.
Paul Irish’s technique, outlined at the bottom of the article, seems like it might be your best bet.

Hey Pullo

Cool, thanks for getting back and checking, I’ll take a look at this link see if I can get fouc resolved.

Thanks again for your help,
Barry

Hi

For the record, I fixed the fouc problem from the reference you gave, works great!
I was going to start a new thread though, just a small issue which is best if I add the updated fiddle.

I’ve changed the #nav to .nav so you can see the problem.

The problem is that I’ll be using this functionality on numerous parts of the site, and sometimes numerous times on the same page and all the markup is the same. So now when I click one link all the other instances of the carousel change.

My question.
How do we, or where do we add another $this so only the link clicked is affected?

http://jsfiddle.net/whskrhL6/8/

Thanks, Barry

Hi,

The problem was the way in which the script was getting the active panel contents to slideUp.

Change your event handler to this:

$('.nav li').on('click', function (event) {
    var $panel = $(this),
        $accordion = $panel.parent(),
        $panelContents = $panel.find("ul"),
        $activePanelContents = $accordion.find($(".active > ul"));

    if($panel.hasClass("active")){
        $panelContents.slideUp();
    } else {
        $activePanelContents.slideUp();        
        $panelContents.slideDown();
    }
    
    event.stopPropagation();
    event.preventDefault();
});

and all will be well :smile: