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?
df
CodePen
Untitled
...
30 Replies
ἔρως
ἔρως3w ago
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 repeated
Helgi
HelgiOP3w ago
Even if they don't repeat, and I replace everything with classes, JS still doesn't work. Why?
ἔρως
ἔρως3w ago
it's not the right approach but show us the new code
Helgi
HelgiOP3w ago
So how should it have been done then?
ἔρως
ἔρως3w ago
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 outside
Helgi
HelgiOP3w ago
fuck yes.. so actually problem was those fucking buttons which supposed to be repeated once? i mean like even when i used ids for them?
ἔρως
ἔρως3w ago
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
Helgi
HelgiOP3w ago
what do you mean by reading every s-s. indv.
ἔρως
ἔρως3w ago
you're doing a document.querySelector() for each slide instead, just grab all the divs and handle them all
Helgi
HelgiOP3w ago
const 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);
ἔρως
ἔρως3w ago
yes you will add more have you imagined the INSANITY it will be when you have 5 slides?
Helgi
HelgiOP3w ago
yes i understand what you mean
ἔρως
ἔρως3w ago
just 5
Helgi
HelgiOP3w ago
because when i will have 10 slides its impossible
ἔρως
ἔρως3w ago
exactly an unmaintainable tangled mess perfect spaghetti code with onion the onion is for the tears when reading the code
Helgi
HelgiOP3w ago
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.
ἔρως
ἔρως3w ago
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(...)
Helgi
HelgiOP3w ago
so by default i have 1 slide as active, and then a switch it every time i press button?
ἔρως
ἔρως3w ago
if there isn't a next sibling, go to the first like if the array was infinite
Helgi
HelgiOP3w ago
ahhhh
ἔρως
ἔρως3w ago
it's easier than you would think it is
Helgi
HelgiOP3w ago
oh thats actually jquery?
ἔρως
ἔρως3w ago
not at all vanilla javascript
Helgi
HelgiOP3w ago
i have a question. how do i position buttons relative to my div-img, if i dont use them in .tanya and .john div?
ἔρως
ἔρως3w ago
position: relative then the with with both buttons has position absolute
Helgi
HelgiOP3w ago
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?
Helgi
HelgiOP3w ago
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
df
CodePen
Untitled
...
Helgi
HelgiOP3w ago
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"); });
ἔρως
ἔρως3w ago
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

Did you find this page helpful?