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

Multiple slides animation #55

Open
curdin opened this issue Oct 20, 2015 · 14 comments
Open

Multiple slides animation #55

curdin opened this issue Oct 20, 2015 · 14 comments
Labels

Comments

@curdin
Copy link

curdin commented Oct 20, 2015

I've got a setup with multiple sliders on a page all using a slide animation. The issue is that only one actually animates as the other ones seem to get their classes stripped (they still advance but without animation)
It seems the issue happens due to the addClass code in WS.goto() or possibly the removeAllHelperSettings.

Do you have any insights on what is happening there?

@peduarte
Copy link
Owner

Hey, that's interesting, I will look into it and get back to you.
Thanks

@peduarte
Copy link
Owner

I cannot seem to replicate your issue.
I've created this Pen to test it out: http://codepen.io/peduarte/pen/gaoEXz

Can you show me how you're getting this problem?

@peduarte
Copy link
Owner

Closing as I am unable to replicate your issue, feel free to re open or ask any questions.

@curdin
Copy link
Author

curdin commented Oct 22, 2015

@peduarte Apologies - i was away from a computer for a couple of days. I've updated the codepen to demostrate the issue. If you click the Advance both button you'll see that the left one fails to use a transition.
http://codepen.io/anon/pen/xwYLNJ

@curdin
Copy link
Author

curdin commented Oct 22, 2015

(I seem to be unable to reopen this issue)

@peduarte peduarte reopened this Oct 23, 2015
@peduarte peduarte added the bug label Oct 23, 2015
@peduarte
Copy link
Owner

Wow, amazing find!
Will look into it

@afmarchetti
Copy link

Hi guys Did you solve the issue? I have to slider with autoplay and the animations doesn't work properly

@xaddict
Copy link

xaddict commented Apr 14, 2016

I found this issue too when using the autoplay example but changing it to autoplay 2 sliders. The classes 'hideprevious' and 'shownext' get added to only 1 of the sliders, not all.

@peduarte
Copy link
Owner

Hey guys, so sorry for the delay on this. I promise I'll look into it as soon as I can. 🙏

@benjaminpkane-zz
Copy link

benjaminpkane-zz commented May 7, 2016

Here

  // Reset all settings by removing classes and attributes added by goTo() & updateButtonStates()
  WS.removeAllHelperSettings = function () {
    removeClass(this.allItemsArray[this.currentItemIndex], this.options.currentItemClass);
    removeClass($$(this.options.hidePreviousClass)[0], this.options.hidePreviousClass);
    removeClass($$(this.options.hideNextClass)[0], this.options.hideNextClass);
    removeClass($$(this.options.showPreviousClass)[0], this.options.showPreviousClass);
    removeClass($$(this.options.showNextClass)[0], this.options.showNextClass);

    if (!this.buttonPrevious && !this.buttonNext) { return; }

    this.buttonPrevious.removeAttribute('disabled');
    this.buttonNext.removeAttribute('disabled');
  };

and here

  // Helper functions
  function $$(element) {
    if (!element) { return; }
    return document.querySelectorAll('.' + element);
  }

$$ is taking element, but it's just a string of a css class. The if statement in $$ is pointless.

document.querySelectorAll is querying the entire DOM. Also, it will always return an array, empty or not. 0 indexing an empty array returns null.

removeAllHelperSettings is basically just going to town on any DOM element with those classes.

@Djules
Copy link
Contributor

Djules commented Jul 1, 2016

I've extended the $$ helper function in order to query within a given container instead of the entire DOM, and fixed the OP issue as well.

@peduarte
Copy link
Owner

@Djules Thanks, I'll take a look at the PR. What's the OP issue?

@Djules
Copy link
Contributor

Djules commented Jul 13, 2016

OP stands for Original Poster, @curdin in this case :)

@peduarte
Copy link
Owner

@Djules Ahhh ok! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants