Added autoplay and interval props#37
Conversation
| componentDidMount() { | ||
| const { autoplay, interval } = this.props; | ||
| if (autoplay) { | ||
| this.state.intervalId = setInterval(() => { | ||
| this.moveSlide(1); | ||
| }, interval); | ||
| } | ||
| } |
There was a problem hiding this comment.
In my opinion it might be better to make autplay/interval prop changes affect the interval even after the component was mounted.
| if(this.state.intervalId) { | ||
| clearInterval(this.state.intervalId); | ||
| } |
There was a problem hiding this comment.
| if(this.state.intervalId) { | |
| clearInterval(this.state.intervalId); | |
| } | |
| if (this.state.intervalId) { | |
| clearInterval(this.state.intervalId); | |
| } |
| autoplay: true, | ||
| interval: 2, |
There was a problem hiding this comment.
Maybe we can just use interval here and default it to null or 0, this would simplify the API and prevent users from accidentally setting inconsistent prop values.
| { | ||
| "name": "react-spring-3d-carousel", | ||
| "version": "1.2.1", | ||
| "version": "1.3.1", |
There was a problem hiding this comment.
| "version": "1.3.1", | |
| "version": "1.3.0", |
|
Looks pretty good overall, left some minor comments and would be happy to merge once they're addressed. Great work! 🎉 |
|
Don't really have much time to spend on this, I had done this PR when I needed the lib for a specific project. Will try to see if I can do the suggested changes, else, if anyone want to pick it up, feel free to :) |
2 participants
Heavily based on @Satyam-code143 version, also added a way to clear the timeout to avoid leaks.