JS "Tabs" element

Question

How can this be improved?

I'm looking particularly at the twice repeated tabButtons.forEach(button => {

const tabButtons = document.querySelectorAll('[data-tab-button]');
const tabContent = document.querySelectorAll('[data-tab-content]');

tabButtons.forEach(button => {
    button.onclick = () => {
        tabButtons.forEach(button => {
            button.classList.remove('text-black');
            button.classList.add('text-border-grey');
            button.classList.remove('border-b-2');
        });
        tabContent.forEach(content => {
            if(button.dataset.tabButton == content.dataset.tabContent) {
                content.classList.remove('hidden');
                button.classList.remove('text-border-grey');
                button.classList.add('text-black');
                button.classList.add('border-b-2');
            } else {
                content.classList.add('hidden');
            }
        });
    };
});

Answer 1

I don't think the two loops of tabButtons.forEach is avoidable because these are executed at different times and both are necessary. The first is assigning a click handler to each button, which is necessary to make them interactive; and the second, on the click of a tab button, loops over them all to ensure that each has its visual state updated.

What I do find difficult to read about the code is all the .add and .remove calls. My suggestion is to consolidate these so that it is easier to understand which classes apply to which states and so that a particular type of element - tab button or content - can have its style updated in a single line of code.

First, I would put all of the class names for the active and inactive button states into string variables that can be easily and clearly referenced.

const activeButtonClassName = 'border-b-2 text-black';
const inactiveButtonClassName = 'text-border-grey'

If in the future we need to add or modify these styles, we now have a single place in which to do it.

Second, I would create a function which takes as argument a tabId which represents the attribute value which links a tab button (via its data-tab-button attribute) to a tab content (via its data-tab-content attribute). The function will do the work of visiting each tab button and tab content and setting the styling to active or inactive based on the value of tabId.

const activateTab = tabId => {
  tabButtons.forEach(tabButton => {
    const isActive = tabButton.dataset.tabButton === tabId;
    
    tabButton.className = isActive ? activeButtonClassName : inactiveButtonClassName;
  });

  tabContent.forEach(tabContent => {
    const isActive = tabContent.dataset.tabContent === tabId;

    tabContent.classList.toggle('hidden', !isActive);
  });
};

We still have the looping over each tab button and tab content, but I think the code that applies the styling is much more easily comprehensible.

I am using Element.className instead of Element.classList for the tabButton so that I can set the full string class attribute as either of the two variables we defined earlier. The assignment is clear: if active, assign active class; otherwise assign inactive class.

For the tabContent, I am using classList.toggle because all we are doing is toggling the hidden class depending on whether or not the content is active.

We will still need to assign a click handler to each tab button and a loop is still the best way to do this.

tabButtons.forEach(tabButton => {
  tabButton.addEventListener('click', (event) => {
    const activeTabId = event.target.dataset.tabButton;

    activateTab(activeTabId);
  });
});

I am using Element.addEventListener instead of Element.onclick because that's how I was raised, but I do not think that the differences are significant for this piece of code.

An additional benefit to having the tab activation logic in its own function is that it allows me to use JavaScript to initialize the application with the firast tab activated.

activateTab("tab1");

Putting it all together, the full example becomes:

const tabButtons = document.querySelectorAll('[data-tab-button]');
const tabContent = document.querySelectorAll('[data-tab-content]');

const activeButtonClassName = 'border-b-2 text-black';
const inactiveButtonClassName = 'text-border-grey'

const activateTab = tabId => {
  tabButtons.forEach(tabButton => {
    const isActive = tabButton.dataset.tabButton === tabId;
    
    tabButton.className = isActive ? activeButtonClassName : inactiveButtonClassName;
  });

  tabContent.forEach(tabContent => {
    const isActive = tabContent.dataset.tabContent === tabId;

    tabContent.classList.toggle('hidden', !isActive);
  });
};

tabButtons.forEach(tabButton => {
  tabButton.addEventListener('click', (event) => {
    const activeTabId = event.target.dataset.tabButton;

        activateTab(activeTabId);
  });
});

activateTab("tab1");

* {
  box-sizing: border-box;
  border-width: 0;
  border-style: solid;
  border-color: #e5e7eb;
}

button {
  font-family: inherit;
  font-size: 100%;
  font-weight: inherit;
  line-height: inherit;
  color: inherit;
  cursor: pointer;
  margin: 0;
  padding: 0;
}

[type="button"] {
  --webkit-appearance: button;
  background-color: transparent;
  background-image: none;
}

.border-b-2 {
  border-bottom-width: 2px;
}

.hidden {
  display: none;
}

.text-black {
    --tw-text-opacity: 1;
    color: rgb(0 0 0 / var(--tw-text-opacity));
}

.text-gray-800 {
  --tw-text-opacity: 1;
  color: rgb(31 41 55 / var(--tw-text-opacity));
}

<main class="text-gray-800">
  <button class="border-b-2" data-tab-button="tab1" type="button">
    Tab 1
  </button>
  <button data-tab-button="tab2" type="button">
    Tab 2
  </button>
  <button data-tab-button="tab3" type="button">
    Tab 3
  </button>

  <div data-tab-content="tab1">
    <p>
      Tab 1 content
    </p>
  </div>
  <div data-tab-content="tab2">
    <p>
      Tab 2 content
    </p>
  </div>
  <div data-tab-content="tab3">
    <p>
      Tab 3 content
    </p>
  </div>
</main>

I have also created a fiddle for reference.