Reputation: 477
I am trying to create in javascript a set of buttons that makes a slider swap the images depending on what button I click.
I came up with this ugly redundant code:
var changer1 = document.getElementById("changer1");
var changer2 = document.getElementById("changer2");
var changer3 = document.getElementById("changer3");
var r = document.getElementsByClassName('carousel-item');
changer1.addEventListener('click', function() {
for (let i = 0; i < r.length; i++) {
r[i].classList.remove('active')
}
r[0].classList.add('active')
});
changer2.addEventListener('click', function() {
for (let i = 0; i < r.length; i++) {
r[i].classList.remove('active')
}
r[1].classList.add('active')
});
changer3.addEventListener('click', function() {
for (let i = 0; i < r.length; i++) {
r[i].classList.remove('active')
}
r[2].classList.add('active')
});
And it worked, but I know that redundant code can be optimized, so I tried this:
var changersArray = document.getElementsByClassName('changers');
for (let i = 0; i < 3; i++) {
changer[i].addEventListener('click', function() {
for (let i = 0; i < r.length; i++) {
r[i].classList.remove('active')
}
r[0].classList.add('active')
});
}
This is the HTML:
<section class="container-fluid sectionFive">
<div class="row title_image">
<div class="col-sm-12">
<h1 class="wow flipInY delay-1s slow">Testimonials</h1>
</div>
</div>
<div class="row">
<div class="col-sm-2"></div>
<div class="col-sm-8 fetchDiv">
<div id="carouselExampleControls" class="carousel slide wow
fadeIn" data-ride="carousel">
<div class="carousel-inner">
<div class="carousel-item active">
<img class="mt-2 rounded-circle people" src="images/person_3.jpg" alt="">
<p class="frase"></p>
<p class="author"></p>
</div>
<div class="carousel-item">
<img class="mt-2 rounded-circle people" src="images/person_2.jpg" alt="">
<p class="frase"></p>
<p class="author"></p>
</div>
<div class="carousel-item">
<img class="mt-2 rounded-circle people" src="images/person_1.jpg" alt="">
<p class="frase"></p>
<p class="author"></p>
</div>
</div>
<a class="carousel-control-prev" href="#carouselExampleControls" role="button" data-slide="prev">
<span class="carousel-control-prev-icon" aria-hidden="true"></span>
<span class="sr-only">Previous</span>
</a>
<a class="carousel-control-next" href="#carouselExampleControls" role="button" data-slide="next">
<span class="carousel-control-next-icon" aria-hidden="true"></span>
<span class="sr-only">Next</span>
</a>
</div>
</div>
<div class="col-sm-2">
</div>
</div>
<div class="row changersRow">
<button class="changers" id="changer1">1</button><button class="changers" id="changer2">2</button><button class="changers" id="changer3">3</button>
</div>
</section>
I was trying to dynamically create changer1, changer2, and changer3 with by writing changer[i] inside the loop but didn't work.
Please, can you help me figure out why it does not work, and how can I make it work?
Thanks.
Upvotes: 1
Views: 185
Reputation: 2946
First two improvements that come from the top of my head:
for (let i = 0; i < 3; i++) {
changer[i].addEventListener('click', function() {
for (let j = 0; j < r.length; j++) {
r[j].classList.remove('active')
}
r[i].classList.add('active')
});
}
Notice the inner i
replaced and the r[0]
changed to use the actual index
Upvotes: 2