Reputation: 27
I have the following situation which I cannot solve. I am relatively new to js. I have a js that runs on a webpage. The script runs when a KB shortcut is pressed. After modifying a few things, it pops up an html banner in which I want to put certain messages and buttons depending on what thing the user ran a script on. For a simple case, let's say there are two potential messages that can go in this popup. I have the details in an object array:
NH_Bann = {
STC: {
active: false,
bannText: "Force Title Case: ",
id: "toTitleCaseStrong",
value: "Yes",
action: function() {
var newNameStr = toTitleCaseStrong(vname);
if (newNameStr !== name) {
//**update function
NH_Bann.STC.active = false;
}
}
},
DTC: {
active: false,
bannText: "Enable DTC? ",
id: "addDTC",
value: "Yes",
action: function() {
//**update function
NH_Bann.DTC.active = false;
}
}
}
When the script is run, there are some if statments that can change the active keys to true. After the script runs, I want to run through the objects in NH_Bann, and if the active key is true, make a message with an action button that fires the action button. The part I am having trouble with is making the buttons dynamically. From other threads, I thought I could store the buttons in an array, but maybe the onclick doesn't work that way? This is what I have:
function setupButtons() {
var ixButt = 0;
var btn = [];
for (var NHix = 0; NHix < Object.keys(NH_Bann).length; NHix++ ) {
tempKey = Object.keys(NH_Bann)[NHix];
if (NH_Bann[tempKey].active) {
btn[ixButt] = document.getElementById(NH_Bann[tempKey].id);
btn[ixButt].onclick = function(){
NH_Bann[tempKey].action();
assembleBanner(); // makes the html for the banner
}
ixButt++;
}
}
}
I make the buttons in another piece of code which sets up the ids:
function assembleBanner() {
sidebarMessageEXT = [sidebarMessage.slice(0)];
var EXTOption = false;
for (var NHix = 0; NHix < Object.keys(NH_Bann).length; NHix++ ) {
tempKey = Object.keys(NH_Bann)[NHix];
if (NH_Bann[tempKey].active) {
sidebarMessageEXT.push(NH_Bann[tempKey].bannText + '<input id="' + NH_Bann[tempKey].id + '" type="button" value="' + NH_Bann[tempKey].value + '">');
EXTOption = true;
}
}
if (EXTOption) {
sidebarMessageEXT = sidebarMessageEXT.join("<li>");
displayBanners(sidebarMessageEXT,severity);
setupButtons();
} else {
displayBanners(sidebarMessage,severity);
setupButtons();
}
}
The issue i'm having is that I get the two distinct messages and two buttons in the banner if both objects are active==true, but pressing them always fires the update function of the DTC object. Any suggestions? I'm open to other methods, but I need to be able to add to the object list over time and have the buttons be displayed conditionally on the status of the active key for each object. Thx!
Upvotes: 0
Views: 264
Reputation: 78850
The problem has to do with closures. In this code:
function setupButtons() {
var ixButt = 0;
var btn = [];
for (var NHix = 0; NHix < Object.keys(NH_Bann).length; NHix++ ) {
tempKey = Object.keys(NH_Bann)[NHix];
if (NH_Bann[tempKey].active) {
btn[ixButt] = document.getElementById(NH_Bann[tempKey].id);
btn[ixButt].onclick = function(){
NH_Bann[tempKey].action();
assembleBanner(); // makes the html for the banner
}
ixButt++;
}
}
}
...tempKey
is a variable that lives within the call to setupButtons
. Notice that you're creating two onclick
function callbacks in a loop, and both make reference to tempKey
. However, they're not going to be referencing the variable's value at the time of function creation, but rather the latest value of the variable. So once the loop completes, tempKey
is going to reference the last value it had.
To work around this, you can use this trick to create a new closure for each onclick
that will have the correct value:
function setupButtons() {
var ixButt = 0;
var btn = [];
for (var NHix = 0; NHix < Object.keys(NH_Bann).length; NHix++ ) {
tempKey = Object.keys(NH_Bann)[NHix];
if (NH_Bann[tempKey].active) {
btn[ixButt] = document.getElementById(NH_Bann[tempKey].id);
btn[ixButt].onclick = (function(buttonId) {
return function() {
NH_Bann[buttonId].action();
assembleBanner(); // makes the html for the banner
}
})(tempKey);
ixButt++;
}
}
}
Essentially, this is binding the current value of tempKey
to a new variable by passing it to an immediately-executing function, so both onclick
functions no longer reference the variable which changes during the loop.
For a less esoteric way to do this, you could move the creation of each button into its own named function, passing the required data:
function setupButtons() {
var btn = [];
for (var NHix = 0; NHix < Object.keys(NH_Bann).length; NHix++) {
tempKey = Object.keys(NH_Bann)[NHix];
if (NH_Bann[tempKey].active) {
btn.push(setupButton(NH_Bann[tempKey]);
}
}
}
function setupButton(bannerData) {
var button = document.getElementById(bannerData.id);
button.onclick = function() {
bannerData.action();
assembleBanner();
};
return button;
}
Upvotes: 1