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

[BUG] slickNext, slickPrev and slickGoTo broken when focusOnSelect={true} and fade={false} #1949

Open
felixmeziere opened this issue Dec 15, 2020 · 3 comments

Comments

@felixmeziere
Copy link

felixmeziere commented Dec 15, 2020

The solution to this issue #1074 which was solved in this commit NickIannelli@30d5c2f seems to now break the three methods slickNext, slickPrev and slickGoTo.

Actual behaviour: when calling slickNext on the first slide, the slider immediately jumps to the 2nd slide and then slides to the third slide.
Expected behaviour: just slide to the second slide.

This only happens if fade={false} and focusOnSelect={true}.

Fix is to remove the setTimeouts on the methods (seems like a weird solution anyway to be honest, that doesn't seem like a great way to solve race condition :/).

I'm not sure on how to solve this if the "use slider in componentDidMount" usecase is important (shouldn't we use the initialSlide prop instead? Or a setTimeout particular to the application that waits a little bit before navigating? I suggest removing the setTimeouts in the code here...).

I don't have time for a PR but if it's ok to not handle the usecase above then in the library the setimeouts here should be removed:

slickPrev = () => {
    // this and fellow methods are wrapped in setTimeout
    // to make sure initialize setState has happened before
    // any of such methods are called
    this.callbackTimers.push(
      setTimeout(() => this.changeSlide({ message: "previous" }), 0)
    );
  };
  slickNext = () => {
    this.callbackTimers.push(
      setTimeout(() => this.changeSlide({ message: "next" }), 0)
    );
  };
  slickGoTo = (slide, dontAnimate = false) => {
    slide = Number(slide);
    if (isNaN(slide)) return "";
    this.callbackTimers.push(
      setTimeout(
        () =>
          this.changeSlide(
            {
              message: "index",
              index: slide,
              currentSlide: this.state.currentSlide
            },
            dontAnimate
          ),
        0
      )
    );
  };

Sandbox : https://codesandbox.io/s/react-slick-playground-forked-cdbf8?file=/index.js

Otherwise it's a problem with focusOnSelect, no time to investigate more now sorry :/

@felixmeziere felixmeziere changed the title [BUG] slickNext, slickPrev and slickGoTo broken when focusOnSelect={true} and fade={false} [BUG] slickNex`, slickPrev and slickGoTo broken when focusOnSelect={true} and fade={false} Dec 15, 2020
@felixmeziere felixmeziere changed the title [BUG] slickNex`, slickPrev and slickGoTo broken when focusOnSelect={true} and fade={false} [BUG] slickNext, slickPrev and slickGoTo broken when focusOnSelect={true} and fade={false} Dec 15, 2020
@thinkanddoit
Copy link

Can i try this?

@VadimSvirdoff
Copy link

@felixmeziere could you share example with bug in https://codesandbox.io
In this example all work right https://react-slick.neostack.com/docs/example/previous-next-methods

@felixmeziere
Copy link
Author

@VadimSvirdoff I put a sandbox above with the bug reproduced, what is missing from it to be useful? :-)

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

No branches or pull requests

3 participants