-
Notifications
You must be signed in to change notification settings - Fork 50
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
Adding functionality to set frame index directly #126
base: master
Are you sure you want to change the base?
Conversation
liuloppan
commented
Dec 1, 2019
- Adding functionality to set the intital "current frame index" by passing it as a prop into Carousel.
- Adding the function setFrame to Carousel and passing it as a Widget prop ('setFrameHandler') which can be used by Indicator dots to transition multiple frames at once.
* Added functionality to pass currentFrameIndex as props in the cases where you may have a certain order but not necessarily want to start at index 0 * Added functionality to set a frame index directly with setFrame(index) which can be passed as the prop 'setFrameHandler' to Widgets in a similar fashion as 'nextHandler' and 'prevHandler'
setFrame now adds timeouts for 'next' and 'prev' which creates a smoother transition between frames.
Thanks @liuloppan! will look into it soon :D |
const diff = Math.abs(index - this.state.currentFrameIndex); | ||
if (index < this.state.currentFrameIndex) { | ||
for (let i = 0; i < diff; i++) { | ||
setTimeout(() => this.prev(), i * ms); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There would be a series of animation for switching to target frame, which works but not ideal, especially when user jumps between a lot frames. We need to switch to target frame directly. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I preferred to have the animation and to be able to control the animation time. For a small number of frames setting 'ms' to 0 will visually have the same effect, but for many frames this could be an issue as you say. But I will look into creating a function that allows changing it directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amio I have updated the PR. Also updated index.js so that the example is using this functionality. Currently currentFrameIndex i passed as prop and is set to 3 to show how it works, but it could be removed.
Now you can chose to have the animation if you put in how many milliseconds (ms) it should animate between. Otherwise (default) is that the state.currentFrameIndex is set as well as state.movingFrames (which has a special override case). Which means we don't need to render any frames in between.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@liuloppan Great! Just a few changes:
- since
loop
cannot work withcurrentFrameIndex
, we should log a warning in case of that, - let's keep the default demo as original, add some description of
currentFrameIndex
in README.md
|