Marko Masulovic
Marko Masulovic

Reputation: 1

What's wrong with my code, click event fires only once JavaScript?

The loop works only once and then nothing happens. I have three testimonials, and can go only once forward or backwords.Thanks for help!

const nextBtn = document.querySelector(".next-btn");
const prevBtn = document.querySelector(".prev-btn");
const testimonials = document.querySelectorAll(".testimonial");

let index = 0;
window.addEventListener("DOMContentLoaded", function () {
  show(index);
});
function show(index) {
  testimonials.forEach((testimonial) => {
    testimonial.style.display = "none";
  });
  testimonials[index].style.display = "flex";
}

nextBtn.addEventListener("click", function () {
  index++;
  if (index > testimonials.length - 1) {
    index = 0;
  }
  show(index);
});

prevBtn.addEventListener("click", function () {
  index--;
  if (index < 0) {
    index = testimonials.length - 1;
  }
  show(index);
});

Upvotes: 0

Views: 87

Answers (1)

Mr. Polywhirl
Mr. Polywhirl

Reputation: 48600

I would use a "hidden" class to hide the non-active testimonials instead of manipulating the element's style in-line. Also, your navigation logic can be simplified to a modulo operation.

The code your originally posted seemed to work out well, but it seems to cluttered with redundancy (code reuse). It also lacks structural flow (readability).

const
  modulo = (n, m) => (m + n) % m,
  moduloWithOffset = (n, m, o) => modulo(n + o, m);

const
  nextBtn = document.querySelector('.next-btn'),
  prevBtn = document.querySelector('.prev-btn'),
  testimonials = document.querySelectorAll('.testimonial');

let index = 0;

const show = (index) => {
  testimonials.forEach((testimonial, currIndex) => {
    testimonial.classList.toggle('hidden', currIndex !== index)
  });
}

const navigate = (amount) => {
  index = moduloWithOffset(index, testimonials.length, amount);
  show(index);
}

// Create handlers
const onLoad = (e) => show(index);
const onPrevClick = (e) => navigate(-1);
const onNextClick = (e) => navigate(1);

// Add handlers
window.addEventListener('DOMContentLoaded', onLoad);
nextBtn.addEventListener('click', onNextClick);
prevBtn.addEventListener('click', onPrevClick);
.testimonial {
  display: flex;
}

.testimonial.hidden {
  display: none;
}
<div>
  <button class="prev-btn">Prev</button>
  <button class="next-btn">Next</button>
</div>
<div>
  <div class="testimonial">A</div>
  <div class="testimonial">B</div>
  <div class="testimonial">C</div>
  <div class="testimonial">D</div>
  <div class="testimonial">E</div>
  <div class="testimonial">F</div>
</div>

Upvotes: 1

Related Questions