Can I change a function that uses for loop to a faster one with only pure JavaScript, (no jQuery)?

Question

I have the following function that is used to show one div and hide all others.

async function showHide(showId, hideList) {
  let ele = document.getElementById(showId);
  await hideList.forEach(item => {
    item.style.display = 'none';
  });
  ele.style.display = 'block';
}

#c2,
#c3,
#c4 {
  display: none;
}

li {
  cursor: pointer;
}

<div>
  <li onclick="showHide('c1', [c2,c3,c4]);">Show One</li>
  <li onclick="showHide('c2', [c1,c3,c4]);">Show Two</li>
  <li onclick="showHide('c3', [c1,c2,c4]);">Show Three</li>
  <li onclick="showHide('c4', [c1,c2,c3]);">Show Four</li>
  <div id="c1">Showing Item 1</div>
  <div id="c2">Showing Item 2</div>
  <div id="c3">Showing Item 3</div>
  <div id="c4">Showing Item 4</div>
</div>

The function works great, however I was wondering if there is a faster, shorter or better way to do this? Ideally all 3 of the above combined would be great.

score 3 · Accepted Answer · answered Dec 15 '22 at 07:47

It's syntactically invalid for a <li> to be a child of a <div> - a list item should only be a child of a list element. Either use one of the list elements to surround the <li>s (and only the <li>s), or use a different tag, like a button or span.

forEach does not return anything, so awaiting it doesn't help at all.

Rather than iterating through an array of the other IDs, consider selecting them all at once with a different selector method. A nice way would be to put them all in the same container, then select children of the container. You can then use the index of the clicked element in its container to determine the matching element to be shown below.

Don't use inline handlers - they're quite terrible practice. Attach listeners properly using JavaScript instead.

Instead of assigning to the style of elements directly, consider toggling a CSS class instead.

const ul = document.querySelector('ul');
const items = document.querySelectorAll('.item-container > div');
ul.addEventListener('click', (e) => {
  if (!e.target.matches('li')) return;
  const indexToShow = [...ul.children].indexOf(e.target);
  items.forEach((item, i) => {
    item.classList.toggle('show', i === indexToShow);
  });
});

li {
  cursor: pointer;
}
.item-container > div:not(.show) {
  display: none;
}

<ul>
  <li>Show One</li>
  <li>Show Two</li>
  <li>Show Three</li>
  <li>Show Four</li>
</ul>
<div class="item-container">
  <div class="show">Showing Item 1</div>
  <div>Showing Item 2</div>
  <div>Showing Item 3</div>
  <div>Showing Item 4</div>
</div>

The JavaScript is a bit longer to account for the generic functionality without hard-coding IDs, but that's the cost for making the HTML and CSS less repetitive.

Can I change a function that uses for loop to a faster one with only pure JavaScript, (no jQuery)?

1 Answers1