let tabs = document.getElementsByClassName('tab'); let sections = document.getElementsByClassName('section'); for(let i =0; i<tabs.length; i++){ tabs[i].onclick = tabclick } function tabclick(event){ let tab = event.target; let tabId = tab.dataset.id; for(let k =0; k<tabs.length; k++){ tabs[k].classList.remove('active'); tabs[tabId-1].classList.add('active'); sections[k].classList.remove('active'); sections[tabId-1].classList.add('active'); } } 
 .tabs{ margin: 0; padding: 0; list-style:none; cursor:pointer; } .tab{ margin: 0; padding: 0; float:left; border: 1px solid black; padding: 10px; } .tab.active { background-color: #333; color: #eee; } .output{ clear:both; } .section { display: none; } .section.active { display: block; } 
 <ul class="tabs"> <li class="tab active" data-id="1">tab1</li> <li class="tab" data-id="2">tab2</li> <li class="tab" data-id="3">tab3</li> </ul> <div class="output"> <section class="section active">section1</section> <section class="section">section2</section> <section class="section">section3</section> </div>

  • one
    you can not hang the handler on each tab, and hang 1, on a common parent. They say it will be easier for the browser - Dmytryk
  • It is not very good to tie logic to the markup. The sequence of .section can be different. Better for each .section to do a data-attribute. In addition, it is bad to attach an event through onclick . Better through addEventListener . - Stepan Kasyanenko

2 answers 2

Most of the comments relate to the use of the modern version of JavaScript instead of the old ES5.

 const tabs = document.getElementsByClassName('tab'); const sections = document.getElementsByClassName('section'); [...tabs].forEach(tab => tab.addEventListener('click', tabClick)); function tabClick(event) { const tabId = event.target.dataset.id; [...tabs].forEach((tab, i) => { tab.classList.remove('active'); sections[i].classList.remove('active'); }) tabs[tabId - 1].classList.add('active'); sections[tabId - 1].classList.add('active'); } 
 .tabs{ margin: 0; padding: 0; list-style:none; cursor:pointer; } .tab{ margin: 0; padding: 0; float:left; border: 1px solid black; padding: 10px; } .tab.active { background-color: #333; color: #eee; } .output{ clear:both; } .section { display: none; } .section.active { display: block; } 
 <ul class="tabs"> <li class="tab active" data-id="1">tab1</li> <li class="tab" data-id="2">tab2</li> <li class="tab" data-id="3">tab3</li> </ul> <div class="output"> <section class="section active">section1</section> <section class="section">section2</section> <section class="section">section3</section> </div>

  • Variables are not reassigned, so it is better to use const
  • You can replace the for loop with an array method.
  • Installing the onclick listener is best done with addEventListener
  • For function names and variable names in JS, it is customary to use camelCase
  • Unused variable let tab = event.target;
  • Adding a class .active enough to call once, so it's best to take it out of the loop.
  • in your case, you can not hang the handler on each tab, but use delegation - ThisMan
  • It depends on the case. If the content of a tab is not one element, but for example an icon + text + text, the delegation function will begin to grow to provide all the conditions. But in general, yes, in terms of performance, delegation will be the best option - Alexandr Tovmach

I see no reason to use an array at all.

Here is my implementation example:

 $('.tabs-block .tab-link').on('click', function() { if (!$(this).hasClass('active')) { var parentTabs = $(this).closest('.tabs-block'); parentTabs.find('.tab-link.active, .tab-content.active').removeClass('active'); var elemIndex = $(this).index(); $(this).addClass('active'); parentTabs.find('.tab-content').eq(elemIndex).addClass('active'); } }); 
 body {background: #333;} .tabs-block { display: block; width: 100%; border-radius: 3px; overflow: hidden; } .tab-link-block { display: block; width: 100%; background: #4184f3; overflow: hidden; } .tab-link { padding: 0 10px; height: 50px; line-height: 50px; color: #bbb; white-space: nowrap; text-overflow: ellipsis; overflow: hidden; cursor: pointer; } .tab-link:not(.active):hover { background: rgba(255, 255, 255, .1); } .tab-link.active { background: rgba(255, 255, 255, .1); color: #fff; } .tab-content-block { display: block; width: calc(100% - 20px); min-height: calc(50px - 20px); padding: 10px; background: #fefefe; color: #333; } .tab-content:not(.active) { display: none; } /* horizontal */ .tabs-block:not(.-vertical) .tab-link-block { height: 50px; } .tabs-block:not(.-vertical) .tab-link-block::after { content: ''; display: block; clear: both; } .tabs-block:not(.-vertical) .tab-link { display: inline-block; float: left; min-width: calc(50px - 20px); max-width: calc(150px - 20px); margin-right: 2px; } .tabs-block:not(.-vertical) .tab-link.active { box-shadow: 0 -3px 0 0 #f4b142 inset; } .tabs-block:not(.-vertical) .tab-link:last-child { margin-right: 0; } /* horizontal */ /* vertical */ .tabs-block.-vertical { display: grid; grid-template-columns: 150px 1fr; grid-gap: 0; } .tabs-block.-vertical .tab-link { display: block; width: calc(100% - 20px); } .tabs-block.-vertical .tab-link.active { box-shadow: -3px 0 0 0 #f4b142 inset; } /* vertical */ .tab-link { -webkit-user-select: none; -moz-user-select: none; -ms-user-select: none; transition: all linear .2s; } 
 <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script> <div class="tabs-block"> <div class="tab-link-block"> <div class="tab-link active">Первая вкладка</div> <div class="tab-link">Вторая вкладка</div> <div class="tab-link">Третья вкладка</div> </div> <div class="tab-content-block"> <div class="tab-content active">1</div> <div class="tab-content">2</div> <div class="tab-content">3</div> </div> </div> <br><br> <div class="tabs-block -vertical"> <div class="tab-link-block"> <div class="tab-link active">Первая вкладка</div> <div class="tab-link">Вторая вкладка</div> <div class="tab-link">Третья вкладка</div> </div> <div class="tab-content-block"> <div class="tab-content active">1</div> <div class="tab-content">2</div> <div class="tab-content">3</div> </div> </div>

  • First: You used jQuery, which, as it were, does not directly relate to this issue. Secondly: The fact that arrays are clearly not used does not mean that you did not use them at all. parentTabs is an array of all elements. parentTabs.find is an array search method. - Alexandr Tovmach
  • @AlexandrTovmach, you can talk a lot about JQ, so immediately close the topic. About find did not find the information that the search is conducted in the array, about the DOM - everywhere. - CbIPoK2513
  • github.com/jquery/jquery/blob/ ... github.com/jquery/jquery/blob / ... github.com/jquery/jquery/blob/… - Alexandr Tovmach