Fonction simple de basculement javascript div

Question

J'ai écrit un code qui déclenche ma div avec un autre menu. Je ne sais pas lequel est le meilleur et doit être utilisé. Puis-je avoir des commentaires ?

Premier code dans une fonction :

function sidebar() {
var menu = document.querySelector('#sidebar-container');
      var menuSmall = document.querySelector('#sidebar-container2');
      var icon = document.getElementById('right');
      menuSmall.style.display = "block";
      menu.style.display = "none";
      if(menu.style.display == "none") {
        icon.onclick = function () {
          menu.style.display = "block";
          menuSmall.style.display = "none";
}
}

Deuxième code avec deux fonctions mais fait le même travail :

function sidebar() {
      var menu = document.querySelector('#sidebar-container');
      var menuSmall = document.querySelector('#sidebar-container2');
      menuSmall.style.display = "block";
      menu.style.display = "none";
        
    },
    function openbar() {
      var menu = document.querySelector('#sidebar-container');
      var menuSmall = document.querySelector('#sidebar-container2');
      menuSmall.style.display = "none";
      menu.style.display = "block";
    }

Answer 1

Je pense qu'il est plus facile d'écrire et de comprendre le changement d'état si vous n'avez pas besoin de penser aux cas extrêmes.

Dans le premier cas, vous démarrez déjà dans l'état que vous souhaitez et n'avez qu'à inverser les deux états. Dans le second, il vous suffit de "désactiver" tous les états et "d'activer" celui que vous souhaitez.

Je pense que c'est plus simple si vous pensez de cette façon.

Deux barres latérales

Je pense que vous pouvez utiliser DOMTokenList.toggle() :

const sidebars = document.querySelectorAll('.sidebar');
const icon = document.getElementById('icon');

const toggle = el => el.classList.toggle('hidden');

icon.addEventListener('click',() => sidebars.forEach(toggle));

/* the display value of a div is block by default */
.hidden {
  display: none;
}

<!-- you don't need to find which sidebar is open in your js -->
<div class="sidebar">first</div>
<div class="sidebar hidden">second</div>
<span id="icon">icon</span>

Documents :

• DOMTokenList
• Element.classList
• DOMTokenList.forEach()

---

Plus de deux barres latérales

Peut-être que la bascule ne peut pas être utile dans ce cas. Cependant, je pense que les ensembles de données pourraient aider

const sidebars = document.querySelectorAll('.sidebar');
const btns = document.querySelectorAll('.btn');

const getSidebar = btn => btn.dataset.sidebar;

const hide = el => el.classList.add('hidden');

btns.forEach(btn =>
  btn.addEventListener('click', () => {
    const target = document.getElementById(getSidebar(btn));
    sidebars.forEach(hide);
    target.classList.remove('hidden');
  })
);

.hidden {
  display: none;
}

<div class="sidebar" id="first">sidebar 1</div>
<div class="sidebar hidden" id="second">sidebar 2</div>
<div class="sidebar hidden" id="third">sidebar 3</div>
<button class="btn" data-sidebar="first">1</button>
<button class="btn" data-sidebar="second">2</button>
<button class="btn" data-sidebar="third">3</button>

---

Answer 2

Si je devais choisir entre vos 2 méthodes, j'opterais pour la deuxième méthode.

Vous devez toujours pécher par excès de lisibilité et aussi le principe de responsabilité unique .

Mais si je devais suggérer une alternative à votre code d'origine, je proposerais l'utilisation de classes CSS.

function toggleMenu(selectedMenu) {
  // Grab the currently open menu
  var openMenu = document.querySelector('.js-menu-open');
  
  // Grab the menu you want to open
  var targetMenu = document.querySelector(selectedMenu);

  // Scenario 1: a menu is already open
  // Close the open menu.
  if(openMenu) {
    openMenu.classList.replace('js-menu-open', 'js-menu-closed');
  }
  
  // Secnario 2: if the menu you are trying to open isn't the currently open menu
  // Open the target menu.
  if(openMenu !== targetMenu) {
    targetMenu.classList.replace('js-menu-closed', 'js-menu-open');
  }
  
}

.js-menu-closed {
  display: none;
}

.js-menu-open {
  display: block;
}

<div id="sidebar-container" class="js-menu-closed">
  Sidebar 1
</div>

<div id="sidebar-container2" class="js-menu-closed">
  Sidebar 2
</div>

<div id="sidebar-container3" class="js-menu-closed">
  Sidebar 3
</div>

<button onclick="toggleMenu('#sidebar-container')">Toggle sidebar 1</button>
<button onclick="toggleMenu('#sidebar-container2')">Toggle Sidebar 2</button>
<button onclick="toggleMenu('#sidebar-container3')">Toggle sidebar 3</button>

Cette méthode utilise essentiellement les classes CSS comme une sorte d'état persistant, ce qui permet de simplifier votre logique JS, et réduit également la répétition de votre code (en le gardant DRY ) ET il est réutilisable à l'infini pour autant de menus que vous le souhaitez ! :)

L'autre avantage est que vous pouvez désormais choisir d'ouvrir l'un de vos menus par défaut et d'appliquer des animations plus performantes en CSS.

J'espère que cela a aidé.