fix: fix missing translateY prop #18
Conversation
Hi. Keep doing the good work. There is a minor glitch I have encountered on several devices when trying your lib. TranslateY prop is often undefined. You can easily reproduce it even on yor demo by pressing the FIRST list item -> transition is often bumpy as the cloned element misses translateY value. It holds true even when coming back from the detail page into the list view. By introducing this minor change, the transition animation is butter-smooth with no more unpleasant jumpy behavior. Thx for your consideration.
src/SharedElementRenderer.js
Outdated
| @@ -56,7 +56,7 @@ class SharedElementRenderer extends PureComponent { | |||
|
|
|||
| const animations = []; | |||
|
|
|||
| if (source.position.pageY !== destination.position.pageY) { | |||
| if (source.position.pageY && destination.position.pageY) { | |||
There was a problem hiding this comment.
I think the intention of the comparison is to also avoid an animation and render if the pageY values are identical. And also, it should be able to handle the 0 case, which is falsy and does not pass your new test. So perhaps change it to this which takes care of your issue as well as satisfies the original intent:
if (source.position.pageY !== undefined
&& destination.position.pageY !== undefined
&& source.position.pageY !== destination.position.pageY)
{
There was a problem hiding this comment.
Or even more economically (without such a long IF condition) as in the updated merge request.
There was a problem hiding this comment.
I do like what you're going for. However, won't setting the state with a new Animated.Value instance invoke a render, when the point of the check is to avoid it? Perhaps move the setState call under the source !== dest check and you'll be in business.
There was a problem hiding this comment.
You were right, the re-render was needless. The pull request was modified accordingly.
|
Any chance of merging? I would like to stop using a fork with the fixed Y prop. |
|
Cheers mate, and sorry it took sooooo long ... |
|
🎉 This PR is included in version 1.0.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Hi. Keep doing the good work. There is, however, a minor glitch I have encountered
on several devices when trying your lib. TranslateY prop is often
undefined. You can easily reproduce it even on your demo by pressing the
very first list item -> transition is often bumpy as the cloned element misses
translateY value. It holds true even when coming back from the detail
page into the list view. By introducing this minor change, the transition
animation is butter-smooth with no more unpleasant jumpy behavior as a result of a missing/undefined value. Thx for your consideration.