Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Create interaction handle during animation #314

Merged
merged 1 commit into from
Apr 12, 2019
Merged

Create interaction handle during animation #314

merged 1 commit into from
Apr 12, 2019

Conversation

Jyrno42
Copy link
Contributor

@Jyrno42 Jyrno42 commented Apr 1, 2019

This makes the path animation compatible with InteractionManager.runAfterInteractions

see https://facebook.github.io/react-native/docs/interactionmanager

@Jyrno42
Create interaction handle during animation
1122de9 
This makes the path animation compatible with InteractionManager.runAfterInteractions
@Jyrno42
Copy link
Contributor Author

Jyrno42 commented Apr 1, 2019

Note: The tests failure seems to be unrelated to my changes.

JesperLekland
JesperLekland reviewed Apr 2, 2019
View reviewed changes
Copy link
Owner

@JesperLekland JesperLekland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice 👍! Is it properly tested? And did it make any performance difference?

@Jyrno42
Copy link
Contributor Author

Jyrno42 commented Apr 2, 2019

I tested it manually and the regular functionality still worked. It generally won't make any difference in performance, unless the consumers use InteractionManager.runAfterInteractions.

In the app I am building I have a display type toggle. Tapping on it changes the rendered chart values and also notifies other component that the display type changed. The animation for the chart is smoother when I move the notification code into InteractionManager.runAfterInteractions.

@Jyrno42 Jyrno42 closed this Apr 10, 2019
@Jyrno42
Copy link
Contributor Author

Jyrno42 commented Apr 10, 2019

Accidentally pressed close

@Jyrno42 Jyrno42 reopened this Apr 10, 2019
@Jyrno42
Copy link
Contributor Author

Jyrno42 commented Apr 11, 2019

@JesperLekland Let me know if this needs something more to get it merged.

@JesperLekland
Copy link
Owner

Great job, I'll merge and try to get it released this weekend 👍

@JesperLekland JesperLekland merged commit 56b781f into JesperLekland:dev Apr 12, 2019
@Jyrno42
Copy link
Contributor Author

Jyrno42 commented May 14, 2019

Hey @JesperLekland, not to rush you or anything, but could you please do the release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JesperLekland JesperLekland JesperLekland left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Jyrno42 @JesperLekland