dynamicweb / swiffy-slider

Super fast carousel and slider with touch for optimized websites running in modern browsers.
MIT License
238 stars 29 forks source link

Remove html template tag from children #47

Open Teods opened 1 year ago

Teods commented 1 year ago

The problem solved with this PR is about the tag "template" who is parsed as an slide or an indicator when the library init the slider.

See issue https://github.com/dynamicweb/swiffy-slider/issues/46

nicped commented 1 year ago

Thanks for your pull. Would changing the selector to ‘*:not(template)’ template achieve the same?

Teods commented 1 year ago

I'm not sure about what part you are referring too.

Maybe you could point me on the code directly thanks :-)

nicped commented 1 year ago

Sorry - i am only on my phone as i am on vacation :-). The changes you made in js - could also just use the change of selector like your css change - original code is this: sliderElement.querySelectorAll(".slider-container>*") Just change the selector to also include the :not(template) part. I am not fan of the iteration of children on the slider element in your 2 new methods, and the tolower as I see both as performance degrations (though barely measurable). Maybe the methods should be kept, but with css query selector instead - whatever is the smallest footprint kb wise. You can also leave the pr as is - and I will take a look when back.

Teods commented 1 year ago

Sorry i'm quite busy these days and on holiday for 2 weeks tonight. I'll try to make the changes as soon as possible when i get back from holiday.