event.which always equals 1, how to determine which event is happening

Question

I am creating a rating system, in which you have 5 stars. As you hover over the stars they turn gold and when you click on them they run a function. When I try to get which star you clicked I always get 1, probably because of the way I am putting my action listener on, but I don't know what I can do different. a sample of my code is:

function loadStars()
{
    that.starChange = document.getElementsByClassName("starImg");
    var k=0;
    while (k<=4)
    {
        that.starChange[k].addEventListener("mousedown", that.starGoldClick);
        k++;
    }
}
this.starGoldClick = function()
{
    var star;
    that.starValue = document.getElementsByClassName("starImg");
    var i=0;
    while (i<=4)
    {
        that.starValue[i].addEventListener("mouseup", function()
        {
            this.star = i;
            console.log(event.which);
        });
        i++;
    }

    console.log(star);

}

I have tried with the console log to figure out the answer here. Is there a way to setup a whole class of items with a action listener at the same time (without a while loop) that will change the event.which to something other than 1?

Answer 1

The first problem you have is that you are trying to access event, but since you didn't declare that argument in your callback function, the Global window.event is being accessed, which may not represent what you are after.

Second, which (while can be used for mouse and keyboard events) is either non-standard or deprecated, depending on how you use it.

Next, you don't need to set up event handlers on each star. Use event delegation and set up your events on an ancestor of the stars and let events that originate with the stars bubble up to that ancestor element. Then, at that level, determine which star triggered the event using the handler's event argument and its target property.

Also, by setting things up this way, there is no need to programmatically determine what event took place. You'll know that because of the event callback that is running.

Finally, don't use getElementsByClassName(), ever.

Here's an example:

// Don't use getElementsByClassName(), use querySelectorAll()
// And convert the node list returned into an Array so .indexOf can be used
let stars = Array.prototype.slice.call(document.querySelectorAll(".star"));

// Just set up your event handlers at the parent/ancestor
document.getElementById("parent").addEventListener("mouseover", function(event){
  // Use the event argument and its target property to determine which
  // actual element within the parent/ancestor triggered the event

  // We only want to do something when a star is the source of the event
  if(event.target.classList.contains("star")){
    console.clear();
    console.log("You moused over star #" + (stars.indexOf(event.target) + 1));
  }
});

document.getElementById("parent").addEventListener("click", function(event){
  // Use the event argument and its target property to determine which
  // actual element within the parent/ancestor triggered the event

  // We only want to do something when a star is the source of the event
  if(event.target.classList.contains("star")){
    // You can just have the "star clicked" code right here, or you can
    // call out to another function from here, which is what I'll show
    console.clear();
    console.log("You clicked on star #" + (stars.indexOf(event.target) + 1));
    starClicked(event.target); // Pass a reference to the clicked star
  }
});

function starClicked(star){
  star.classList.add("clicked")
  console.clear();
  console.log("Star clicked!");
}

.clicked { color:red; font-weight:bold; }
.star { font-size:2em; }
.star:hover { color:pink; cursor:pointer; }

<div id="parent">
  <span class="star">&#9733;</span>
  <span class="star">&#9733;</span>
  <span class="star">&#9733;</span>
  <span class="star">&#9733;</span>
  <span class="star">&#9733;</span>  
</div>