adding onClick event to JavaScript for loop

Question

What I am trying to do is to create an App in which there are 26 boxes with ids case0, case1, case2 and so on. When clicked, these should add class 'black' to respective item0, item1, item2 and so on. My current code looks like this:

let num = [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];
let guess = [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];
let i,x;
let arrayShuffle = function(arr){
    let newPos,temp;

    for (let i = arr.length-1; i > 0; i--) {
        newPos=Math.floor(Math.random()*i+1);
        temp=arr[i],
        arr[i]=arr[newPos];
        arr[newPos]=temp;
    }
    return arr;

    };
let numRandom = arrayShuffle(guess);
function suitCase(){
    for (x = 0; x < num.length; x++) {
    document.getElementById("casePanel").innerHTML+="<a href=\"#\" id=\"case"+numRandom[x]+"\"><div class=\"case\"><span>"+ num[x] + "</span></div></a>"; 
    document.getElementById('case'+numRandom[x]).onclick=function(){clickCase();}
    }
};

function clickCase(){
    for(x = 0; x < num.length; x++){
    document.getElementById('item'+[x]).classList.add('black');
    }
}

With this code I am having two problems. First class 'black' is applied to all items not single and only after I click the last box. Rest of the 25 boxes don't do anything when clicked. And secondly, only if I use variable numRandom[x], the whole array is visible but if I use simply [x] or num[x], just the first two elements of array that is 0 and 1 are visible. Can someone please help me with this and suggest a better code?

Here is my html in case someone needs it for reference:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>The Suitcase</title>

    <link rel="stylesheet" href="style.css">
    <script src="script.js"></script>
</head>
<body onload="suitCase()">
<div class="container">
    <header>
        <h1>This is a sample title for this nice suitcase casino</h1>
    </header>

    <main class="wrap">
        <!-- Left price panel -->
        <div class="left-panel">
            <ul>
            <?php
                $priceL = [".01","1","5","10","25","50","75","100","200","300","400","500","750"];

                for($i=0;$i<count($priceL);$i++){
                    echo "<li id=\"item$i\">$".$priceL[$i],"</li>";
                }
            ?>
            </ul>
        </div>
        <!-- ./Left price panel -->

        <div class="case-panel" id="casePanel">

        </div>

        <!-- Right price panel -->
        <div class="right-panel">
            <ul>
            <?php
                $priceR = ["1000","5000","10,000","25,000","50,000","75,000","100,000","200,000","300,000","400,000","500,000","750,000","1,000,000"];
                $x = 13;
                for($i=0;$i<count($priceR);$i++){
                    echo "<li id=\"item$x\">$".$priceR[$i],"</li>";
                    $x++;
                }
            ?>
            </ul>
        </div>
        <!-- ./Right price panel -->
    </main>

    <footer></footer>
</div>

</body>
</html>

Pretty sure the getElementById() only retrieves the first one. I'd do $('#item').each(function() { *the code* }); — Joel Hager
– Joel Hager, Commented May 15, 2020 at 1:49
Yes case0, case1, case2 when clicked should change css class of item0, item1 and item2 respectively — Waleed Ahmad
– Waleed Ahmad, Commented May 15, 2020 at 1:54
It depends on what you're doing. The .each() function will go over every element that matches the selector. The reason it's only affecting the 1st one is the selector is just grabbing the first one. — Joel Hager
– Joel Hager, Commented May 15, 2020 at 1:54

Barmar · Accepted Answer · 2020-05-15 02:16:34Z

2

The onclick function should get the ID of the element that was clicked. Then replace case with item to get the corresponding item, and pass that as an argument to clickCase().

The reason only the last item is clickable is because you're rewriting document.getElementById("casePanel").innerHTML each time through the loop. This recreates all the elements, so all the previous elements no longer have their click handlers.

You can use insertAdjacentHTML() to add HTML to an element without reparsing what it already contains.

let num = [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];
let guess = [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];
let i, x;
let arrayShuffle = function(arr) {
  let newPos, temp;
  for (let i = arr.length - 1; i > 0; i--) {
    newPos = Math.floor(Math.random() * i + 1);
    temp = arr[i],
      arr[i] = arr[newPos];
    arr[newPos] = temp;
  }
  return arr;
};

let numRandom = arrayShuffle(guess);
suitCase();

function suitCase() {
  for (x = 0; x < num.length; x++) {
    document.getElementById("casePanel").insertAdjacentHTML('beforeend', "<a href=\"#\" id=\"case" + numRandom[x] + "\"><div class=\"case\"><span>" + num[x] + "</span></div></a>");
    document.getElementById('case' + numRandom[x]).onclick = function() {
      clickCase(this.id.replace('case', 'item'));
    }
  }
};

function clickCase(id) {
  document.getElementById(id).classList.add('black');
}

.black {
  background-color: blue;
}

<div class="container">
  <header>
    <h1>This is a sample title for this nice suitcase casino</h1>
  </header>

  <main class="wrap">
    <!-- Left price panel -->
    <div class="left-panel">
      <ul>
        <li id="item0">$.01</li>
        <li id="item1">$1</li>
        <li id="item2">$5</li>
        <li id="item3">$10</li>
        <li id="item4">$25</li>
        <li id="item5">$50</li>
        <li id="item6">$75</li>
        <li id="item7">$100</li>
        <li id="item8">$200</li>
        <li id="item9">$300</li>
        <li id="item10">$400</li>
        <li id="item11">$500</li>
        <li id="item12">$750</li>
      </ul>
    </div>
    <!-- ./Left price panel -->

    <div class="case-panel" id="casePanel">

    </div>

    <!-- Right price panel -->
    <div class="right-panel">
      <ul>
        <li id="item13">$1000</li>
        <li id="item14">$5000</li>
        <li id="item15">$10,000</li>
        <li id="item16">$25,000</li>
        <li id="item17">$50,000</li>
        <li id="item18">$75,000</li>
        <li id="item19">$100,000</li>
        <li id="item20">$200,000</li>
        <li id="item21">$300,000</li>
        <li id="item22">$400,000</li>
        <li id="item23">$500,000</li>
        <li id="item24">$750,000</li>
        <li id="item25">$1,000,000</li>
      </ul>
    </div>
    <!-- ./Right price panel -->
  </main>

  <footer></footer>
</div>

edited May 15, 2020 at 2:16

answered May 15, 2020 at 1:56

Barmar

789k57 gold badges554 silver badges669 bronze badges

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

5 Comments

Waleed Ahmad Over a year ago

This worked a bit like now it is targeting single items but still only after I click the last case

Waleed Ahmad Over a year ago

So whatever is the id of last case box, it targets the same id in items box. But rest of the 25 boxes do not respond to click events. It's probably a loop problem

Barmar Over a year ago

I've updated the answer with the explanation and fix for that.

Waleed Ahmad Over a year ago

Thank you so much @Barmar this worked like a charm. I am getting an error in the console Cannot read property 'insertAdjacentHTML' of null and I assume that's because of some loop indexing. However my app works perfect so I am fine with it.

Barmar Over a year ago

See stackoverflow.com/questions/14028959/… for likely explanations of that error.

Collectives™ on Stack Overflow

adding onClick event to JavaScript for loop

1 Answer 1

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related