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

fix: remove dummy nativeOnly #3056

Merged
merged 1 commit into from
Jul 8, 2023
Merged

Conversation

Sunbreak
Copy link
Contributor

@Sunbreak Sunbreak commented Mar 9, 2023

We are working on fabric support for video. This is the first one

Describe the changes

According to facebook/react-native#28351 (comment), requireNativeComponent recieves only one argument since react-native@0.56

According to https://github.com/facebook/react-native/tree/v0.69.4/Libraries/Components/Switch, native only properties are declared in <ComponentName>NativeComponent.js, e.g. on, enabled and trackTintColor

@Sunbreak Sunbreak mentioned this pull request Mar 9, 2023
@Sunbreak Sunbreak force-pushed the fabric-1 branch 2 times, most recently from c1ee7e6 to 86d4c87 Compare March 15, 2023 01:06
@stanwolverine
Copy link

Hey @Sunbreak , Where are you on this? I would like to help you.

@Sunbreak
Copy link
Contributor Author

Sunbreak commented Apr 3, 2023

It is WIP. I'll let you known if it is ready

@@ -571,10 +563,4 @@ Video.propTypes = {
...ViewPropTypes,
};

const RCTVideo = requireNativeComponent('RCTVideo', Video, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I had a quick review of requireNativeComponent api, It looks like 2 last props are not used...
Did you check on which react-native version it has been removed ?
I think this can be merged rapidly. I just merge your previous small Prs
Thanks you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to facebook/react-native#28351 (comment), requireNativeComponent recieves only one argument since react-native@0.56

@freeboub freeboub merged commit c0517f8 into TheWidlarzGroup:master Jul 8, 2023
@Sunbreak Sunbreak deleted the fabric-1 branch July 11, 2023 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants