1

I would like to simplify my code, to use the same function, for the same class, but with only triggering the one that's being clicked on (changing the text). With plain JavaScript. I got it working now, but I know it can be simpler, and reusable, even if I were to use 100 of the same thing.

Could you help me simplify and improve the below code?

Code:

// Change the title of dropdown to show/hide    
// 1. create function to change text
const changeText = function changeText() {
  if (this.innerHTML == "Show details") {
    this.innerHTML = "Hide details"
  } else {
    this.innerHTML = "Show details"
  }
};

// 2. add click event on element to trigger text change
document.querySelector('summary.priceSummary').onclick = changeText;
document.querySelector('summary.priceSummary2').onclick = changeText;
document.querySelector('summary.priceSummary3').onclick = changeText;
<details>
  <summary class="priceSummary">Show details</summary>
  <ul>
    <li>test</li>
    <li>test</li>
    <li>test</li>
    <li>test</li>
  </ul>
</details>

<details>
  <summary class="priceSummary2">Show details</summary>
  <ul>
    <li>test</li>
    <li>test</li>
    <li>test</li>
    <li>test</li>
  </ul>
</details>

<details>
  <summary class="priceSummary3">Show details</summary>
  <ul>
    <li>test</li>
    <li>test</li>
    <li>`enter code here`test</li>
    <li>test</li>
  </ul>
</details>
Barmar
  • 741,623
  • 53
  • 500
  • 612
Bart
  • 149
  • 1
  • 16
  • 1
    [Related](/q/51573435/4642212). Use [event delegation](//developer.mozilla.org/docs/Learn/JavaScript/Building_blocks/Events#Event_delegation) instead of assigning multiple event listeners — it’s more maintainable, and applies to dynamically added elements. E.g., use an [event argument](//developer.mozilla.org/docs/Web/API/EventTarget/addEventListener#The_event_listener_callback)’s [`target`](//developer.mozilla.org/docs/Web/API/Event/target). See [the tag info](/tags/event-delegation/info) and [What is DOM Event delegation?](/q/1687296/4642212). – Sebastian Simon Jan 01 '22 at 16:51
  • Note that a [``](//html.spec.whatwg.org/multipage/interactive-elements.html#the-summary-element) represents a _summary, caption, or legend_ for the rest of the contents of the `
    ` tag. _“Show details”_ and _“Hide details”_ do not do that. A `title` attribute with these calls to action may be used instead.
    – Sebastian Simon Jan 01 '22 at 16:54

2 Answers2

2

Give all your elements the same class, then use document.querySelectorAll() and loop over them.

// Change the title of dropdown to show/hide    
// 1. create function to change text
const changeText = function changeText() {
  if (this.innerHTML == "Show details") {
    this.innerHTML = "Hide details"
  } else {
    this.innerHTML = "Show details"
  }
};

// 2. add click event on element to trigger text change
document.querySelectorAll('summary.priceSummary').forEach(el => el.addEventListener("click", changeText));
<details>
  <summary class="priceSummary">Show details</summary>
  <ul>
    <li>test</li>
    <li>test</li>
    <li>test</li>
    <li>test</li>
  </ul>
</details>

<details>
  <summary class="priceSummary">Show details</summary>
  <ul>
    <li>test</li>
    <li>test</li>
    <li>test</li>
    <li>test</li>
  </ul>
</details>

<details>
  <summary class="priceSummary">Show details</summary>
  <ul>
    <li>test</li>
    <li>test</li>
    <li>`enter code here`test</li>
    <li>test</li>
  </ul>
</details>
Barmar
  • 741,623
  • 53
  • 500
  • 612
1

Just select all summary elements and iterate over them instead of selecting the priceSummary2 etcs.

for (const summary of document.querySelectorAll('summary')) {
  summary.onclick = () => summary.textContent = summary.textContent =
    'Show details' ? 'Hide details' : 'Show details';
}

If you have other summary elements, then give these a class name in common - eg, call them all priceSummary, and then you can do querySelectorAll('.priceSummary') instead.

Another option is to toggle a class, and have CSS rules show or hide the "Hide details" or "Show details" element appropriately.

for (const summary of document.querySelectorAll('.priceSummary')) {
  summary.addEventListener('click', () => summary.classList.toggle('show'));
}
.priceSummary:not(.show) > :first-child,
.priceSummary.show > :nth-child(2) {
  display: none;
}
<details>
  <summary class="priceSummary show"><span>Show</span><span>hide</span> details</summary>
  <ul>
    <li>test</li>
    <li>test</li>
    <li>test</li>
    <li>test</li>
  </ul>
</details>

<details>
  <summary class="priceSummary show"><span>Show</span><span>hide</span> details</summary>
  <ul>
    <li>test</li>
    <li>test</li>
    <li>test</li>
    <li>test</li>
  </ul>
</details>

<details>
  <summary class="priceSummary show"><span>Show</span><span>hide</span> details</summary>
  <ul>
    <li>test</li>
    <li>test</li>
    <li>`enter code here`test</li>
    <li>test</li>
  </ul>
</details>
CertainPerformance
  • 356,069
  • 52
  • 309
  • 320