jQuery / Javascript - loop

Question

I want to make it so when I click somewhere in my website, the background changes. I have three backgrounds, and I want to make a loop of them.

$(document).ready(function() {
 $('body').click((function(){
  return function()
  {
   if (counter == null) {
    var counter = 1;
   }
   if(counter == 3) {
     $(this).css("background-image","url(3.jpg)");
     $(this).css("background-position","10% 35%");
     var counter = null;
   }
   if(counter == 2) {
     $(this).css("background-image","url(2.jpg)");
     $(this).css("background-position","10% 35%");
     var counter = 3;
   }
   if(counter == 1) {
     $(this).css("background-image","url(1.jpg)");
     $(this).css("background-position","40% 35%");
     var counter = 2;
   }


  }
 })());

});

Why doesn't this work?

see what happens when you take out the '()' at the end of the 'click' function above. — Mike Webb
– Mike Webb, Commented Oct 15, 2010 at 18:10
Did you ever get this resolved successfully? Do you still need help with this? — jcolebrand
– jcolebrand, Commented Dec 14, 2010 at 4:18

Nick Craver · Accepted Answer · 2010-10-15 18:46:23Z

6

Your counter variable isn't scoped right, you need one counter variable. Overall though, why not let .toggle() manage this for you? Here's what it would look like:

$(function() {
  $('body').toggle(function(){
     $(this).css({"background-image":"url(1.jpg)", "background-position":"40% 35%"});
  }, function() {
     $(this).css({"background-image":"url(2.jpg)", "background-position":"10% 35%"});
  }, function() {
     $(this).css({"background-image":"url(3.jpg)", "background-position":"10% 35%"});
  });
});

Although the name and common usages suggest that .toggle() only takes 2 functions, it actually takes 2 or more and will cycle through them.

edited Oct 15, 2010 at 18:46

answered Oct 15, 2010 at 18:09

Nick Craver

631k138 gold badges1.3k silver badges1.2k bronze badges

Sign up to request clarification or add additional context in comments.

17 Comments

Šime Vidas Over a year ago

To much redundancy ... but I won't down-vote, that would be to harsh

Nick Craver Over a year ago

@Sime - What's reundant? You don't know for sure if he's using a background color, etc...so you can't just use "background", and the position isn't the same on all of them.

Aardvark Over a year ago

@Sime: The redundancy makes it much easier to understand for someone like the OP who's trying to learn JS. It would be a good exercise for the OP to try and remove some of the redundancy and would help him understand whats happening in the code...

Šime Vidas Over a year ago

You don't see the redundancy? Let's say that you want to also change the background-attachment or some other CSS property defined on the body element (and you have more than a dozen of images that you toggle). How hard would it be to set this new property on every click. Look at my answer, and you will see that I only set $(this).css() once.

Nick Craver Over a year ago

@Sime - If it gets more complicated then sure, use an array of objects and pass one of those to .css(), but as it stands your code is longer and more complicated than this. Don't over-complicate the simple things for some hypothetical scenario, address the question at hand :)

|

prodigitalson · Accepted Answer · 2010-10-15 18:08:36Z

1

this no longer refers to the body element, it refers to the anonymous function.

answered Oct 15, 2010 at 18:08

prodigitalson

60.6k10 gold badges106 silver badges118 bronze badges

Comments

jcolebrand · Accepted Answer · 2010-10-15 18:08:10Z

Does this code work?

var counter = 1;
$(document).ready(function() {
    $('body').click(function() {
        if (counter == null) {
            counter = 1;
        }
        if (counter == 3) {
            $(this).css("background-image", "url(3.jpg)");
            $(this).css("background-position", "10% 35%");
            counter = 1;
        }
        if (counter == 2) {
            $(this).css("background-image", "url(2.jpg)");
            $(this).css("background-position", "10% 35%");
            counter = 3;
        }
        if (counter == 1) {
            $(this).css("background-image", "url(1.jpg)");
            $(this).css("background-position", "40% 35%");
            counter = 2;
        }
    });
});

BrunoLM · Accepted Answer · 2010-10-15 18:11:12Z

0

Your function uses this which is refering to itself, not the element. This would fix it:

$('body').click((function(){
    var $this = $(this);
    return ... {
        $this // use $this instead of $(this)

Also, have a look on jQuery .toggle

answered Oct 15, 2010 at 18:11

BrunoLM

101k86 gold badges311 silver badges464 bronze badges

Comments

casablanca · Accepted Answer · 2010-10-15 18:18:08Z

0

Your counter declarations are strewn all over the place which makes it difficult to follow what's happening. Further, counter is declared local to the callback function, which means it loses its value every time the function executes.

Here's a simpler solution:

$(function() {  // this is equivalent to $(document).ready(...)
  var counter = 0;
  var images = [
    [ '1.jpg', '40% 35%' ],
    [ '2.jpg', '10% 35%' ],
    [ '3.jpg', '10% 35%' ]
  ];
  $('body').click(function() {
    $(this).css('background-image', 'url(' + images[counter][0] + ')');
    $(this).css('background-position', images[counter][1]);

    // increment counter, wrapping over to 0 when it reaches end of array
    counter = (counter + 1) % images.length;
  });
});

You can easily extend to this to any number of images by simply adding more entries to the images array.

edited Oct 15, 2010 at 18:18

answered Oct 15, 2010 at 18:11

casablanca

70.9k8 gold badges139 silver badges155 bronze badges

3 Comments

Nick Craver Over a year ago

The background position isn't the same for all the if statements ;)

casablanca Over a year ago

Oops, didn't notice that. Fixed using an array instead.

Šime Vidas Over a year ago

Wow, I also came up with that same approach where I place the data in an array. I did not copy it from you.

Šime Vidas · Accepted Answer · 2010-10-15 20:18:45Z

$(document).ready(function () {

    function changeBgImage() {
        var imgs = [
            ["1.jpg", "10% 35%"],
            ["2.jpg", "10% 35%"],
            ["3.jpg", "40% 35%"]
        ];
        var counter = 0;

        return function() {
            $(this).css({
                "backgroundImage": "url(" + imgs[counter][0] + ")",
                "backgroundPosition": imgs[counter][1]
            });
            counter += 1;
            if (counter === imgs.length) { counter = 0; }
        };      
    }

    $('body').click(changeBgImage());

});

Update:

OK, so here we have another solution. It is basically Nick's answer but without redundancy.

$(function () {    
    var imgs = [["1.jpg", "10% 35%"], ["2.jpg", "10% 35%"], ["3.jpg", "40% 35%"]];
    var i = 0;    
    $("body").click(function () {
        $(this).css({"background-image": "url(" + imgs[i][0] + ")", "background-position": imgs[i][1]});
        if (++i === imgs.length) { i = 0; }
    });    
});

Collectives™ on Stack Overflow

jQuery / Javascript - loop

6 Answers 6

17 Comments

Comments

Comments

Comments

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

17 Comments

Comments

Comments

Comments

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related