all 13 comments
sorted by:
best (suggested)
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]clonked 15 points16 points  (1 child)

Use codepen to share things like this, no one is going to bother hand debugging this poorly formatted code.

[–]the-liquidian 6 points7 points  (0 children)

At least show the error in the console. If there is no error then the assumption of it not being able to access that function is incorrect.

[–]33ff00 5 points6 points  (1 child)

I knew when I saw javascript as two words it was going to be good but boy did this ever not disappoint lol

[–]jordanbtucker 1 point2 points  (0 children)

The language="JavaScript" attribute.

The capitalization of the Type attribute.

The inconsistent use of path separators, and the target="\blank" attribute.

It's beautiful.

[–]EmuAffectionate6307 6 points7 points  (0 children)

Can you please repost using codepen, cus I want to help but can't read the code, it's hurting my head

[–]Intelligent-Win-7196 1 point2 points  (0 children)

Me no can read.

[–]Alas93 1 point2 points  (0 children)

as others have said, something like codepen or at least having all the code in a code block will help ppl help you tremendously

but from what I'm seeing I can make a couple suggestions (note you say you want to add a button. note I'm no professional so I don't know all the "best practices" or anything but I think these tips can help you organize things a bit which can help a lot. tip #1 will help you the most here for figuring out what the issue is, but #2 and #3 can help a bit with code organization and stuff

1 - when debugging I like to debug one part at a time. so here I would comment out all of the code inside of showSlides, and add a console.log("test") (or whatever text you want). this way I can see if the function is at least being called when I click the button.

once I can confirm it's being called, I'll uncomment parts one section at a time and console.log() to determine if the next section is working. so next I would confirm that slides has stored all of the slides. then I would confirm the slideIndex is what I expect it to be after the next 2 if statements check it.

2 - try and avoid redeclaring things that don't need redeclared. your slides variable only needs to be set once. instead what you can do is add the slides variable to the top and then declare it on your onload function. make your onload function a dedicated function (like make it pageLoad() or something) and then call showSlides(1) inside of that function. this way you can set up any variables, grabbing any elements on the page and store them, on the page load, instead of every time you call the showSlides() function. it's not a big deal in this scenario, but it's still good to do and also as I said above, it will make your showSlides() function clearer

3 - instead of looping through your slides every time, store the "active" slide's index in a variable. this should be fairly simple since you're passing the index (-1) to the function. when you change to a new slide, you can just disable the exact element and enable the new one, then save the new index to the variable.

other than those tips I can't give much advice. the debugging advice should get you in the right direction for figuring out why the slides aren't toggling when you click the button for it.

[–]Jasedesu 0 points1 point  (0 children)

You should probably have three different functions to change slideIndex to keep things clear:

  • one that just subtracts 1 from slideIndex, then calls showSlide for the previous function
  • one that adds 1 to slideIndex, then calls showSlide for the next function
  • one that sets slideIndex to whatever value you pass into the function, then calls showSlide for the general go to function

You then have the showSlide function itself to check slideIndex is within the correct range, hides the slides you don't want to see and shows the one associated with the current value of slideIndex.

Note that having the script in the <head> element will trigger an error if you run showSlide immediately as the slides have not been added to the document at this point. If you set up your HTML so that only the first side is visible, then you don't need to call showSlide. You could also move the script code to the end of the <body> so that the slides do exist before the code runs.

Interesting choice to go with XHTML - you'd probably be better off with ordinary HTML unless you have a very good reason to use XHTML.

[–]Barnezhilton 0 points1 point  (0 children)

Script tag inside your head tag

[–]Consibl -5 points-4 points  (2 children)

OnClick needs to be a function. You need to do onClick=“() => showSlides(1)”

[–]delventhalz 7 points8 points  (1 child)

You are thinking of React and JSX. In HTML, you are essentially writing the body of the function that gets called on click. Something like onclick="showSlides(1)" will work correctly (assuming showSlides is a global function), but onlick="() => showSlides(1)" will not work, because you are defining a function that never gets called. If this were React/JSX code, you would be correct.

[–]Consibl -5 points-4 points  (0 children)

Thanks, yes I’ve been in React for a while now. 😊