CODEWITHSUNDEEP
javascripthtml
Liondancer
Liondancer

Reputation: 16469

Running functions onclick. All buttons being clicked

I have many buttons that have class="clearSelect". I want these buttons the execute a function onclick. I am new to javascript and not quite sure why this is occurring but I think my functions are being executed instead of only executing onclick

The code below is what is calling all my other functions causing every button to be clicked.

code:

var buttons = document.getElementsByClassName("clearSelect");  // objects with class="clearSelect"

for (var i = 0; i < buttons.length; i++) {
    var button = buttons[i];
    // button.addEventListener("onclick", clearOrSelectAll(button.id));
    button.onclick = clearOrSelectAll(button.id);
}

These are the functions being called:

function clearOrSelectAll(btn) {
    var cleartab = clearButtonSet[btn];
    var selecttab = selectButtonSet[btn];
    // console.log("clicked!");
    if (cleartab != null) {
        getOSList(cleartab, false);
    } else {
        getOSList(selecttab, true);
    }
}

function getOSList(tabVal, fate) {
    var configTab = document.getElementById(tabVal);
    var browserList = configTab.getElementsByClassName("browser_list");
    // var idObjs = browserList.getElementsByTagName("li");
    for (var h = 0; h < browserList.length; h++) {
        var idObjs = browserList[h].getElementsByTagName("li");
        // console.log(h);
        // console.log(idObjs);
        // select all
        if (fate) {
            for (var i = 0; i < idObjs.length; i++) {
                if (configs[idObjs[i].id] == null) {
                    idObjs[i].className = "selected";
                    configs[idObjs[i].id] = config_dictionary[idObjs[i].id];
                }
            }
        // clear all
        } else {
            for (var j = 0; j < idObjs.length; j++) {
                if (configs[idObjs[j].id] == null) {
                    idObjs[j].className = "unselected";
                    delete configs[idObjs[j].id];
                }
            }
        }
    }
}

Upvotes: 0

Views: 3069

Answers (2)

Christopher
Christopher

Reputation: 404

In your for loop when you attach the event to all of the buttons, you are calling the clearOrSelectAll function. You probably want to wrap it in an anonymous function to make sure it's only called when the event is fired.

// Non-ideal solution: see edit
button.onclick = function() {
    clearOrSelectAll(button.id);
}

EDIT: It has been pointed out that the 'this' context variable will point to the element in question when an event handler is attached by means of the onclick property, or the addEventListener method. As such it would probably be cleaner (and easier to read) if you were to reference that instead of using 'button' as a closure and count on javascript engines to not optimize your loop too heavily (as that would mess with the value of 'button' at the time that the event is called.

button.onclick = function() {
    clearOrSelectAll(this.id);
};

Upvotes: 1

Rick Hitchcock
Rick Hitchcock

Reputation: 35670

@Christopher was very close, but button.id should be this.id.

button.onclick = function() {
  clearOrSelectAll(this.id);
}

The reason button.id doesn't work can be demonstrated with this code:

var buttons= document.getElementsByTagName('button');

for (var i = 0; i < buttons.length; i++) {
  var button = buttons[i];
  button.onclick = function() {
    alert(button.id);
  }
}
<button id="B1">Button 1</button>
<button id="B2">Button 2</button>
<button id="B3">Button 3</button>

Each button returns "B3," because that's the last object that the variable button is assigned to.

Upvotes: 2

Related Questions