3

I'm attempting to create onclick events for a list of items that will basically achieve the same affect from a different sources of information that depends on the list item selected.

function buttonOnclick(whichButton) {
    document.getElementById("dialog0").innerHTML = document.getElementById("dialog" + whichButton.toString());
}

document.getElementById("button1").onclick = function breakerOnclick1() {
    buttonOnclick(1);
}

document.getElementById("button2").onclick = function breakerOnclick2() {
    buttonOnclick(2);
}

document.getElementById("button3").onclick = function breakerOnclick3() {
    buttonOnclick(3);
}

document.getElementById("button4").onclick = function breakerOnclick4() {
    buttonOnclick(4);
}

document.getElementById("button5").onclick = function breakerOnclick5() {
    buttonOnclick(5);
}

I would like to achieve this affect with a for loop instead of so manually. I know there is probably a more object oriented oriented or simple approach I'm missing being green to JavaScript.

How can this affect be achieved more programmatically?

Solution: Just to be clear on the answer to this question. The following produces the correct results quickly and reliably. All though, I'm sure there are other relevant suggestions below.

for (var i = 1; i < NUM_ENTRIES; i++){
    document.getElementById("button" + i).onclick = function(){
        var replacement = document.getElementById("dialog" + this.id[this.id.length-1]).innerHTML;
        document.getElementById("dialog0").innerHTML = replacement;
    }
}
Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
  • do you need the name of the function eg `breakerOnclick1`? – Nina Scholz Sep 24 '15 at 20:39
  • can't you set it from dom? if the dom is generated you can generate the onclick attribute to all buttonOnClick(buttonId) as well. – toskv Sep 24 '15 at 20:39
  • I don't think the title makes sense. I think you simply want to loop 5 times, and attach your event to `"button"+i` – Farzher Sep 24 '15 at 20:40
  • 1
    If you use jQuery you can write this in about 3 lines of code. – Dave Sep 24 '15 at 20:41
  • yo, look at this: http://jsfiddle.net/1ej5mw4f/1/ (works, I'm not sure if I would use it tho) – pkn Sep 24 '15 at 20:55
  • @NinaScholz I just have a tendency to name any functions to produce legible (to me) source, of course it is not needed. – Paul Beaudet Sep 25 '15 at 14:03

6 Answers6

1

Well, first off, assigning a DOM element to another element's innerHTML is just going to produce "[object HTMLDivElement]" or similar depending on the element. Perhaps you want

function buttonOnclick(whichButton) {
 document.getElementById("dialog0").innerHTML = document.getElementById("dialog" + whichButton.toString()).outerHTML;
 //which would make the innerHTML be `<div>stuff</div>`
}

As for your other issue, just use a query, iterate to assign, and pass the button's id number along

var buttons = document.querySelectorAll("[id^=button]");
for(var i = 0; i < buttons.length; i++){
 buttons[i].onclick = function(){
  buttonOnclick(this.id[this.id.length-1]);
 };
}
Travis J
  • 81,153
  • 41
  • 202
  • 273
  • The first issue mentioned was definitely a typo on my part, I had meant to draw on the second element with the .innerHTML attribute instead of asking for the element itself. – Paul Beaudet Sep 25 '15 at 13:43
  • .innerHTML only ever has produced the contents of the HTML from my observation. It seems as though you are suggesting there are cases ware this becomes a bad assumption? – Paul Beaudet Sep 25 '15 at 14:11
0

With a for loop?

function buttonOnclick(whichButton) {
    document.getElementById("dialog0").innerHTML =  document.getElementById("dialog" + whichButton).innerHTML;
}
for (var i = 1; i < 6; i++) {
    document.getElementById("button" + i).onclick = function (n) {
        return function () {
            buttonOnclick(n);
        }
    }(i);
}
Selected: <strong><span id="dialog0"></span></strong><br>
<button id="button1">1</button> <span id="dialog1">abc</span><br>
<button id="button2">2</button> <span id="dialog2">def</span><br>
<button id="button3">3</button> <span id="dialog3">ghi</span><br>
<button id="button4">4</button> <span id="dialog4">jkl</span><br>
<button id="button5">5</button> <span id="dialog5">mno</span>   
Nina Scholz
  • 376,160
  • 25
  • 347
  • 392
  • 1
    This is wrong, see http://stackoverflow.com/questions/1451009/javascript-infamous-loop-issue – Barmar Sep 24 '15 at 20:43
  • You need parentheses around the function to make it an IIFE. – Barmar Sep 24 '15 at 20:50
  • no, because it is an assignment, so the expression is evaluated – Nina Scholz Sep 24 '15 at 20:51
  • @Barmar The function's place as the RHS to an `=` operator already makes it an expression, I believe. Granted, I would always use parens for clarity, but I think this code is now syntactically valid and operates as intended. – apsillers Sep 24 '15 at 20:52
0

This will give you want you're looking for.

function doSomething(id) {
    document.getElementById("dialog0").innerHTML = id;
}

for (let i = 0; i < 5; i++) {
    document.getElementById('button' + i).onclick = function(){
        doSomething(i);
    });
}
lostPixels
  • 1,303
  • 9
  • 23
0

to beginn you don't need to name your Anonymous function

then you could use a simple for loop?

for(var i=0;i<=5;i++){
  document.getElementById("button"+i).onclick = (function(i) {
     return function(){
       buttonOnclick(i);
     })(i); //saving i for later
}

the best solution would be to do something like this

<button data-id="1" class="bar">
<button data-id="2" class="bar">
                 ....

and add a event listener to getElementsByClassName("bar"), and read the data-id attribute.

yellowsir
  • 741
  • 1
  • 9
  • 27
0

Instead of hard-coding the button number in the function, get it from the element.

for var i = 1; i < 6; i++) {
    document.getElementById("button" + i).addEventListener("click", function () {
        var buttonNum = this.id.replace('button', '');
        buttonOnClick(buttonNum);
    }
}
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • 1
    Why? You have the index inside of your loop, doing string interpolation is slower. – lostPixels Sep 24 '15 at 20:48
  • 1
    Are you really worried about the performance of something that runs once when you click? It's not a bottleneck. – Barmar Sep 24 '15 at 20:49
  • Always. Although it's performance hit is arguably negligible in this case, it's also more fragile and prone to errors when this application scales. – lostPixels Sep 24 '15 at 20:52
  • On the other hand, this is more adaptable to changes where the parameter to the other function isn't just an index. – Barmar Sep 24 '15 at 20:53
0

A simple call to forEach can be used to iterate through the buttons.

[].forEach.call(document.getElementsByClassName('dialog-button'), function (el) {
  el.onclick = function () {
    document.getElementById("dialog0").innerHTML = document.getElementById(el.id.replace('button', 'dialog')).innerHTML;
  }
});
div {
  border: 1px dashed blue;
  margin: 10px;
  display: inline-block;
  width: 20px;
}
<div id="dialog0"></div>
<div id="dialog1">1</div>
<div id="dialog2">2</div>
<div id="dialog3">3</div>
<div id="dialog4">4</div>
<div id="dialog5">5</div>

<br />

<button id="button1" class="dialog-button">1</button>
<button id="button2" class="dialog-button">2</button>
<button id="button3" class="dialog-button">3</button>
<button id="button4" class="dialog-button">4</button>
<button id="button5" class="dialog-button">5</button>
Drew Gaynor
  • 8,292
  • 5
  • 40
  • 53