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, May 15 '20 at 01:49
Yes case0, case1, case2 when clicked should change css class of item0, item1 and item2 respectively — Waleed Ahmad, May 15 '20 at 01: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, May 15 '20 at 01:54
But that's the problem I am unable to resolve. My selectors are in a loop so there are 26 of them. And similarly there are 26 targets — Waleed Ahmad, May 15 '20 at 01:56
Just noticed that there are duplicate IDs in the left and right panels. — Barmar, May 15 '20 at 02:03
The ones on the right start from x=13 so they aren't really duplicate — Waleed Ahmad, May 15 '20 at 02:04

Barmar · Accepted Answer · 2020-05-15T02:16:34.787

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>

This worked a bit like now it is targeting single items but still only after I click the last case — Waleed Ahmad, May 15 '20 at 02:01
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 — Waleed Ahmad, May 15 '20 at 02:07
I've updated the answer with the explanation and fix for that. — Barmar, May 15 '20 at 02:16
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. — Waleed Ahmad, May 15 '20 at 02:28
See https://stackoverflow.com/questions/14028959/why-does-jquery-or-a-dom-method-such-as-getelementbyid-not-find-the-element for likely explanations of that error. — Barmar, May 15 '20 at 15:00

adding onClick event to JavaScript for loop

1 Answers1

Linked

Related