-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Remove iOS 11 deprecation warnings around SafeArea #32851
Conversation
NSStringFromUIEdgeInsets(_currentSafeAreaInsets)]; | ||
} | ||
|
||
- (BOOL)isSupportedByOS | ||
{ | ||
return [self respondsToSelector:@selector(safeAreaInsets)]; |
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.
Always true/YES
- (UIEdgeInsets)safeAreaInsetsIfSupportedAndEnabled | ||
{ | ||
if (self.isSupportedByOS) { | ||
return self.safeAreaInsets; |
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.
We can delete this property because safeAreaInsetsIfSupportedAndEnabled
always returns self.safeAreaInsets
.
return self.emulateUnlessSupported ? self.emulatedSafeAreaInsets : UIEdgeInsetsZero; | ||
} | ||
|
||
- (UIEdgeInsets)emulatedSafeAreaInsets |
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.
We don't this fallback any longer.
{ | ||
[super layoutSubviews]; | ||
|
||
if (!self.isSupportedByOS && self.emulateUnlessSupported) { |
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 never called because isSupportedByOS
is always true/YES
Base commit: 46ab59c |
Base commit: 46ab59c |
@sota000 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
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.
Thanks for working on this! I have a couple of suggestions I'd like you to see if it makes sense.
exported = React.forwardRef< | ||
ViewProps, | ||
React.ElementRef<HostComponent<mixed>>, | ||
>(function SafeAreaView(props, forwardedRef) { | ||
return <View {...props} ref={forwardedRef} />; | ||
}); |
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.
Can this be simplified to this?
exported = React.forwardRef< | |
ViewProps, | |
React.ElementRef<HostComponent<mixed>>, | |
>(function SafeAreaView(props, forwardedRef) { | |
return <View {...props} ref={forwardedRef} />; | |
}); | |
exported = View; | |
}); |
exported = React.forwardRef< | ||
ViewProps, | ||
React.ElementRef<HostComponent<mixed>>, | ||
>(function SafeAreaView(props, forwardedRef) { | ||
return <RCTSafeAreaViewNativeComponent {...props} ref={forwardedRef} />; | ||
}); |
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.
Can this be simplified to this?
exported = React.forwardRef< | |
ViewProps, | |
React.ElementRef<HostComponent<mixed>>, | |
>(function SafeAreaView(props, forwardedRef) { | |
return <RCTSafeAreaViewNativeComponent {...props} ref={forwardedRef} />; | |
}); | |
exported = require('./RCTSafeAreaViewNativeComponent').default; | |
}); |
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.
Thank you for the feedback!
24f43ae
to
82a4906
Compare
@sota000 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
82a4906
to
786d4a4
Compare
@sota000 Rebased, and all checks have passed 👍 |
This pull request was successfully merged by @ken0nek in c73e021. When will my fix make it into a release? | Upcoming Releases |
Summary: I removed the code checking iOS 12 availability because the iOS minimum deployment target is now iOS 12.4 after these commits (982ca30, c71e6ef). My previous pull requests regarding iOS 11 * [Remove iOS 11 version check by ken0nek · Pull Request #32151 · facebook/react-native](#32151) * [Remove iOS 11 availability check by ken0nek · Pull Request #32488 · facebook/react-native](#32488) * [Remove iOS 11 deprecation warnings around SafeArea by ken0nek · Pull Request #32851 · facebook/react-native](#32851) ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [iOS] [Changed] - Remove iOS 12 availability check Pull Request resolved: #33460 Reviewed By: NickGerleman Differential Revision: D35021632 Pulled By: javache fbshipit-source-id: bf85d44874a2c10cb345d33df7c9e4789312a7cd
Summary
We don't have to check or emulate the safe area for iOS 11 above. I deleted the unnecessary check for the safe area.
This is a continuation pull request of these iOS 11 availability check.
topLayoutGuide
,bottomLayoutGuide
)RCTSafeAreaView
emulateUnlessSupported
propertyDocs PR: facebook/react-native-website#2919
Changelog
[iOS] [Removed] - Remove
emulateUnlessSupported
Test Plan