better-simple-slideshow icon indicating copy to clipboard operation
better-simple-slideshow copied to clipboard

`makeBSS()` only works with classes

Open KatieK2 opened this issue 11 years ago • 9 comments
trafficstars

It looks like makeBSS() only works to select elements by class, and not id. This actually make sense since the slideshow is vanilla.js based (not jQuery based) but it's probably worth calling out in the documentation.

KatieK2 avatar Oct 13 '14 21:10 KatieK2

I believe that it should work fine with id as well as class. Here's a test case: http://codepen.io/leemark/pen/zHAau

It's using querySelectorAll() so should work when passing in any valid CSS selector as the first argument. That's how it's meant to work anyway, is it not working as expected for you?

leemark avatar Oct 14 '14 03:10 leemark

Thanks for your test case. Here's a demo that I've been working from: http://codepen.io/KatieK2/pen/LvcyB?editors=101

After some more digging, it looks like the slide-show only works if class="bss-slides" exists on the slide container. If I remove that class from your pen, the slideshow stops working. I would have expected makeBSS() to add the appropriate class to the passed-in element.

KatieK2 avatar Oct 14 '14 21:10 KatieK2

Excellent point, an oversight on my part! Yes, the CSS is written such that the sldieshow container has to have a particular class. Like you're saying, because the CSS relies on the container element having that particular class, the js should test for that and add it if needed.

I guess I initially wanted the flexibility of people being able to be able to modify the CSS to use whatever class name they want (as long as it's a class on the container element). But that flexibility of not having the class name hardcoded into the javascript and being able to change the class on the container and in the CSS if you choose, comes at the cost of a little bit more configuration to get the slideshow up and running (i.e. having to add class="bss-slides" onto the container element). It's a trade-off either way, but I think your suggestion is a good idea. Test for that class, add it if it doesn't exist.

Thank you!

leemark avatar Oct 14 '14 23:10 leemark

Glad to help - thanks for working on this cool project!

KatieK2 avatar Oct 15 '14 03:10 KatieK2

I think you should edit the Getting started example in README.md to make explicit that people should include the bss-slides class. Nice work :)

cmolina avatar Nov 20 '14 15:11 cmolina

I may be running into a related issue as my images disappear and nothing is happening. http://codepen.io/ezwerk/pen/MaGZqY

Any input would be greatly appreciated.

ezwerk avatar Oct 29 '15 03:10 ezwerk

Hi @ezwerk - in your codepen the link to the js file is not working. Line 20 in the html. When I change the src to point to this js file instead it starts working: http://leemark.github.io/better-simple-slideshow/js/better-simple-slideshow.min.js

leemark avatar Oct 29 '15 04:10 leemark

That was it! Sheesh... had the folders mismatched. Thanks for catching that and also for getting back to me so quick!

Greatly appreciated!

ezwerk avatar Oct 30 '15 04:10 ezwerk

@ezwerk no problem, good luck! :thumbsup:

leemark avatar Oct 30 '15 16:10 leemark