Search code examples
javascriptvariablesgetelementbyid

Problem with javascript loop and setting documentGetElementById with parameters

I'm working on a star rating system. I want to set event listeners on a list of star images such that clicking on a star either fills in the stars with solid or blank stars. The problem is that when setting getElementById with parameters to add to the list items an event listener, a variable (myId) is correctly set according to the loop iteration but passing the same variable to a function to run onclick returns the last value of (myId) rather than the loop iteration value. enter image description here I know the code is working as if I hard code a value for the passed variable it works Here is the code:

    <!doctype html>
    <html>
    <head>
    <meta charset="utf-8">
    <title>Star Rating</title>
    </head>

    <body>
    
    <ul>
    <li class="myClass"  id="liId113"><img src="images/starFade.png"></li>
    <li class="myClass"  id="liId213"><img src="images/starFade.png"></li>
    <li class="myClass"  id="liId313"><img src="images/starFade.png"></li>
    <li class="myClass"  id="liId413"><img src="images/starFade.png"></li>
    <li class="myClass"  id="liId513"><img src="images/starFade.png"></li>
  
    </ul>

    </body>
    
    <script>
    window.onload = setArray;
    
    function setArray() {
    let myArray = document.getElementsByClassName("myClass");
            
    let l = myArray.length;
    
    for(var i = 0;i < l;i++) {
   
    myId = String(myArray[i].id);
            
    document.getElementById(myId).addEventListener("click", function () {highlightStar(myId);});
    //This adds event listeners to each <li> element using the correct myId but the myId of highlightStar is the last value of MyId

    }
    }
    function highlightStar(myId) {
    
    firstPart = myId.slice(0,4);
    sequenceNo = myId.charAt(4);
    group = myId.slice(-2);
        
    
    if (sequenceNo == 5) {
    document.getElementById(myId).innerHTML = "<img src='images/starFull.png'>";
    document.getElementById(firstPart+4+group).innerHTML = "<img src='images/starFull.png'>";   
    document.getElementById(firstPart+3+group).innerHTML = "<img src='images/starFull.png'>";   
    document.getElementById(firstPart+2+group).innerHTML = "<img src='images/starFull.png'>";   
    document.getElementById(firstPart+1+group).innerHTML = "<img src='images/starFull.png'>";   
    }
    else if (sequenceNo == 4) {
    document.getElementById(myId).innerHTML = "<img src='images/starFull.png'>";
    document.getElementById(firstPart+5+group).innerHTML = "<img src='images/starFade.png'>";   
    document.getElementById(firstPart+3+group).innerHTML = "<img src='images/starFull.png'>";   
    document.getElementById(firstPart+2+group).innerHTML = "<img src='images/starFull.png'>";
    document.getElementById(firstPart+1+group).innerHTML = "<img src='images/starFull.png'>";       
    }
    else if (sequenceNo == 3) {
    document.getElementById(myId).innerHTML = "<img src='images/starFull.png'>";
    document.getElementById(firstPart+5+group).innerHTML = "<img src='images/starFade.png'>";   
    document.getElementById(firstPart+4+group).innerHTML = "<img src='images/starFade.png'>";   
    document.getElementById(firstPart+2+group).innerHTML = "<img src='images/starFull.png'>";
    document.getElementById(firstPart+1+group).innerHTML = "<img src='images/starFull.png'>";       
    }
    else if (sequenceNo == 2) {
    document.getElementById(myId).innerHTML = "<img src='images/starFull.png'>";
    document.getElementById(firstPart+5+group).innerHTML = "<img src='images/starFade.png'>";   
    document.getElementById(firstPart+4+group).innerHTML = "<img src='images/starFade.png'>";   
    document.getElementById(firstPart+3+group).innerHTML = "<img src='images/starFade.png'>";
    document.getElementById(firstPart+1+group).innerHTML = "<img src='images/starFull.png'>";           
    }
    else if (sequenceNo == 1) { 
    document.getElementById(myId).innerHTML = "<img src='images/starFull.png'>";
    document.getElementById(firstPart+5+group).innerHTML = "<img src='images/starFade.png'>";   
    document.getElementById(firstPart+4+group).innerHTML = "<img src='images/starFade.png'>";   
    document.getElementById(firstPart+3+group).innerHTML = "<img src='images/starFade.png'>";
    document.getElementById(firstPart+2+group).innerHTML = "<img src='images/starFade.png'>";           
    }
    }
    </script>
    
    

    </html>

Blank Starenter Full star

Solution

  • The issue you're facing involves variable scoping. At the moment, your variable myId isn't scoped to the loop and consequently gets overwritten with each iteration, resulting in the last value being stored and used in your event listener. To overcome this issue, you could use a JavaScript closure. This can ensure that each event listener retains access to its pertinent loop iteration value:

    <!doctype html>
<html>
    <head>
        <meta charset="utf-8">
        <title>Star Rating</title>
    </head>
    <body>
        <ul>
            <li class="myClass"  id="li1"><img src="images/starFade.png" alt="star image"></li>
            <li class="myClass"  id="li2"><img src="images/starFade.png" alt="star image"></li>
            <li class="myClass"  id="li3"><img src="images/starFade.png" alt="star image"></li>
            <li class="myClass"  id="li4"><img src="images/starFade.png" alt="star image"></li>
            <li class="myClass"  id="li5"><img src="images/starFade.png" alt="star image"></li>
        </ul>
    </body>
    
    <script>
        window.onload = function setArray() {
            let myArray = document.getElementsByClassName("myClass");
            for(var i = 0; i < myArray.length; i++) {
                let myId = myArray[i].id;
                document.getElementById(myId).addEventListener("click", (function(myId) {
                    return function() {highlightStar(myId);};
                })(myId));
            }
        }
        
        function highlightStar(myId) {
            let sequenceNo = parseInt(myId.replace('li',''));
            let myArray = document.getElementsByClassName("myClass");
            for(var i = 0; i < myArray.length; i++) {
                let id = myArray[i].id;
                let currentNo = parseInt(id.replace('li',''));
                if (currentNo <= sequenceNo) {
                    document.getElementById(id).innerHTML = "<img src='images/starFull.png' alt='star image'>";
                } else {
                    document.getElementById(id).innerHTML = "<img src='images/starFade.png' alt='star image'>";
                }
            }
        }
    </script>
</html>