Improve performance in shine animation #56
Conversation
- Activate useNativeDriver option - Fix to start animation in callback only when finished
|
Hello and thanks for the PR! 😄 I'm sorry, I'm from Europe and can't answer you as soon as I would like 😅 . Can you fix the problem concerning the test coverage? Again, thank you! 👌 |
| @@ -27,9 +28,12 @@ const Shine = ({ children }) => { | |||
| Animated.timing(animation, { | |||
| toValue: END_VALUE, | |||
| duration: DURATION, | |||
| useNativeDriver, | |||
There was a problem hiding this comment.
I'm not sure useNativeDriver is useful right there since it's not applicable to marginLeftanimation (https://facebook.github.io/react-native/docs/animations#caveats) 😊
| ]).start(() => { | ||
| start(); | ||
| ]).start((e) => { | ||
| if (e.finished) { |
There was a problem hiding this comment.
Well, I don't know if it's a way to solve the problem:
When calling start, we start the animation. I think the callback is immediately called. What is it's not finished yet? Does it mean that we would run the animation only once?
Thanks for your help 👍
There was a problem hiding this comment.
I also see this issue and testing (e.finished) does fix the memory leak we are seeing.
The callback to start is called when the animation is complete and not immediately, so we must test if it finished correctly or was terminated for some reason, otherwise any cancelled or stopped animation will just be started again.
There was a problem hiding this comment.
I'm not sure the OP will continue this pr 😅
You'd like to take the point on a separate PR? Or would you like me to dot it? 😊
There was a problem hiding this comment.
Would be great if you could do it - I was in the midst of forking and fixing to be able to pull in our package.json :)
There was a problem hiding this comment.
But useNativeDriver is not needed and crashes for me as per you comment marginLeft is not supported
|
Close and fixed in the v1.3.0 release, changelog here: https://github.com/mfrachet/rn-placeholder/blob/master/CHANGELOG.md |
Thank you for the amazing library!
While using
shineanimation we noticed it uses so much CPU power and leaks memory, and after changing to usefadeanimation that kind of thing never happens.After comparing two animation logic we realized differences below:
So this PR addresses above two points. What do you think?
ref: #33