0

I have this for loop:

    for(tab of tabGroupMain.tabs) {
        tab.button.addEventListener("click", function(ev) {
            UI.ToggleTab(tab.content, tabGroupMain);
        });
    }

and if I decide to log the current tab it correctly returns me the current tab e.g.

Object { name: "Info", id: 1, button: span#infoTabButton.title.medium.bold.menuTitle, content: div#infoTabContent.menuTab }

however the moment I try to get the current tab in the addEventListener method it returns the latest element of the array, no matter which tab it currently is e.g.

Object { name: "Market", id: 0, button: span#marketTabButton.title.medium.bold.menuTitle, content: div#marketTabContent.menuTab }

(should return):

Object { name: "Info", id: 1, button: span#infoTabButton.title.medium.bold.menuTitle, content: div#infoTabContent.menuTab }

code for context:

    new UI.Tab("Market", "marketTabButton", "marketTabContent");
    new UI.Tab("Info", "infoTabButton", "infoTabContent");

    new UI.TabGroup("Main", [UI.tabByName("Info"), UI.tabByName("Market")] );

is the code broken? in that case what can I do to fix it?

x8c8r
  • 23
  • 6
  • a minor comment, please declare all variables. – Nina Scholz Feb 18 '23 at 08:44
  • @NinaScholz sorry, but can you be more specific about what variables I need to declare? – x8c8r Feb 18 '23 at 08:46
  • You should use the `ev` object to find the tab that was clicked: `ev.target.content`. – Kokodoko Feb 18 '23 at 08:48
  • 1
    `for(tab of tabGroupMain.tabs)` should be `for (const tab of tabGroupMain.tabs)` to prevent changing variables outside of the function/block. – Nina Scholz Feb 18 '23 at 08:52
  • @Kokodoko it assigns the event to a button, and I don't currently see a way to trace back to the parent object from it, which is a requirement in this case – x8c8r Feb 18 '23 at 08:52
  • @NinaScholz is there any way to avoid the const? It works, but I would rather not use ES6 keywords (don't ask why) – x8c8r Feb 18 '23 at 08:54
  • 1
    with using `of`, you get `let` and `const`. if in doubt use old `var` instead. – Nina Scholz Feb 18 '23 at 08:55
  • @NinaScholz I guess true, novadays it's tough to find what keywords come from which version of JS. So, I kind of broke my own rule already, I will look into use var, but for now, yea, I guess I should just give up and use const. – x8c8r Feb 18 '23 at 08:58
  • @NinaScholz you can use `ev.target.parentElement` to find the parent of the clicked element. I think you should always use the event object when working with listeners. It’s more readable, you don’t have to worry about your variables scope in the for loop. – Kokodoko Feb 19 '23 at 11:38
  • @Kokodoko, that is right. – Nina Scholz Feb 19 '23 at 12:06

1 Answers1

0
  1. Not declaring a variable causes it to be declared as global (which is bad).
  2. In your loop, tab will iterate over all the values and end on the last one. So, if you refer to it afterwards it will hold the value of that last item. This will happen both if you skip declaration (creating a global variable) or define it with var, which defines the variable in the function scope (if you are inside a function) or global scope (if outside).

To have the expected behavior, either use const or let, which define the variable in the block scope and are distinct from one iteration of the loop to another, or use forEach Array method like so (given tabGroupMain.tabs is an Array):

tabGroupMain.tabs.forEach(tab => {
    tab.button.addEventListener("click", function(ev) {
        UI.ToggleTab(tab.content, tabGroupMain);
    });
}
Tigran
  • 2,800
  • 1
  • 19
  • 19