CODEWITHSUNDEEP
jquery
dezman
dezman

Reputation: 19378

.not(this) not working?

Fiddle

I've got this basic accordion which toggles a class of 'block'. Everything is working, except the .not(this) in $('.accordionInner').not(this).removeClass('block'); is not working, so you can never close all the accordions. I am sure it is a simple fix, but I don't know what I'm doing wrong.

HTML: 

<div class="accordionHeading">
    <p>Header</p>
</div>
<div class="accordionInner">
    <p>Inner</p>
</div>
<div class="accordionHeading">
    <p>Header</p>
</div>
<div class="accordionInner">
    <p>Inner</p>
</div>

CSS:

.accordionHeading {
    cursor: pointer;
    background: yellow;
}
.accordionInner {
    display: none;
}
.block {
    display: block !important;
}

JS:

 $('body').on('click', '.accordionHeading', function(){
  $('.accordionInner').not(this).removeClass('block');
  $(this).next('.accordionInner').toggleClass('block');
 });

Upvotes: 1

Views: 146

Answers (3)

cernunnos
cernunnos

Reputation: 2806

Updated your fiddle http://jsfiddle.net/tnXxF/4/

The problem was here: 

$('.accordionInner').not(this).removeClass('block');

You are selecting accordionInner, but the click event was on accordionHeading, so .not(this) could never work.

Solution in fiddle:

$('body').on('click', '.accordionHeading', function(){
  var targetInner = $(this).next('.accordionInner');
  $('.accordionInner').not(targetInner).removeClass('block');
  targetInner.toggleClass('block');
});

Edit: As mentioned in the comments, targetInner does not need to be wrapped in a jquery object again.

Upvotes: 2

STW
STW

Reputation: 46394

Since your .accordionHeading and .accordionInner elements are siblings (not parent:child) you shouldn't need to use the this on your header's click events.

Your Javascript is mostly correct, just change it to:

$('body').on('click', '.accordionHeading', function(){
  $(this).next('.accordionInner').toggleClass('block');
});

So now when an accordionHeading is clicked it will find the next occurance of an accordionInner and toggle the block class to trigger expansion or contraction.

Optionally you could also use the jQuery .toggle() method to do the show/hide for you without a custom class.

Upvotes: 0

Guffa
Guffa

Reputation: 700680

The event is on the heading, so this will never be any of the "inner" elements.

Use the reference that you get using next in the not also:

$('body').on('click', '.accordionHeading', function(){
  var inner = $(this).next('.accordionInner');
  $('.accordionInner').not(inner).removeClass('block');
  inner.toggleClass('block');
});

Upvotes: 7

Related Questions