1

I'm really new to javascript from C# and i'm having a little trouble. I wrote this function to make adding menu's a bit easier on my site. It works well except I can't seem to give my div's an individual url, even though I can give them an individual innerHtml.

I've been stuck trying different things such as divs[i].location.url etc.. but I can't seem to have anything work. My current solution has each div link to /contact.html which I'm a little confused by.

function DrawMainMenu() {
    var btns = [
        ["About", "/about.html"],
        ["Portfolio", "/portfolio.html"],
        ["Resume", "/resume.html"],
        ["Contact", "/contact.html"]
    ];
    var numOfBtns = btns.length;
    var divs = new Array(numOfBtns);
    for (var i = 0; i < numOfBtns; i++) {
        divs[i] = document.createElement("div");
        divs[i].className = "menuBtn";
        divs[i].innerHTML = btns[i][0];
        divs[i].style.height = (30 / numOfBtns) + "%";
        divs[i].style.lineHeight = 3.5;
        var link = btns[i][1];
        divs[i].addEventListener('click', function() {
            location.href = link;
        }, false);
        document.getElementById("buttons").appendChild(divs[i]);
    }
}

Thanks

rrk
  • 15,677
  • 4
  • 29
  • 45
lacking-cypher
  • 483
  • 4
  • 16
  • Possible duplicate of [Javascript infamous Loop issue?](http://stackoverflow.com/questions/1451009/javascript-infamous-loop-issue) – Andreas Feb 27 '16 at 16:27

1 Answers1

1

The problem is that the variable link gets overwritten each iteration, so when the event handler it gets link, which is the string '/contact.html', since that was the last value given to it.

You can try setting onclick attribute to elements, which will store the variable in the attribute onclick. Therefore, it will have the old and correct value.

function DrawMainMenu() {
    var btns = [
        ["About", "/about.html"],
        ["Portfolio", "/portfolio.html"],
        ["Resume", "/resume.html"],
        ["Contact", "/contact.html"]
    ];
    var numOfBtns = btns.length;
    var divs = new Array(numOfBtns);
    for (var i = 0; i < numOfBtns; i++) {
        divs[i] = document.createElement("div");
        divs[i].className = "menuBtn";
        divs[i].innerHTML = btns[i][0];
        divs[i].style.height = (30 / numOfBtns) + "%";
        divs[i].style.lineHeight = 3.5;
        var link = btns[i][1];
        divs[i].setAttribute('onclick', 'location.href = "' + link + '"');
        document.getElementById("buttons").appendChild(divs[i]);
    }
}
DrawMainMenu();
<div id="buttons"><div> 

Updated answer

Here we make use of closures. Using a closure (closing the values of link) we bind the value to the scope of the click handler.

function DrawMainMenu() {
  var btns = [
    ["About", "/about.html"],
    ["Portfolio", "/portfolio.html"],
    ["Resume", "/resume.html"],
    ["Contact", "/contact.html"]
  ];
  var numOfBtns = btns.length;
  var divs = new Array(numOfBtns);
  for (var i = 0; i < numOfBtns; i++) {
    (function() {
      divs[i] = document.createElement("div");
      divs[i].className = "menuBtn";
      divs[i].innerHTML = btns[i][0];
      divs[i].style.height = (30 / numOfBtns) + "%";
      divs[i].style.lineHeight = 3.5;
      var link = btns[i][1];
      document.getElementById("buttons").appendChild(divs[i]);
      divs[i].addEventListener('click', function() {
        location.href = link;
      }, false);
    }());
  }
}
DrawMainMenu();
<div id="buttons"><div>
rrk
  • 15,677
  • 4
  • 29
  • 45
  • What a great solution! Really cool that you can use 'onclick' in setAttribute(). Thanks for the help, I really appreciate the reply! – lacking-cypher Feb 27 '16 at 17:09
  • @luna-games actually there is an alternative too. I've added that to my code. You can try that too. – rrk Feb 27 '16 at 17:12