-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
replaced UIManager.RCTVideo > UIManager.getViewManagerConfig('RCTVideo') #1487
Conversation
Video.js
Outdated
@@ -225,13 +225,13 @@ export default class Video extends Component { | |||
|
|||
let nativeResizeMode; | |||
if (resizeMode === VideoResizeMode.stretch) { | |||
nativeResizeMode = NativeModules.UIManager.RCTVideo.Constants.ScaleToFill; | |||
nativeResizeMode = NativeModules.UIManager.getViewManagerConfig('RCTVideo').Constants.ScaleToFill; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break old react native should use wrapper function. Example implementation react-native-webview/react-native-webview@263fc5e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just made changes you suggested, thanks for the reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use this helper function to ensure backwards compat.
i already did. didn't I? line 213, 234 |
Oh for some reason I was one commit behind sorry! |
Could you please also update the changelog? |
done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nitnk9 Thank you!
@nitnk9 @n1ru4l This is blowing up on master for me due to UIManager not being defined. I haven't stayed up on this PR so I'm hoping one of you two understands what we need to do to import that module. I'm testing on RN 0.57. Can you put up a PR to resolve this asap please, there's a bunch of new features that would be great to ship once we have everything sorted out. |
It should be |
Also this reminds me that we have to setup CI that does basic stuff like running eslint which could have prevented this 😅 |
Yes we definitely need that. |
…IManager.getViewManagerConfig('RCTVideo')` (and ensuring backwards compat) (TheWidlarzGroup#1487) * replaced UIManager.RCTVideo > UIManager.getViewManagerConfig('RCTVideo') * added requested changes * updated changelog.md * docs: adjust wording
…IManager.getViewManagerConfig('RCTVideo')` (and ensuring backwards compat) (TheWidlarzGroup#1487) * replaced UIManager.RCTVideo > UIManager.getViewManagerConfig('RCTVideo') * added requested changes * updated changelog.md * docs: adjust wording (rebased from commit b448b30)
Describe the changes
just updated UIManager.RCTVideo > UIManager.getViewManagerConfig('RCTVideo') to remove warnings from the debug app.
Closes #1466