0

I have 3 buttons. They all toggle different content with css option display: none; display: flex; With javascript i toggle between the class .show For each button i have a toggle script.

It detects if another button is allready opened but it does not work properly. If i click button 1 and then 2 it works. But if i hit number 3 after that, it removes everything instead of toggle the right class.

Also i find this to be a very long code for something that can most likely be done with less.

Im happy with less code but the point of the question is so that i understand what ive done wrong.

// Button 1
let b1 = document.querySelector(".button1");
let b1_toggle = document.querySelector(".toggle-button-1");
b1.addEventListener("click", function(){
    let b2_toggle = document.querySelector(".toggle-button-2");
    let b3_toggle = document.querySelector(".toggle-button-3");
    let alreadyOpen = false;
    if (b2_toggle.classList.contains("show"), b3_toggle.classList.contains("show")) alreadyOpen = true;
    b2_toggle.classList.remove("show");
    b3_toggle.classList.remove("show");
    if (!alreadyOpen)b1_toggle.classList.toggle("show");
});
// Button 2
let b2 = document.querySelector(".button2");
let b2_toggle = document.querySelector(".toggle-button-2");
b2.addEventListener("click", function(){
    let b1_toggle = document.querySelector(".toggle-button-1");
    let b3_toggle = document.querySelector(".toggle-button-3");
    let alreadyOpen = false;
    if (b1_toggle.classList.contains("show"), b3_toggle.classList.contains("show")) alreadyOpen = true;
    b1_toggle.classList.remove("show");
    b3_toggle.classList.remove("show");
    if (!alreadyOpen)b2_toggle.classList.toggle("show");
});
// Button 3
let b3 = document.querySelector(".button3");
let b3_toggle = document.querySelector(".toggle-button-3");
b3.addEventListener("click", function(){
    let b1_toggle = document.querySelector(".toggle-button-1");
    let b2_toggle = document.querySelector(".toggle-button-2");
    let alreadyOpen = false;
    if (b1_toggle.classList.contains("show"), b2_toggle.classList.contains("show")) alreadyOpen = true;
    b1_toggle.classList.remove("show");
    b2_toggle.classList.remove("show");
    if (!alreadyOpen)b3_toggle.classList.toggle("show");
});
.flex {
  display: flex;
}
.button {
  background-color: red;
  color: white;
  padding: 10px 20px;
}
.toggle-button-1 { display: none;}
.toggle-button-1.show { display: flex;}
.toggle-button-2 { display: none;}
.toggle-button-2.show { display: flex;}
.toggle-button-3 { display: none;}
.toggle-button-3.show { display: flex;}
<div class="flex">
  <div class="button button1">Button1</div>
  <div class="button button2">Button2</div>
  <div class="button button3">button3</div>
</div>

<div class="toggle-button-1">Button 1 toggled</div>
<div class="toggle-button-2">Button 2 toggled</div>
<div class="toggle-button-3">Button 3 toggled</div>
RaRa Ritalin
  • 157
  • 12
  • 2
    It seems you're confusing the [comma operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Comma_Operator) for [logical and](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_AND)... – Siguza Feb 06 '21 at 01:45
  • Also, programming is not about copy/pasting code for every button. Imagine you had 100 buttons... you need only one function not hundred. Also don't mistake classes for IDs. ClassNames should be common selectors, ID should be unique! – Roko C. Buljan Feb 06 '21 at 01:48
  • @Siguza turn it into an answer and ill give you the credit. – RaRa Ritalin Feb 06 '21 at 01:52
  • @RokoC.Buljan indeed. But since im pretty new to javascript this is all i could come up with. If you have any tips or suggestion i would love to hear them – RaRa Ritalin Feb 06 '21 at 01:54
  • 1
    I think the proper course of action is to close as a duplicate: [Why does javascript accept commas in if statements?](https://stackoverflow.com/questions/5347995/why-does-javascript-accept-commas-in-if-statements) – Siguza Feb 06 '21 at 01:55

1 Answers1

1

the "right" way for codding that:

const toggleButtons = document.querySelector('#toggle-buttons')

document.querySelector('#buttons').onclick = ({target}) =>
  {
  if (!target.matches('div[data-button]')) return

  let actualButton = toggleButtons.className
    , targetButton = target.dataset.button
    ;
  toggleButtons.className = (actualButton===targetButton) 
                          ?  '' 
                          : targetButton
  }
.flex {
  display : flex;
  }
#buttons > div[data-button] {
  background-color : red;
  color            : white;
  padding          : 10px 20px;
  }
#toggle-buttons > div { 
  display : none;
  }
#toggle-buttons.b1 > .toggle-button-1,
#toggle-buttons.b2 > .toggle-button-2, 
#toggle-buttons.b3 > .toggle-button-3 { 
  display : flex;
  }
<div id="buttons" class="flex">
  <div data-button="b1" > Button1 </div>
  <div data-button="b2" > Button2 </div>
  <div data-button="b3" > button3 </div>
</div>

<div id="toggle-buttons"> 
  <div class="toggle-button-1">Button 1 toggled</div>
  <div class="toggle-button-2">Button 2 toggled</div>
  <div class="toggle-button-3">Button 3 toggled</div>
</div>
Mister Jojo
  • 20,093
  • 6
  • 21
  • 40