DLK
DLK

Reputation: 11

Toggle elements on buttons click

Could someone show me how to shorten the code below?
I want to toggle each DIV (hidden/visible) by buttons click, and wrote this code by copy/pasting the same logic. Looks so ridiculous, and I believe it can be simplified.

PS: I did tried to use for loop but it didn't worked.
Please help me to go through this.

const btn1 = document.getElementById('category');
const hidediv1 = document.getElementById('hidecategory');

const btn2 = document.getElementById('brand');
const hidediv2 = document.getElementById('hidebrand');

const btn3 = document.getElementById('gender');
const hidediv3 = document.getElementById('hidegender');

btn1.addEventListener('click', function() {
  if (hidediv1.style.display === 'none') {
    hidediv1.style.display = 'block';
  } else {
    hidediv1.style.display = 'none';
  }
})

btn2.addEventListener('click', function() {
  if (hidediv2.style.display === 'none') {
    hidediv2.style.display = 'block';
  } else {
    hidediv2.style.display = 'none';
  }
})

btn3.addEventListener('click', function() {
  if (hidediv3.style.display === 'none') {
    hidediv3.style.display = 'block';
  } else {
    hidediv3.style.display = 'none';
  }
})

Upvotes: 0

Views: 1331

Answers (7)

Roko C. Buljan
Roko C. Buljan

Reputation: 206555

Don't hardcode arbitrary properties or elements selectors into JS - specially the ones that are already assigned in HTML. Simply read them, and use them.
Instead: make your code much more flexible by using data-* attribute.
Inside such a data-* attribute store the selector of the target Element you want to toggle.
Speaking of toggle, use the preferred way with classList.toggle()

// DOM utility functions:

const el = (sel, par) => (par || document).querySelector(sel);
const els = (sel, par) => (par || document).querySelectorAll(sel);

// Task:

const toggleElement = (selector) => el(selector).classList.toggle("u-hide");

els("[data-toggle]").forEach(elBtn => {
  elBtn.addEventListener("click", () => toggleElement(elBtn.dataset.toggle));
});

// Tips&Tricks:
// You can also call the above function programmatically 
// (without a click) like i.e:
// toggleHide("#brand");
/* Utility classes */
.u-hide { display: none; }
<button type="button" data-toggle="#category">Category</button>
<button type="button" data-toggle="#brand">Brand</button>
<button type="button" data-toggle="#gender">Gender</button>

<div id="category" class="u-hide">Category article...</div>
<div id="brand" class="u-hide">Brand article...</div>
<div id="gender" class="u-hide">Gender article...</div>

<hr>

<button type="button" data-toggle="#brand">I also toggle Brand</button>

The additional beauty of the above is in that:

  • You can reuse your buttons in a single page without the need to change a single line of JS
  • You have a nifty reusable CSS utility class: .u-hide
  • You can make a specific DIV initially visible (on page load) by simply removing the u-hide className: <div id="category">Category article visible on load</div>
  • You can programmatically call your toggleElement function like i.e: toggleElement("#someElementId") right from your code.

Upvotes: 0

zer00ne
zer00ne

Reputation: 44086

Update

Mr. Buljan brings up some valid points, see comments. Should you update your post with some HTML, I can update my answer accordingly.

5 Lines of JavaScript -- No Loops

I agree, that's stupid. Event delegation allows us to bind events on a single ancestor element (like document) and provides control over any element that is a descendent of said ancestor element whether the element is static (existing when page is loaded) or dynamic (programmatically added to the DOM).

Example Requirements

Requirement Description
<button> Any amount of <button>s static and/or dynamic
<any> Any type of element that directly proceeds a <button>
.onevent property or .addEventListener() Only one ancestor element needs to be bound to a registered event

Common Necessities Not Needed for Example

Not Needed
id, class, any attr
multiple .onevent or .addEventListener()
iteration for, for...of loops, Array methods, etc

Details are commented in example

// Bind click event to document
document.addEventListener('click', handleClick);
// Event handler passes Event Object by default
function handleClick(event) {
  // Reference the tag the user clicked
  const clicked = event.target;
  // If the user clicked a <button>
  if (clicked.matches('button')) {
    /*
    Find the element that proceeds the clicked <button> and toggle 
    .show class on it.
    */
    clicked.nextElementSibling.classList.toggle('show');
  }
}
@keyframes fadeIn {
  0% {
    opacity: 0;
  }
  100% {
    opacity: 1.0;
  }
}

@keyframes fadeOut {
  0% {
    opacity: 1.0;
  }
  100% {
    opacity: 0;
  }
}

html {
  font: 300 2ch/1 'Segoe UI'
}

body {
  display: flex;
}

fieldset {
  max-width: max-content;
  margin: 0;
  padding: 0;
  opacity: 0;
  animation: fadeOut 0.5s ease-in forwards
}

.show {
  animation: fadeIn 0.5s ease-out forwards
}

button {
  cursor: pointer;
}
<button>A</button>
<fieldset>A</fieldset>
<button>B</button>
<fieldset>B</fieldset>
<button>C</button>
<fieldset>C</fieldset>
<button>D</button>
<fieldset>D</fieldset>
<button>E</button>
<fieldset>E</fieldset>
<button>F</button>
<fieldset>F</fieldset>

Upvotes: 1

Tommeeboi
Tommeeboi

Reputation: 1

You could use a for loop like this, where the variables are constantly reassigned to each button and div, and an event listener for each button is added. You just need to change the ids of the button to be btn1, btn2, and btn3. Same with the divs, div1, div2, and div3.

for (let a = 1; a < 4; a++) {
    let btn = document.getElementById(`btn${a}`);
    let hidediv = document.getElementById(`div${a}`);

    btn.addEventListener('click', function(){
    if (hidediv.style.display === 'none'){
    hidediv.style.display = 'block';
  }
    else {
      hidediv.style.display = 'none';
    }
  })
}

Upvotes: 0

Nina Scholz
Nina Scholz

Reputation: 386776

You could take an object for changing states and a closure for the event which takes the element reference.

const
    states = { none: 'block', block: 'none' },
    toggle = element => () => element.style.display = states[elememt.style.display];


// usage
btn1.addEventListener('click', toggle(document.getElementById('hidecategory')));

Upvotes: 2

TechySharnav
TechySharnav

Reputation: 5084

Create array of ids to iterate over them and add event listener to element, and use object to toggle the display between none and block.

let displayState = {none: "block", block: "none"}
let ids = ["category", "brand", "gender"]

function toggleDisplayState(id) {
  let ele = document.getElementById("hide" + id)
  ele.style.display = displayState[ele.style.display];
}

ids.forEach(id=>{
  document.getElementById(id).addEventListener("click", ()=>{
    toggleDisplayState(id);
  })
})
<button id="category">category</button>
<div style="display:block" id="hidecategory">hidecategory</div>

<button id="brand">branch</button>
<div style="display:block" id="hidebrand">hidebrand</div>

<button id="gender">gender</button>
<div style="display:block" id="hidegender">hidegender</div>

Upvotes: 0

DecPK
DecPK

Reputation: 25406

You can add all elements in an array of array then you can look over th array and add event listener as:

btn.addEventListener( 'click', () => {
        hideDiv.style.display = hideDiv.style.display === 'none' ? 'block' : 'none';
    } );

const elementArr = [
    ['category', 'hidecategory'],
    ['brand', 'hidebrand'],
    ['gender', 'hidegender']
];

elementArr.forEach( ( [el, hideEl] ) => {
    const btn = document.getElementById( el );
    const hideDiv = document.getElementById( hideEl );

    btn.addEventListener( 'click', () => {
        hideDiv.style.display = hideDiv.style.display === 'none' ? 'block' : 'none';
    } );
} );
    <button id="category">category</button>
    <div id="hidecategory">hidecategory</div>

    <button id="brand">branch</button>
    <div id="hidebrand">hidebrand</div>

    <button id="gender">gender</button>
    <div id="hidegender">hidegender</div>

Upvotes: 0

Jonathan
Jonathan

Reputation: 7561

const elementSets = [
    { button: 'categry', div: 'hidecategory' },
    { button: 'brand', div: 'hidebrand' },
    { button: 'gender', div: 'hidegender' },
];

for (const elementSet of elementSets) {
    elementSet.button.addEventListener('click', function () {
        if (elementSet.div.style.display === 'none') {
            elementSet.div.style.display = 'block';
        }
        else {
            elementSet.div.style.display = 'none';
        }
    });
}

// or...
for (const elementSet of elementSets) {
    elementSet.button.addEventListener('click', function () {
        elementSet.div.style.display = (elementSet.div.style.display === 'none') ? 'block': 'none';
    });
}

Upvotes: 0

Related Questions