CODEWITHSUNDEEP
javascriptjqueryhtml
Meow
Meow

Reputation: 19121

Why my addClass removeClass with event handler does not work as expected?

jQuery version 1.6

Goal: When a user clicks on down arrow (expander-down), show the advanced area. Like wise, when a user clicks on upper arrow (expander-up), hide the advanced area.

Problem: I thought it was simple to achieve with the code below. After I click on the expander-up, the advanced search area shows up as expected however when I click on expander-down, nothing happens.

FYI, I use image splite technique to swap the look of arrow by add/removing classes (expander-down, expander-up).

javascript:

$(".expander-down").unbind("click").click(function ()
{
   $(this).addClass("expander-up").removeClass("expander-down");
   $("#advanced-search").show();
});

$(".expander-up").unbind("click").click(function ()
{
   $(this).addClass("expander-down").removeClass("expander-up");
   $("#advanced-search").hide();
});

HTML

<a href='#'>
  <span class='expander-down'></span>
</a>

<div id='advanced-search'>advanced!</div>

Upvotes: 1

Views: 210

Answers (2)

T.J. Crowder
T.J. Crowder

Reputation: 1075219

Because the elements to which the click handlers are assigned are never changed. Even when the user clicks an expander, you leave the same handler (down or up) in place on it.

I would refactor it like this:

$(".expander-down, .expander-up").click(function() {
    var $this = $(this);
    $this.toggleClass("expander-down expander-up");
    $("#advanced-search").toggle($this.hasClass("expander-down"));
    return false; // Stops propagation and prevents default action
});

That responds to a click by toggling the down/up state, and then shows/hides the search box based on the resulting state. I think your code was showing it when the expander was down, so that's what I've done above, but that doesn't immediately make sense to me. If I'm misreading, just change expander-down in the hasClass to expander-up.

Upvotes: 3

Juan Guerrero
Juan Guerrero

Reputation: 613

The problem lies in the fact that .expander-up doesn't exist on pageload, meaning that event won't be ever bound to your span.

Unbinding all click functions isn't neccesary at all. Also, you can merge both functions in one:

$(".expander").click(function(e) {
   e.preventDefault(); // This avoids unwanted scrolling effect
   $(this).toggleClass("expander-up expander-down");
   $("#advanced-search").toggle();
});

toggleClass adds the css class to the element if it's not already set, and removes the class if it's set. toggle does the same but for element visibility.

Your html should be like:

<a href='#'>
  <span class='expander expander-down'></span>
</a>

<div id='advanced-search'>advanced!</div>

Upvotes: 1

Related Questions