CODEWITHSUNDEEP
javascriptjqueryhtmlcss
EagL
EagL

Reputation: 77

Toggling background color of div

Title, my only problem is that when I've created all elements on my page, and clicked all of them, my page looks like a chess board.

I can only "toggle" the background color of half too. So it's not only that they don't change color on the first click, they don't change at all.

This is my Javascript:

for (var i = 0; i < 10; i++) {
    var itemContainer = document.createElement("div" + i);
    itemContainer.id = "div" + i;
    itemContainer.className = "item";
    itemContainer.innerHTML = "Hello!";

    for (let i = 0; i < 10; i++) {
        $('div' + i).click(function() {
            if (this.className == "item") {
                this.className = "itemselected";
            } else {
                this.className = "item";
            }
        });
    }

    document.getElementById("page").appendChild(itemContainer);
}

I made a JSFiddle for you who want it.

I've seen a few other questions about how to toggle the color of backgrounds, but none of them have the same problem as me.

Upvotes: 1

Views: 94

Answers (7)

EA-Lille
EA-Lille

Reputation: 561

You were close, missing "#" of id element

$('div' + i).click(function() {


$('#div' + i).click(function() {

and you have inserted the second loop inside first one

https://jsfiddle.net/snbtchph/

Upvotes: 1

AlexG
AlexG

Reputation: 5919

You inserted your second loop into the first one, every second i got skipped. And probably was able to change your divs up to i=18

JSFiddle

for (var i = 0; i < 10; i++) {
    var itemContainer = document.createElement("div" + i);
    itemContainer.id = "div" + i;
    itemContainer.className = "item";
    itemContainer.innerHTML = "Hello!";

    document.getElementById("page").appendChild(itemContainer);
}

for (let i = 0; i < 10; i++) {
    $('div' + i).click(function() {
        if (this.className == "item") {
            this.className = "itemselected";
        } else {
            this.className = "item";
        }
    });
}

Edit: You could simply put the content of your second loop into the first loop, to simplify your code a bit.

Upvotes: 3

&#214;zg&#252;r Ersil
&#214;zg&#252;r Ersil

Reputation: 7013

You don't need 2 loops try that

for (var i = 0; i < 10; i++) {
  var itemContainer = document.createElement("div");
  itemContainer.id = "div" + i;
  itemContainer.className = "item";
  itemContainer.innerHTML = "Hello!";
  document.getElementById("page").appendChild(itemContainer);

    $('#div' + i).click(function() {
    alert("here");
      if (this.className == "item") {
        this.className = "itemselected";
      } else {
        this.className = "item";
      }
    });
}

fiddle example

Upvotes: 1

Moumit
Moumit

Reputation: 9510

JSFIDDLE

for (var i = 0; i < 10; i++) {
  var itemContainer = document.createElement("div" + i);
  itemContainer.id = "div" + i;
  itemContainer.className = "item";
  itemContainer.innerHTML = "Hello!";

  $(itemContainer).click(function() {
    if (this.className == "item") {
      this.className = "itemselected";
    } else {
      this.className = "item";
    }
  });

  document.getElementById("page").appendChild(itemContainer);
}

Upvotes: 0

Ejaz
Ejaz

Reputation: 145

Why do you have 2 nested loops? try this

for (var i = 0; i < 10; i++) {
    var itemContainer = document.createElement("div" + i);
    itemContainer.id = "div" + i;
    itemContainer.className = "item";
    itemContainer.innerHTML = "Hello!";    

    document.getElementById("page").appendChild(itemContainer);
}

for (let i = 0; i < 10; i++) {
        $('div' + i).click(function() {
            if (this.className == "item") {
                this.className = "itemselected";
            } else {
                this.className = "item";
            }
        });
    }

Upvotes: 0

roberrrt-s
roberrrt-s

Reputation: 8210

There are a few problems with your code:

var itemContainer = document.createElement("div" + i);

  • Creating non-existant elements like <div1> is impossible, remove the iterator.

for (let i = 0; i < 10; i++) {

  • jQuery's .click() doesn't need a for loop, but adds the event listener to every case, this is not needed.

document.getElementById("page").appendChild(itemContainer);

  • Apply this directly in after the .innerHTML

In addition, you seem to randomly use ES6, jQuery, and VanillaJS through your entire codebase, I'd like to advise you to be consistant with how you write your applications.

I've updated your fiddle with the working changes. https://jsfiddle.net/xfr496p6/8/

Updated javascript:

for (var i = 0; i < 10; i++) {
    var itemContainer = document.createElement("div");
    itemContainer.id = "div" + i;
    itemContainer.className = "item";
    itemContainer.innerHTML = "Hello!" + i;
    document.getElementById("page").appendChild(itemContainer);
}

$('div').click(function() {
    if (this.className == "item") {
        this.className = "itemselected";
    } else {
        this.className = "item";
    }
});

Upvotes: 0

Zay Lau
Zay Lau

Reputation: 1864

Your selector at line 8 of your JavaScript is missing the # so the jQuery is looking for <div0>, <div1>, <div2>..., and, your line 2 of JavaScript is var itemContainer = document.createElement("div" + i); which actual creating elements div0, div1....

And since you are using jQuery , I have also revised some code to use it instead of native JavaScript: https://jsfiddle.net/xfr496p6/5/

I have also added css .item { display: inline-block; } to makes the elements placed in a row.

Upvotes: 0

Related Questions