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

[NativeMethodsMixin] ProgressViewIOS: Remove PropTypes and NativeMethodsMixin, convert to functional component #21588

Closed
wants to merge 2 commits into from

Conversation

empyrical
Copy link
Contributor

This PR converts ProgressViewIOS from a createReactClass component to a functional component, and removes the remaining proptypes. Its use of NativeMethodsMixin has been ported to a forwardRef to the native component.

Test Plan:

I have used it in RNTester, and the example progress bars work fine.

Release Notes:

[GENERAL] [ENHANCEMENT] [Libraries/Components/ProgressViewIOS/ProgressViewIOS.ios.js] - Remove PropTypes and NativeEventsMixin, convert to functional component

@empyrical empyrical requested a review from elicwhite October 9, 2018 05:31
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 9, 2018
});
const ProgressViewIOS = (
props: Props,
forwardedRef?: ?React.Ref<'RCTProgressView'>,
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if this is right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the way the other components (like Text) seem to do it - but I notice that View does it a better way, so I updated it to do it the way View does it. The other ones should be cleaned up too, perhaps!

@react-native-bot react-native-bot added Platform: iOS iOS applications. 🔶Components Type: Enhancement A new feature or enhancement of an existing feature. labels Oct 9, 2018
@RSNara RSNara changed the title ProgressViewIOS: Remove PropTypes and NativeEventsMixin, convert to functional component [NativeEventsMixin] ProgressViewIOS: Remove PropTypes and NativeEventsMixin, convert to functional component Oct 9, 2018
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.

RSNara has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@RSNara RSNara left a comment

Choose a reason for hiding this comment

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

Looks good! 👌🏽

@RSNara RSNara changed the title [NativeEventsMixin] ProgressViewIOS: Remove PropTypes and NativeEventsMixin, convert to functional component [NativeMethodsMixin] ProgressViewIOS: Remove PropTypes and NativeMethodsMixin, convert to functional component Oct 11, 2018
@react-native-bot
Copy link
Collaborator

@empyrical merged commit 93e6ae1 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Oct 12, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 12, 2018
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
…functional component (facebook#21588)

Summary:
This PR converts `ProgressViewIOS` from a `createReactClass` component to a functional component, and removes the remaining proptypes. Its use of `NativeMethodsMixin` has been ported to a `forwardRef` to the native component.
Pull Request resolved: facebook#21588

Reviewed By: hramos

Differential Revision: D10338888

Pulled By: RSNara

fbshipit-source-id: c49807e97a0e2cf774971d9aa5a8426f15a3e48d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants