0

$('.tabLabel').click(function() {
  if (!$(this).hasClass('activeTab')) {
    $('.tabLabel').removeClass('activeTab');
    $(this).addClass('activeTab');
    $('.tab').toggleClass('activeTabContent');
  }
});

var tabLabels = document.querySelectorAll('.tabLabel');

Array.from(tabLabels).forEach(function(tabLabel) {
  tabLabel.addEventListener('click', function(e) {
    var activeTabLabel = e.target.classList.contains("activeTab");

    if (!activeTabLabel) {
      tabLabel.classList.remove('activeTab');
    }
  });
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<div class="container">
  <div class="tabLabel activeTab">
    <p>Tab One</p>
  </div>
  <div class="tabLabel">
    <p>Tab Two</p>
  </div>

</div>

I have this click function that I'm getting a bit lost in converting to "Vanilla" Js (plain js). Any help would be appreciated. I've tried so far what I can but the lexical this is what is confusing me.

UPDATE: I'm still having troubles but I can at least console.log the elements that I want to target.

// Working Code in jQuery
$('.tabLabel').click(function() {
  if (!$(this).hasClass('activeTab')) {
    $('.tabLabel').removeClass('activeTab');
    $(this).addClass('activeTab');
    $('.tab').toggleClass('activeTabContent');
  }
});


// My attempt at converting
// Does not work. returns err => Cannot read property 'remove' of undefined  || Cannot read property 'toggle' of undefined
var tabLabels = document.querySelectorAll('.tabLabel');
var tab = docuement.querySelectorAll('.tab');

Array.from(tabLabels).forEach(function(tabLabel) {
  tabLabel.addEventListener('click', function(e) {
    if (!this.classList.contains('activeTab')) {

      tabLabel.classList.remove('activeTab');
      this.classList.add('activeTab');
      tab.classList.toggle('activeTabContent');
    }
  });
});
cpt-crunchy
  • 373
  • 3
  • 20
  • You need to get `classList` off the `e.target`, not `this`. See [this answer](https://stackoverflow.com/questions/9012537/how-to-get-the-element-clicked-for-the-whole-document#9012576) – Alex Apr 19 '18 at 20:57
  • Looks like it's not a problem with `this`, since it's not complaining about `contains` not being a property of undefined. So it's likely `tabLabel` and `tab` that are causing the problem... – Heretic Monkey Apr 19 '18 at 21:01
  • I believe the event is bound to each tabLabel element. This is my thought process – cpt-crunchy Apr 19 '18 at 21:03

1 Answers1

2

The Problem:

Your code tabLabel.classList.remove('activeTab'); doesn't work because you define your tabLabel in a different scope. It is defined when you create your click listener, however the click listener event is in a completely different scope when it's fired, so it ends up being 'undefined'. This was the root of your problem.

The 'this' keyword can be tricky, especially in embedded callback functions because it's really easy to lose track of what scope you're currently in. I always find it is helpful to console.log(this) just to make sure it is targeting what I want.

Edit- For more information on the 'this' keyword, I suggest checking out all the resources here:

How does the "this" keyword work?

The Solution:

Below is a modified version of your vanilla JS code that toggles the activeTab class between the two on click.

var tabLabels = document.querySelectorAll('.tabLabel');
var tabs = document.querySelectorAll('.tab');

tabLabels.forEach(function(tabLabel) {

    tabLabel.addEventListener('click', function(e) {

        if (!this.classList.contains('activeTab')) {

            tabLabels.forEach(function(tl){         
                  tl.classList.remove('activeTab');
             });      

         this.classList.add('activeTab');
         tabs.forEach(function(tab) {
             tab.classList.toggle('activeTabContent');
         }
        }
    });
});

A couple things to note:

  • You had a typo in the definition of your tab variable, 'docuement' should be 'document'.

  • You don't need to do Array.from(tabLabels).forEach(), querySelectorAll already created an array. You'll see that I modified that.

Edit- As frederickf clarified, querySelectorAll doesn't create an array, but a NodeList. https://developer.mozilla.org/en-US/docs/Web/API/NodeList

  • We have to iterate over your tabLabels array again to remove the 'activeTab' class for each item before we apply it to the clicked control.

Hope that helps!

cpt-crunchy
  • 373
  • 3
  • 20
Ryan Gibbs
  • 1,178
  • 10
  • 24
  • 2
    Good answer. Minor point of correction, querySelectorAll returns a NodeList https://developer.mozilla.org/en-US/docs/Web/API/NodeList. – frederickf Apr 19 '18 at 22:30
  • Thanks @Ryan Gibbs. This was helpful. Again, I got a bit lost with maintaining the scope of the this keyword. I'll give this a shot and see if it resolves my dilema – cpt-crunchy Apr 19 '18 at 22:43
  • @frederickf Thank you for the correction. I've edited it into my response. I wasn't aware of that, so I learned something. :) – Ryan Gibbs Apr 19 '18 at 22:50
  • @cpt-crunchy I put in a link with good explanations on how 'this' works. – Ryan Gibbs Apr 19 '18 at 22:51