Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove html template tag from children #47

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Teods
Copy link

@Teods Teods commented Jul 8, 2022

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 #46

@nicped
Copy link
Member

nicped commented Jul 8, 2022

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

@Teods
Copy link
Author

Teods commented Jul 11, 2022

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

  • If we only change the CSS selector and not the javascript code (for me you need to change both because the javascript will parse the template object and this is not necessary in the slider case) ?
  • Or do you want to apply the same logic for javascript (but for me the *:not(template) only work in CSS not in Javascript) ?

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

@nicped
Copy link
Member

nicped commented Jul 11, 2022

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
Copy link
Author

Teods commented Jul 15, 2022

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.

@Teods Teods force-pushed the remove-html-template-tag-from-children branch from fdaae29 to 41846c7 Compare January 20, 2023 17:48
Thibaut Faucher and others added 3 commits January 20, 2023 20:42
Fix error when auto slide, the slider was sliding by page instead of by slide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants