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

Remove iOS 11 deprecation warnings around SafeArea #32851

Closed

Conversation

ken0nek
Copy link
Contributor

@ken0nek ken0nek commented Jan 8, 2022

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.


  • Stop using layout guide (topLayoutGuide, bottomLayoutGuide)
  • Refactor RCTSafeAreaView
  • Delete emulateUnlessSupported property

Docs PR: facebook/react-native-website#2919

Changelog

[iOS] [Removed] - Remove emulateUnlessSupported

Test Plan

@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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 8, 2022
NSStringFromUIEdgeInsets(_currentSafeAreaInsets)];
}

- (BOOL)isSupportedByOS
{
return [self respondsToSelector:@selector(safeAreaInsets)];
Copy link
Contributor Author

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;
Copy link
Contributor Author

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
Copy link
Contributor Author

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) {
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 never called because isSupportedByOS is always true/YES

@analysis-bot
Copy link

analysis-bot commented Jan 8, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 46ab59c
Branch: main

@analysis-bot
Copy link

analysis-bot commented Jan 8, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,773,459 -426
android hermes armeabi-v7a 7,179,631 -192
android hermes x86 8,082,788 -281
android hermes x86_64 8,061,400 -276
android jsc arm64-v8a 9,646,615 -277
android jsc armeabi-v7a 8,421,182 -31
android jsc x86 9,595,997 -122
android jsc x86_64 10,193,392 -123

Base commit: 46ab59c
Branch: main

@facebook-github-bot
Copy link
Contributor

@sota000 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@sota000 sota000 left a 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.

Comment on lines 33 to 38
exported = React.forwardRef<
ViewProps,
React.ElementRef<HostComponent<mixed>>,
>(function SafeAreaView(props, forwardedRef) {
return <View {...props} ref={forwardedRef} />;
});
Copy link
Contributor

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?

Suggested change
exported = React.forwardRef<
ViewProps,
React.ElementRef<HostComponent<mixed>>,
>(function SafeAreaView(props, forwardedRef) {
return <View {...props} ref={forwardedRef} />;
});
exported = View;
});

Comment on lines 43 to 48
exported = React.forwardRef<
ViewProps,
React.ElementRef<HostComponent<mixed>>,
>(function SafeAreaView(props, forwardedRef) {
return <RCTSafeAreaViewNativeComponent {...props} ref={forwardedRef} />;
});
Copy link
Contributor

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?

Suggested change
exported = React.forwardRef<
ViewProps,
React.ElementRef<HostComponent<mixed>>,
>(function SafeAreaView(props, forwardedRef) {
return <RCTSafeAreaViewNativeComponent {...props} ref={forwardedRef} />;
});
exported = require('./RCTSafeAreaViewNativeComponent').default;
});

Copy link
Contributor Author

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!

@ken0nek ken0nek requested a review from sota000 January 20, 2022 22:14
@ken0nek ken0nek force-pushed the work/ios-11-availability-safe-area branch 3 times, most recently from 24f43ae to 82a4906 Compare January 20, 2022 23:23
@facebook-github-bot
Copy link
Contributor

@sota000 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@ken0nek ken0nek force-pushed the work/ios-11-availability-safe-area branch from 82a4906 to 786d4a4 Compare April 12, 2022 22:38
@ken0nek
Copy link
Contributor Author

ken0nek commented Apr 27, 2022

@sota000 Rebased, and all checks have passed 👍

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @ken0nek in c73e021.

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 May 16, 2022
@ken0nek ken0nek deleted the work/ios-11-availability-safe-area branch May 18, 2022 00:01
facebook-github-bot pushed a commit that referenced this pull request Jun 27, 2023
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
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. Merged This PR has been merged. Platform: iOS iOS applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Type: Removal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants