0

I'm a novice with JavaScript so bear with me.

I'm trying to use JavaScript to generate tabbed windows for specific bits of content. The HTML looks like this:

<div class="tab-win" id="tablinks-d38e44">
   <button class="tablinks" data-target="tab-bash-d38e44">bash</button>
   <button class="tablinks" data-target="tab-gui-d38e44">gui</button>
</div>

<div class="tabcontents" id="tabcontents-d38e44">
   <div class="tabcontent" id="tab-bash-d38e44">This is bash.</div>
   <div class="tabcontent" id="tab-gui-d38e44">This is plain old GUI...</div>
</div>

<p class="p">Here is another one.</p>

<div class="tab-win" id="tablinks-d38e56">
   <button class="tablinks" data-target="tab-bash-d38e56">bash</button>
   <button class="tablinks" data-target="tab-gui-d38e56">gui</button>
</div>

<div class="tabcontents" id="tabcontents-d38e56">
   <div class="tabcontent" id="tab-bash-d38e56">This is bash #2.</div>
   <div class="tabcontent" id="tab-gui-d38e56">This is plain old GUI..#2.</div>
</div>

</div>

I've cobbled together this function. 

function tabbedWindows(evt, env) {
  // Declare all variables
  var i, tabcontent, tablinks;

  // Get all elements with class="tabcontent" and hide them
  tabcontent = document.getElementsByClassName("tabcontent");
  for (i = 0; i < tabcontent.length; i++) {
    tabcontent[i].style.display = "none";
  }

  // Get all elements with class="tablinks" and remove the class "active"
  tablinks = document.getElementsByClassName("tablinks");
  for (i = 0; i < tablinks.length; i++) {
    tablinks[i].className = tablinks[i].className.replace(" active", "");
  }

  // Show the current tab, and add an "active" class to the button that opened the tab
  document.getElementById(env).style.display = "block";
  evt.currentTarget.className += " active";
}

If I add an onclick attribute directly to the button elements like so:

onclick="tabbedWindows(event,'content-id')"

it works great. However, I'm trying to mind separation of concerns and want to add the event dynamically. I'm trying to do this with the following:

const tab = document.querySelectorAll(".tablinks");
const content = document.querySelectorAll(".tabcontent");

for (let i = 0; i < tab.length; i++) {
    let conId = tab[i].getAttribute('data-target');
    let conArray = Array.from(content);
    let con = conArray.find((c) => c.getAttribute('id') === conId);
    let c = con.getAttribute('id');
    tab[i].addEventListener('click', tabbedWindows(event,c), false);
}

This doesn't work due to the following error: main.js:23 Uncaught TypeError: Cannot read property 'currentTarget' of undefined. The line it's failing on is:

evt.currentTarget.className += " active";

I think the reason for this is by the time the function attempts to attach the listener, event variable is already out of scope. That is just what I've been able to glean from research. The long and the short of it is I'm stumped and not even sure if this is the correct approach. I guess some questions I have are:

  1. Why does 'event' work as a variable when passed through an onclick attribute but not when passed in as a parameter to the function invoked through the for loop? I think I've already answered this question but maybe someone more knowledgeable can provide a better description.
  2. My for loop seems unnecessarily convoluted. Am I overcomplicating things here? I'm guessing 'yes'.

Any help is appreciated.

Jason Davis
  • 65
  • 1
  • 8
  • 2
    You need to use `tab[i].addEventListener('click', (event) => tabbedWindows(event,c), false);` - passing a function (with an event parameter) not the result of the function call – Bergi Oct 21 '19 at 21:03
  • i like to use data- attrib to attach the later-needed info to the element, so that it's easily reachable from the event handler's `event.target.dataset.prop`. – dandavis Oct 21 '19 at 21:06
  • @Bergi Sheesh! I guess it's just a matter of attention to details. Still got to learn I guess. Thanks! – Jason Davis Oct 21 '19 at 21:07
  • @dandavis Use `event.currentTarget.dataset.prop` though (or `this.dataset.prop`) – Bergi Oct 21 '19 at 21:10
  • FYI: don't use [`getElementsByClassName()`](https://stackoverflow.com/questions/54952088/how-to-modify-style-to-html-elements-styled-externally-with-css-using-js/54952474#54952474)....Ever. Especially since it appears you are aware of `.querySelectorAll()`. – Scott Marcus Oct 21 '19 at 21:29

2 Answers2

2

When you declare the event handler like this - tab[i].addEventListener('click', tabbedWindows(event,c), false);, you call the tabbedWindows with undefined values. Since evt is undefined this statement - evt.currentTarget - throws the error.

You need to pass a 2nd param to the event handler, because you want the content id that you take from the tab `data-target'. However, the tab is the event's target, and you can get the information when you click it. I've refactored the code, and removed redundancies.

const tabLinks = document.querySelectorAll(".tablinks");
const tabContents = document.querySelectorAll(".tabcontent");

const activateTab = target => {
  const contentId = target.getAttribute('data-target');

  // Get all elements with class="tabcontent" and hide them, and show the current tab
  tabContents.forEach(tabContent => {
    tabContent.style.display = tabContent.id === contentId ? 'block' : 'none';
  });

  // Get all elements with class="tablinks" and remove the class "active"
  tabLinks.forEach(tablink => {
    tablink.classList.remove('active');
  });

  // Add an "active" class to the button that opened the tab
  target.classList.add('active');
};

const tabbedWindows = evt => {
  const target = evt.currentTarget;

  activateTab(target);
}

document.querySelectorAll(".tablinks").forEach(tab => {
  tab.addEventListener('click', tabbedWindows, false);
});

activateTab(tabLinks[0]);
<div class="tab-win" id="tablinks-d38e44">
  <button class="tablinks" data-target="tab-bash-d38e44">bash</button>
  <button class="tablinks" data-target="tab-gui-d38e44">gui</button>
</div>

<div class="tabcontents" id="tabcontents-d38e44">
  <div class="tabcontent" id="tab-bash-d38e44">This is bash.</div>
  <div class="tabcontent" id="tab-gui-d38e44">This is plain old GUI...</div>
</div>

<p class="p">Here is another one.</p>

<div class="tab-win" id="tablinks-d38e56">
  <button class="tablinks" data-target="tab-bash-d38e56">bash</button>
  <button class="tablinks" data-target="tab-gui-d38e56">gui</button>
</div>

<div class="tabcontents" id="tabcontents-d38e56">
  <div class="tabcontent" id="tab-bash-d38e56">This is bash #2.</div>
  <div class="tabcontent" id="tab-gui-d38e56">This is plain old GUI..#2.</div>
</div>

</div>

Previous answer:

You can use currying to get the env, and return a new function that waits for the event object:

/** get env and return a new function that will be called with the event object **/
const tabbedWindows = env => evt => {
  // Declare all variables
  var i, tabcontent, tablinks;

  // Get all elements with class="tabcontent" and hide them
  tabcontent = document.getElementsByClassName("tabcontent");
  for (i = 0; i < tabcontent.length; i++) {
    tabcontent[i].style.display = "none";
  }

  // Get all elements with class="tablinks" and remove the class "active"
  tablinks = document.getElementsByClassName("tablinks");
  for (i = 0; i < tablinks.length; i++) {
    tablinks[i].className = tablinks[i].className.replace(" active", "");
  }

  // Show the current tab, and add an "active" class to the button that opened the tab
  document.getElementById(env).style.display = "block";
  evt.currentTarget.className += " active";
}

const tab = document.querySelectorAll(".tablinks");
const content = document.querySelectorAll(".tabcontent");

for (let i = 0; i < tab.length; i++) {
  let conId = tab[i].getAttribute('data-target');
  let conArray = Array.from(content);
  let con = conArray.find((c) => c.getAttribute('id') === conId);
  let c = con.getAttribute('id');
  tab[i].addEventListener('click', tabbedWindows(c), false); // call tabbedWindows with c
}
<div class="tab-win" id="tablinks-d38e44">
  <button class="tablinks" data-target="tab-bash-d38e44">bash</button>
  <button class="tablinks" data-target="tab-gui-d38e44">gui</button>
</div>

<div class="tabcontents" id="tabcontents-d38e44">
  <div class="tabcontent" id="tab-bash-d38e44">This is bash.</div>
  <div class="tabcontent" id="tab-gui-d38e44">This is plain old GUI...</div>
</div>

<p class="p">Here is another one.</p>

<div class="tab-win" id="tablinks-d38e56">
  <button class="tablinks" data-target="tab-bash-d38e56">bash</button>
  <button class="tablinks" data-target="tab-gui-d38e56">gui</button>
</div>

<div class="tabcontents" id="tabcontents-d38e56">
  <div class="tabcontent" id="tab-bash-d38e56">This is bash #2.</div>
  <div class="tabcontent" id="tab-gui-d38e56">This is plain old GUI..#2.</div>
</div>

</div>
Ori Drori
  • 183,571
  • 29
  • 224
  • 209
  • 1
    it looks more like currying than partial application – marzelin Oct 21 '19 at 21:21
  • @ori-drori great explanation and your refactoring is much more elegant than my hack job. Thanks! – Jason Davis Oct 21 '19 at 22:56
  • @ori-drori this works great. However, I would like to show the first tab by default. I can do something like `document.querySelector(".tablinks").click()` to click the first tab on page load but is there a way to have the function handle a default tab? – Jason Davis Oct 30 '19 at 17:48
  • 1
    You can extract the selection code to another function, and apply it to the tab you wish to activated. See updated code. – Ori Drori Oct 30 '19 at 18:44
0

Maybe you should just try doing this first:

for (let i = 0; i < tab.length; i++) {
    tab[i].addEventListener('click', tabbedWindows, false);
}

and in your tabbedWindows function you can add these lines

function tabbedWindows(event) { 

     let conId = event.currentTarget.getAttribute('data-target');
     let conArray = Array.from(content);
     let con = conArray.find((c) => c.getAttribute('id') === conId);
     let c = con.getAttribute('id');

     // and then rest of your code goes here

}
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
mahadev kalyan srikanth
  • 163
  • 6