React-responsive-carousel: selectedItem retains control when an `onChange()` sets it manually

[mhesham32]

<Carousel autoPlay={true} 
            interval={10000}
             showThumbs={false}
              infiniteLoop={true}
              showStatus={false}
              onChange={()=>this.setState({changed:!this.state.changed})}
              dynamicHeight={true}
             >

[leandrowd]

Yes, you can. The index of the selected item will be passed as argument.

[mhesham32]

sorry I don't understand it doesn't work as I expected when I set the state can you explain more please?

[leandrowd]

What happens when you do it?

[mhesham32]

the autoplay doesn't work and it only display the first two slides when I change them manually

[leandrowd]

I need to see an example to be able to help. However, I won't be close to a computer for the next 2 weeks so there is not much I can do other than maybe point you to the right thing. Sorry! If you find the problem in the code and wish to submit a PR, I can review and publish it...

[mhesham32]

no problem and thanks for your time but what did you mean with The index of the selected item will be passed as argument. ? if you have time

[leandrowd]

I meant that onChange will receive the index of the selected item after a change.

[leandrowd]

Have you seen the examples using storybook?

[mhesham32]

no

[mhesham32]

I just read the docs on npm

[leandrowd]

Check these demos: http://react-responsive-carousel.js.org/storybook/?selectedKind=Carousel&selectedStory=with%20external%20controls&full=0&down=1&left=1&panelRight=0&downPanel=kadirahq%2Fstorybook-addon-actions%2Factions-panel

You might find an example solving your problem if you look at the source of the stories

[dschinkel]

@leandrowd this is annoying you should show examples and provide basic changing of selected item handler code. People at least just want some default basic stuff to work out of the box like clicking the arrow to the next item.

[leandrowd]

@dschinkel please go through the readme and you will find all the answers to your questions. The carousel works out of the box of you follow the basic example and allows customisation when you need. If you find what you need in one of the storybooks, the code is available in the stories folder.

[fxalvarezd]

Same thing happens to me.
When I try to setState using the OnChange prop, the carousel resets after the second slide.

class Home extends React.Component {
  state = {
    selectedPage: 0,
  };

  setSelectedPage = index => {
    console.log(('Index:', index));
    this.setState({
      selectedPage: index,
    });
  };

  render() {
    return (
      <div>
        <div className={s.home}>
          <Carousel
            showStatus={false}
            showIndicators={false}
            showThumbs={false}
            onChange={this.setSelectedPage}
          >
            <div className={cx(s.slide, s.one)} />
            <div className={cx(s.slide, s.two)} />
            <div className={cx(s.slide, s.three)} />
            <div className={cx(s.slide, s.two)} />
          </Carousel>
        </div>

        <Navigation
          selectedPage={this.state.selectedPage}
          onMenuClick={this.setSelectedPage}
        />
      </div>
    );
  }
}

[Cyan005]

I set the value as such for the onClickItem:

onClickItem={(index) => { this.clickBanner(index); }}

 private clickBanner(index): void {
    window.location.href = this.buildNewsLink(this.state.newsBannerItems[index]);
  }

I'm not setting the state but in many other instances in React of the same process, works as expected.

[fxalvarezd]

The problem is with setState @Cyan005, but thanks for the input ;)

[Cyan005]

Alright yeah... I setup the exact same scenario and was able to recreate the issue you're seeing. Spent a few minutes debugging to see where the problem was living and where I'm getting stuck at (for now) is when a setState is declared within our onChange handler, the following block of code is being used in the Carousel component:

key: 'componentWillReceiveProps',
        value: function componentWillReceiveProps(nextProps) {
            if (nextProps.selectedItem !== this.state.selectedItem) {
                this.updateSizes();
                this.moveTo(nextProps.selectedItem);
            }

When the setState on our end is not used, this is not being called. The nextProps.selectedItem is always 0. On the first click the condition is not met as both the nextProps.selectedItem is 0 and the this.state.selectedItem is 0. This works.

However, upon a subsequent onChange, the next pass the nextProps.selectedItem is 0 and the this.state.selectedItem is 1. This puts you into the condition block. And then it uses nextProps.selectedItem which is 0 to move back to position 0. It's as if the nextProps is not updating correctly.

[fxalvarezd]

I think there's a race condition here.
setState is async and React batches them to improve performance.

I noticed that in this method

        _this.selectItem = function (state) {
            _this.setState(state);
            _this.handleOnChange(state.selectedItem, _this.props.children[state.selectedItem]);
        };

handleOnChange is being called before the state is actually set.
It should probably look something like this:

        _this.selectItem = function (state) {
            _this.setState(state, () => {
                _this.handleOnChange(state.selectedItem, _this.props.children[state.selectedItem]);
            });
        };

But this doesn't really fix it.
Will try to look into it more when I have time, but those are my two cents ;)

[Cyan005]

Yeah, I was all over those functions. Unfortunately through debugging, all of the numbers passed in and out of those were exactly as expected. The only 'off' thing I could find was the code block I pasted in my previous message so far.

[fxalvarezd]

If in the code you pasted you change the comparison to this, it works

key: 'componentWillReceiveProps',
        value: function componentWillReceiveProps(nextProps) {
            if (nextProps.selectedItem !== this.props.selectedItem) {
                this.updateSizes();
                this.moveTo(nextProps.selectedItem);
            }

note I changed from state to props on the second part of the if statement

[Cyan005]

So yeah, that does work because that if statement is always 0 !== 0 so it never goes into it. That may be a band aid for this small case, but who knows what it breaks beyond that. Ideally, the goal would be that nextProps.selectedItem comes in correct to the "NEXT" slide you want to navigate to.

On the 2nd click, nextProps.selectedItem is 2. this.state.selectedItem = 1. Then it just makes it work as expected. That's not the case...

[Cyan005]

I see part of what may be going on.

When it enters the IF statement, calling this.moveTo(nextProps.selectedItem) triggers the onChange handler again to be hit twice hence why we are seeing two console log items.

key: 'componentWillReceiveProps',
        value: function componentWillReceiveProps(nextProps) {
            if (nextProps.selectedItem !== this.state.selectedItem) {
                this.updateSizes();
                this.moveTo(nextProps.selectedItem);
            }

_this.moveTo = function (position) {
            var lastPosition = _this.props.children.length - 1;

            if (position < 0) {
                position = _this.props.infiniteLoop ? lastPosition : 0;
            }

            if (position > lastPosition) {
                position = _this.props.infiniteLoop ? 0 : lastPosition;
            }

            _this.selectItem({
                // if it's not a slider, we don't need to set position here
                selectedItem: position
            });

            // don't reset auto play when stop on hover is enabled, doing so will trigger a call to auto play more than once
            // and will result in the interval function not being cleared correctly.
            if (_this.props.autoPlay && _this.state.isMouseEntered === false) {
                _this.resetAutoPlay();
            }
        };



md5-71426001212430ce9a701a87a42ac386



_this.selectItem = function (state) {
            _this.setState(state);
            _this.handleOnChange(state.selectedItem, _this.props.children[state.selectedItem]);
        };

[Cyan005]

Did a very quick hard coded test for the single use case of the 2nd click. If you bypass the second handleOnChange, the next slide works no issue. Just need to figure out how to properly bypass it whether it be with another state property or something else that already exists out there.

        _this.selectItem = function (state) {
            _this.setState(state);

            if (this.state.selectedItem != 1) {
                _this.handleOnChange(state.selectedItem, _this.props.children[state.selectedItem]);
            }
        };

[leandrowd]

I've published an updated example with external controls. Please have a look at the demo and check the source code.

Please check if it solves your problems.

[jasonogasian]

@leandrowd the case you posted above works because the user is not swiping. If the onChange handler fires due to a swipe, and you update the state (state.currentSlide in your example) the transition restarts due to the state change.

Hope this makes sense. I'm really stuck on this and not sure how to proceed.

[qruzz]

I was having the same issue, but there is actually a fairly simple workaround ..
According to the docs, the Carousel component has a selectedItem prop, that takes an Number for the index of the item selected in the image sequence.

That means, that if you initialise your components state with the the item to select, and in the onChange prop set the state to the next selected item, it will still perform the transition.

See code example for clarification:

constructor(props) {
    super(props);

    this.state = {
        selectedItem: 0,
    };
}

renderHeroContainer = () => {
    return (
        <div className="carousel-container">
            <Carousel
                autoPlay
                className="carousel"
                infiniteLoop={true}
                selectedItem={this.state.selectedItem}
                transitionTime={400}
                onChange={(e) => {
                    this.setState({
                        selectedItem: e,
                    });
                }}>
                {this.renderItems()}
            </Carousel>
        </div>
    );
}

[jordanBugaboo]

Thank you for this solution qruzz. It does work doing it this way but it does seem to force a double render which is not ideal. As stated above it's only an issue when you update state but I think using the selectedItem index is a popular use case for this slider. @leandrowd is this something that could potentially be fixed in a future release? Many Thanks

[kitfit-dave]

My opinion on this is that there is a problem with there being no "source of truth" for the selectedItem value. In componentWillReceiveProps the value that comes in props is used to moveTo that index. However, swiping and tapping the thumbs etc, calls setState to change that value. Now you have a set up that means the next time the component renders and componentWillReceiveProps gets called (e.g. when a parent component calls setState itself) the nextProps value does not match the state value and the nextProps value (probably 0 because the prop was never explicitly set) is used to moveTo that index again.

Thus the issue that several people seem to have found where they have a setState in their own onChange handler. Then they can click to the second image, but the next click will set them back to index 0 again.

A choice needs to be made whether selectedItem is controlled or un-controlled. I.e. does the prop set the initial value, and then the state value is the source or truth. OR does the incoming props value take precedence - and then you kind of need to force callers to manage/set the value. Indeed, doing just that, managing the selectedItem state in the parent and sending the value in via props each render works around this issue.

Personally, I would change the name of the prop to initialSelectedItem and leave the internal state to manage the actual selectedItem. I would think that would be most of the use cases. If you need to set the selected item from the parent you can always set the key and get a re-mount... For interesting examples of controlled and uncontrolled props I'd suggest looking at how downshift manages props that can work in either mode - but not both at once or course.

[sezna]

I agree with the above comment and there are a couple of ways this could get fixed.

• Rename selectedItem to initialSelectedItem and don't control it
• Use controlled-component style behavior if selectedItem is passed at all and stop controlling the state of what is selected internally

[sajanbhatia]

please add this code in onchange function
const timer = setTimeout(() => {
this.setState({initialSelectedItem:index});
}, 100);
it is working

[maxizhukov]

Have you solution for that, I have same problem?

[renancleyson-dev]

I will work on it.