aeml aeml - 14 days ago 5
Javascript Question

having trouble with adding an event "onmouseover" to a span

Can someone tell me what I am doing wrong? I've spent an entire day troubleshooting this but I am getting nowhere... I want to add the event "onmouseover" to my span elements. However when I implement the code below, nothing happens. I did a bit of googling and I think it might be a variable scope problem?? Im not too sure... Any help is appreciated!

<!DOCTYPE html>
<html>
<head>
<title>Fixing bugs in JS</title>
<script src="question1.js" type="text/javascript"></script>
<head>
<body>
<div id="output"></div>
</body>
<html>







var NUMBERS = 100;

function go()
{
var out = document.getElementById("output");
for (var i = 1; i < NUMBERS+1; i++) {
var span_one = document.createElement("span");
span_one.id = "span" + i;
span_one.innerHTML = "" + i;
out.appendChild(span_one);

if (isPrime(i) === true) { // where i is a prime number (3, 5, 7..etc)
span_one.style.backgroundColor = "red";
span_one.onmouseover = function() {
hover("span"+i, "yellow", "150%")
};
span_one.onmouseout = function() {
hover("span"+i, "red", "100%") // whatever color in this line always overrides previous set color...
};
}

function hover(id, color, size) {
var span = document.getElementById(id);
span.style.backgroundColor = color;
span.style.fontSize = size;
}


function etc() {
...
}

window.onload=go;



Answer

Your issue is that you have closures around your i variable.

Closures occur whenever you nest a function within another function. Where the code runs unpredictably is when the nested function uses a variable from an ancestor function and the nested function has a longer lifetime than the ancestor in question.

Here, your mouseover and mouseout functions rely on i from the parent function go. Since the mouseover and mouseout functions are being attached to DOM elements and those DOM elements are going to remain in memory until the page is unloaded, those functions will have a longer lifetime than go. This means that the i variable that go declared can't go out of scope when go completes and that both of the mouse functions will SHARE the same value of i. The value that i has by the time a human comes along and moves the mouse is the LAST value it had when the loop ended.

Closures can be challenging at first, but you can read a bit more about them here.

Changing var i to let i on your loop solves that because let introduces block scope for each iteration of the loop.

Also, I saw that you were missing two closing curly braces that were causing errors. I added my own isPrime() function. See comments for locations:

window.onload=go;

const NUMBERS = 100;

function go(){

    
  var out = document.getElementById("output");
  
  // Using let instead of var avoids a closure by making sure
  // that each looping number exists in the block scope of the
  // loop and upon each iteration a new variable is created.
  for (let i = 1; i < NUMBERS+1; i++) {
    
    var span = document.createElement("span");
    span.id = "span" + i;

    span.innerHTML = i + "<br>"; 
    out.appendChild(span);

    if (isPrime(i)) { // where i is a prime number (2, 3, 5, 7..etc)
      span.style.backgroundColor = "red";
      
      // If you use the i variable in nested functions, you will create a
      // closure around it and both the mouseover and mouseout functions will
      // share the last known value of i. Each function must get its own copy
      // of i.
      span.onmouseover = function() {
          hover("span" + i, "yellow", "150%")
      };
      
      span.onmouseout = function() { 
        // whatever color in this line always overrides previous set color...
        hover("span" + i, "red", "100%") 
      };
      
    }  // <-- Missing
  }    // <-- Missing
  
}
  
function isPrime(value) {
    for(var i = 2; i < value; i++) {
        if(value % i === 0) {
            return false;
        }
    }
    return value > 1;
}

function hover(id, color, size) {
    var span = document.getElementById(id);
    span.style.backgroundColor = color;
    span.style.fontSize = size;
    console.log(id, span);
}
<div id="output"></div>