i am running into a problem with removing and showing divs
https://codepen.io/etrbbr/pen/raBRbQM
When I simply create two divs and toggle the hide and visible classes, everything works fine. But in my code, for some reason, it doesn't work. On the first click, it adds the classes, on the second click, it switches to the second slide, and after that, nothing happens when I click again. What am I doing wrong?
30 Replies
you are going about this the wrong way, and you will have lots and lots of pasta code in the future
you have a bug: ids are REQUIRED to be unique
the ids
next
, prev
and big-text
are repeatedEven if they don't repeat, and I replace everything with classes, JS still doesn't work. Why?
it's not the right approach
but show us the new code
So how should it have been done then?
first, you have repeated buttons
you're using
document.querySelector
which only selects 1 element: the button in the first slide
you don't have anything for the next slide
but, you don't need to repeat the buttons: the buttons should be the same but the content changes
just take the div
buttons, and put it outsidefuck yes.. so actually problem was those fucking buttons which supposed to be repeated once?
i mean like even when i used ids for them?
yes, you just need 1 set of buttons
but this still isn't the right approach, since you're reading every single slide individually
you can read all at once
also, dont use images for the buttons: use a button
what do you mean by reading every s-s. indv.
you're doing a
document.querySelector()
for each slide
instead, just grab all the divs and handle them allconst john = document.querySelector(".john");
const tanya = document.querySelector(".tanya");
so instead of this
i should have class slides
and select them like this?
const slides = document.querySelectorAll(slides);
yes
you will add more
have you imagined the INSANITY it will be when you have 5 slides?
yes i understand what you mean
just 5
because when i will have 10 slides
its impossible
exactly
an unmaintainable tangled mess
perfect spaghetti code with onion
the onion is for the tears when reading the code
but how i handle them when i choose them all. do i still give them class like .john . bla . blabla ?
because i imagined i have like 5 slides. i cant use same function idea.
nope, you can just hold a reference to the active slide
like, you keep a variable outside that has the first slide
then you can do
slide.nextSibling.toggleClass(...)
so by default i have 1 slide as active, and then a switch it every time i press button?
if there isn't a next sibling, go to the first
like if the array was infinite
ahhhh
it's easier than you would think it is
oh thats actually jquery?
not at all
vanilla javascript
i have a question. how do i position buttons relative to my div-img, if i dont use them in .tanya and .john div?
position: relative
then the with with both buttons has position absolute
yeah.. but wait, if i am not using my buttons inside my john and tanya divs.. so i actually need to use them inside or?
i mean like i used them before in my div-img
https://codepen.io/etrbbr/pen/raBRbQM
and i removed them completely and put inside main. but now i cant position them.
but if i put them in first div, when i will hide first div, they will disappear also
btw i implemented this
thanks for tips
divs.forEach((div, index) => {
div.setAttribute("data-index", index);
});
next.addEventListener("click", function () {
divs[currentIndex].classList.remove("active");
currentIndex = (currentIndex + 1) % divs.length;
divs[currentIndex].classList.add("active");
});
prev.addEventListener("click", function () {
divs[currentIndex].classList.remove("active");
currentIndex = (currentIndex - 1 + divs.length) % divs.length;
divs[currentIndex].classList.add("active");
});
that's a huge improvement
you don't need to set an attribute, since you can have a counter outside or store a reference to the element