KrispyK
KrispyK

Reputation: 235

How do I build up my Javascript/Jquery for many elements using a loop?

instead of repeating the same code over and over again in my js file with the only difference being the element names, I was hoping to build a loop that would build out my js.

I'm tryign to add toggle functions to some buttons on my page that change their colors and sets a value elsewhere on my page. Here is my code:

        var className;

        var idName;

        var i;

        for (i = 0; i < 11; i++) {
            className = ".feedbackq";
            idName = "#feedbackq";

            className = className + i.ToString();
            idName = idName + i.ToString();

            $(className).toggle(
            function () {
                $(className).each(function () {
                    $(className).css("background-color", "");
                });
                $(this).css("background-color", "red");
                var value = $(this).val();
                $(idName).val(value);
            },
            function () {
                $(this).css("background-color", "");
                $(idName).val("");
            });

        }

This is unfortunately not doing anything. When not in a loop, with hardcoded variable names, the code works, but I need this to be dynamic and constructed through a loop. The 11 count that is shown will eventually be a dynamic variable so I can't do hard coding....

Thanks for the help!

UPDATE: As requested, here is the not in the loop code:

                    $(".feedbackq0").toggle(
                    function () {
                        $(".feedbackq0").each(function () {
                            $(".feedbackq0").css("background-color", "");
                        });
                        $(this).css("background-color", "red");
                        var value = $(this).val();
                        $("#feedbackq0").val(value);
                    },
                    function () {
                        $(this).css("background-color", "");
                        $("#feedbackq0").val("");
                    });
                   $(".feedbackq1").toggle(
                    function () {
                        $(".feedbackq1").each(function () {
                            $(".feedbackq1").css("background-color", "");
                        });
                        $(this).css("background-color", "red");
                        var value = $(this).val();
                        $("#feedbackq1").val(value);
                    },
                    function () {
                        $(this).css("background-color", "");
                        $("#feedbackq1").val("");
                    });
                    $(".feedbackq2").toggle(
                    function () {
                        $(".feedbackq2").each(function () {
                            $(".feedbackq2").css("background-color", "");
                        });
                       $(this).css("background-color", "red");
                        var value = $(this).val();
                        $("#feedbackq2").val(value);
                    },
                    function () {
                        $(this).css("background-color", "");
                        $("#feedbackq2").val("");
                    });

Upvotes: 0

Views: 112

Answers (3)

hugomg
hugomg

Reputation: 69964

You probably fell victim to the closures-inside-for-loops bug. You need the code inside the loop to be in a separate function, so each iteration gets its own className variables instead of them sharing the variables.

You could do this by crating a named function or by using a jQuery iterator function with a callback instead of a for loop

var toggle_stuff = function(i){
   var className = ".feedbackq" + i; //The variables are local to just this iteration now
   var idName = "#feedbackq" + i; //No need to call toString explicitly.

   //And so on...
}

for(var i=0; i<11; i++){
   toggle_stuff(i)
}

Upvotes: 1

jfriend00
jfriend00

Reputation: 707826

One way to do this (without seeing your HTML for further simplifications) is to put the index number on the object before your event handlers using .data() so it can be retrieved later upon demand independent of the for loop index which will have run its course by then:

    var className, idName, i;

    for (i = 0; i < 11; i++) {
        className = ".feedbackq" + i;
        $(className).data("idval", i).toggle(
        function () {
            var idVal = $(this).data("idval");
            $(".feedbckq" + idVal).css("background-color", "");
            $(this).css("background-color", "red");
            var value = $(this).val();
            $("#feedbackq" + idVal).val(value);
        },
        function () {
            var idVal = $(this).data("idval");
            $(this).css("background-color", "");
            $("#feedbackq" + idVal).val("");
        });
    }

Note: I've made a bunch of other simplifications too:

  1. I declare multiple variables with one var statement.
  2. toString(i) is not needed to add a number onto the end of a string and you had it mispelled too (with the wrong capitalization)
  3. .each() is not needed to apply .css() to every item in a jQuery collection

I suspect that if we could see your HTML, we could significantly simplify this further as there are probably relationships between items that could be exploited to reduce code, but without the HTML we can't offer any advice on that.

Upvotes: 2

Alvin Wong
Alvin Wong

Reputation: 12440

I kinda suspect that you are calling the wrong function, should be .toString() instead of .ToString().

Note that JavaScript is case-sensitive.

But if I write the code anyway I will ignore the .toString() part and use the numeric value directly...

Upvotes: 0

Related Questions