jQuery.each() trouble

Question

I will try to keep this short. I'm trying to create boxes of text that hide or show when user clicks on a expand button. I'm using the toggle() method.

The markup is like this:

<span id="toggle0">+</span>
<div id="toggle0Container">
    blablabla...
<div>

<span id="toggle1">+</span>
<div id="toggle1Container">
    blablabla...
<div>

<span id="toggle2">+</span>
<div id="toggle2Container">
    blablabla...
<div>

// etc

The #toggle0 is supposed to toggle the #toggle0Container, the #toggle1 toggles the #toggle1Container and so on. This is all generated by PHP so there can be any number of these containers.

Here's the JQuery code:

$(document).ready(function() {
    // array of numbers, it's pretty redundant, there will never be more than 30 containers
    var arr = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9 , 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36];
    // hide all containers first
    jQuery.each(arr, function() {
        $('#toggle' + this + 'Container').hide();
    });
    // now the toggle buttons
    jQuery.each(arr, function() {
        $('#toggle' + this).click(function() {
            // this line is not working
            $('#toggle' + this + 'Container').slideToggle('slow');
            // and this just changes the toggle button from + to -
            if ($(this).html() == '+') {
                $(this).html('-');
            } else {
                $(this).html('+');
            }
        });
    });
});

It works except for the toggling part (I have added comment above the line that's not working). Where's the problem?

Answer 1

It's because "this" is now in a different scope, and is referring to the actual #toggleN DOM element, not one of the numbers in the array.

I would suggest you ditch the array of numbers, and use a different selector, something like:

$("div[id^=toggle][id$=container]").hide();

$("span[id^=toggle]").click(function() {
     $(this).find("span[id^=toggle][id$=container]").slideToggle("slow");
     // .. do your other html stuff
}

Just ditch those eachs and replace toggle selectors with those.

Answer 2

I would suggest putting a class on your toggle button (span) like "toggler" and your toggle container like "container" or something in order to simplify your javascript:

$(function() {
    // hide all containers first
    $(".container").hide();
    // now the toggle buttons
    $(".toggler").click(function() {
          var container = $("#" + $(this).attr("id") + "Container");
          container.slideToggle("slow");

          $(this).html(container.is(":visible") ? "-" : "+");
    });
});

Give that a shot.

Answer 3

As mgroves said, your scope is getting all jacked up by using each(). See this for more information.

Answer 4

EDIT

I like mgroves solution better for your particular use case, but I will leave my answer up which explains how you can accept the index and element as a parameter on the .each method

ORIGINAL ANSWER

As others have said, the context of this is not what you think it is at the point in which you are referencing it. Here is a great article by Remy Sharp titled "jQuery's this: demystified" that I recommend you read.

the .each function can accept two parameters that will greatly help you out and you won't need that array of numbers.

$('span[id^=toggle']').each(function(idx, elem) {

    // idx is the current index of the jQuery object
    $('#toggle' + idx).click(function() {

        // elem is the current element
        $(elem).next('#toggle' + idx + 'Container').slideToggle('slow');

        if ($(elem).html() == '+') {
            $(elem).html('-');
        } else {
            $(elem).html('+');
        }

    });

});

Answer 5

I was about to post exactly what Max did about using a class. I would only improve his suggestion with this easier selector to find the container:

$(function() {
  // hide all containers first
  $(".container").hide();
  // now the toggle buttons
  $(".toggler").click(function() {
    $(this).next().slideToggle("slow");

    if (container.is(":visible")) {
      $(this).html("-");
    }
    else {
      $(this).html("+");
    }
  });
});

If you can't easily get the classes on the spans, you can probably construct a similar selector using something like $('#SpanContainer>span') to select them by tag name, descending from their containing element.

Answer 6

I would follow mgroves's approach but if you really want to do the array thing, or just want to figure out how to use each() properly, here's how you should use it:

$(document).ready(function() {
    // array of numbers, it's pretty redundant, there will never be more than 30 containers
    var arr = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9 , 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36];
    // hide all containers first
    jQuery.each(arr, function(index, item) {
        $('#toggle' + item + 'Container').hide();
    });
    // now the toggle buttons
    jQuery.each(arr, function(index, item) {
        $('#toggle' + item).click(function() {
            // this line is not working
            $('#toggle' + item + 'Container').slideToggle('slow');
            // and this just changes the toggle button from + to -
            if ($(this).html() == '+') {
                $(this).html('-');
            } else {
                $(this).html('+');
            }
        });
    });
});

Answer 7

I recently wrote a fairly substantial script for behavior just like this. I took a slightly different route in that I chose to wrap all of the elements of a given hide/show element in a containing div. It allows you to have multiple "triggers" for one section if you need it, and you can add an arbitrary number of hide/show elements without worrying about numbering them. I would structure it something like this:

<div class="expSecWrapper">
 <div class="expSecTrigger">Insert Trigger Here</div>
 <div class="expSecBody">
  Content to hide/show
 </div>
</div>

And then the jQuery code would be something like this:

$(document).ready(function(){
 var targets = $(".expSecWrapper")
 targets.find(".expSecTrigger").click(function(){
  targets.find(".expSecBody").hide()
  $(this).parents(".expSecWrapper").eq(0).children(".expSecBody").toggle();
 })
})

You can add in the remaining relevant JS for your application.

It has a few limitations, such as the necessity that the hide/show body is a direct descendant of the wrapper. It can be applied to drop-downs, radio buttons, whatever. It's also nest-able so you can have multiple levels if hide/show.

One final thing is that you actually don't even need to use the .each iterator to assign that behavior. Something like .click() will assign behavior to an entire group of jQuery objects. As long as you use a selector that will grab all of your targets.

Sorry, had to edit that a couple times.

Answer 8

You should give your elements a class that can be searched on, then make use of next.

Make your markup something like this:

<span class="toggleButton">+</span>
<div class="toggleContainer">
   blablabla...
<div>

Now, your code to handle this becomes as simple as this:

$('.toggleButton').click(function() {
    $(this).next('.toggleContainer').slideToggle('slow');
});