CODEWITHSUNDEEP
javascriptjquery
cj333
cj333

Reputation: 2609

js number +1 problem

I need click div.toggle1,control slideup, slidedown the div#text1,

click div.toggle7,control slideup, slidedown the div#text7.

here is my code, also in http://jsfiddle.net/qHY8K/ my number +1 not work, need a help. thanks.

html

<div class="toggle1">click</div>
<div id="text1">text1</div>
<div class="toggle7">click</div>
<div id="text7">text2</div>

js code

jQuery(document).ready(function() {
counter = 0;
    for(i=1;i<11;i++){
        (function(i){
            counter = counter +1;
            $('.toggle'+counter).toggle(function(){
                $('#text'+counter).css('display','none');
            },
            function() {
                $('#text'+counter).css('display','block');
            });
        })(i);
    };  
});

Upvotes: 1

Views: 174

Answers (5)

T.J. Crowder
T.J. Crowder

Reputation: 1074979

Two solutions:

  1. With your HTML as quoted, you can just do this:

    jQuery(document).ready(function() {
    $("div.toggle").click(function() {
        $(this).next().toggle();
    });
});

    ...since the div you're toggling is the next adjacent div. Note also that I'm using jQuery's toggle function to toggle the visibility.

    But if it's possible that may change and you're defending against that, read on...

  2. In your JavaScript code, you're already doing something that makes it possible to avoid the counter entirely, as knitti pointed out. But the way you're doing it creates functions unnecessarily and by using the same name (i) for both your loop counter and the argument to your anonymous function, you're making it very difficult to read and maintain that code.

    So:

    jQuery(document).ready(function() {

    for(i=1;i<11;i++){
        makeToggler(i);
    }

    function makeToggler(index){
        $('.toggle'+index).click(function(){
            $('#text'+index).toggle();
        });
    }
});

    You can see how nice and clear that makes things, and in particular using a different name for the loop counter and the argument to makeToggler avoids confusion. And again, using jQuery's toggle function, no need for you to do it at the click level. (Also note that you don't put ; after the ending brace of a for statement.)

Upvotes: 3

Felix Kling
Felix Kling

Reputation: 816780

If your HTML has the structure as above, you could give all the toggleX elements the same class toggle and then all you have to do is:

$('.toggle').toggle(function(){
    $(this).next().css('display','none');
},
function() {
    $(this).next().css('display','block');
});

DEMO

Upvotes: 1

knittl
knittl

Reputation: 265547

why do you introduce a new variable counter? (you should use var counter = 0; if you do).

in your function you could simply use your copied loop variable i:

for(i=1;i<11;i++){
    (function(i){
        $('.toggle'+i).toggle(function(){
            $('#text'+i).css('display','none');
        },
        function() {
            $('#text'+i).css('display','block');
        });
    })(i);
};

Upvotes: 1

Shadow Wizzard
Shadow Wizzard

Reputation: 66388

You don't need hard coded loop.

Preserve your current HTML and have such jQuery code instead:

$("div[class^='toggle']").each(function() {
    var num = $(this).attr("class").replace("toggle", "");
    $(this).toggle(function(){
        $('#text' + num).css('display','none');
    },
    function() {
        $('#text' + num).css('display','block');
    });
});

This will iterate over all the <div> elements with class name starting with toggle and attach them the proper toggle function.

Updated jsFiddle: http://jsfiddle.net/qHY8K/5/

Upvotes: 2

ninjascript
ninjascript

Reputation: 1751

Lets simplify things a bit. One of the nice things about jQuery is that you can apply an event handler to many elements all at the same time. Start by adding a common classname to all of your 'toggle' divs:

HTML

<div class="toggle toggle1">click</div>
<div id="text1">text1</div>
<div class="toggle toggle7">click</div>
<div id="text7">text2</div>

Now you can use just one selector to target all of those divs. The rest is just a matter of pulling out the numeric difference in each 'toggle' div's classname:

JavaScript

jQuery(document).ready(function() {

    $('.toggle').toggle(off, on);

    function on() {
        var i = this.className.match(/[0-9]+/)[0];
        $('#text'+i).css('display','block');
    }

    function off() {
        var i = this.className.match(/[0-9]+/)[0];
        $('#text'+i).css('display','none');
    }
});

I've updated your jsFiddle project. Hopefully this works out for you: http://jsfiddle.net/ninjascript/qHY8K/3/

Upvotes: 5

Related Questions