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.

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

RNTester: Migrate TransformExample to hooks #27181

Closed

Conversation

empyrical
Copy link
Contributor

Summary

This pull request migrates the animated flip example in TransformExample to using React Hooks.

Changelog

[General] [Changed] - RNTester: Migrate TransformExample to hooks

Test Plan

The animation works exactly as it did as when it was a class component

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. labels Nov 10, 2019
@empyrical
Copy link
Contributor Author

The original version of this component had no logic to stop the animated loop callback from being fired when the <Flip /> component is unmounted. Should logic for cleaning this up be added in the new version of <Flip />?

Copy link
Contributor

@mdvacca mdvacca left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@mdvacca is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@mdvacca
Copy link
Contributor

mdvacca commented Nov 10, 2019

The land operation failed because of flow errors:

Error --------------------------------------------------------------------------- react-native-github/RNTester/js/examples/Transform/TransformExample.js:193:15 Cannot getReact.Elementbecause propertyElement` is missing in object type [1].

react-native-github/RNTester/js/examples/Transform/TransformExample.js:193:15
193| render(): React.Element {
^^^^^^^^^^^^^

References:
v-
398| declare export default {|
399| +DOM: typeof DOM,
400| +PropTypes: typeof PropTypes,
:
429| +useImperativeHandle: typeof useImperativeHandle,
430| |};
---^ [1]

Error ---------------------------------------------------------------------------
react-native-github/RNTester/js/examples/Transform/TransformExample.js:201:15
Cannot get React.Node because property Node is missing in object type [1].

react-native-github/RNTester/js/examples/Transform/TransformExample.js:201:15
201| render(): React.Node {
^^^^^^^^^^

References:
v-
398| declare export default {|
399| +DOM: typeof DOM,
400| +PropTypes: typeof PropTypes,
:
429| +useImperativeHandle: typeof useImperativeHandle,
430| |};
---^ [1]
.....
`

@empyrical
Copy link
Contributor Author

Pushed the fix for the Flow errors. Missed those after I swapped the import style!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@mdvacca is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @empyrical in 58726ce.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. Merged This PR has been merged. RNTester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants