Skip to content

Remove need for platform overrides in SafeAreaView.js #38601

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

Closed

Conversation

christophpurrer
Copy link
Contributor

Summary:
This file is forked in out of tree platforms as:

The change here removes the need of forking this file

Changelog:
[General] [Fixed] - [SafeAreaView] Remove need for platform overrides in SafeAreaView.js

Differential Revision: D47734944

@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. p: Facebook Partner: Facebook Partner fb-exported labels Jul 24, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47734944

@christophpurrer
Copy link
Contributor Author

@Saadnajmi / @acoates-ms > looks good?

@Saadnajmi
Copy link
Contributor

Saadnajmi commented Jul 24, 2023

@Saadnajmi / @acoates-ms > looks good?

Looks good to me. A few notes:

  1. I've actually had a task for this on the macOS repo for a while :D Upstream change: Refactor Platform check in SafeAreaView microsoft/react-native-macos#1654
  2. macOS should probably support SafeAreaView one day (since macOS has a similar native concept). I looked into it and it's doable, just not high priority for me. When that's done, I can just fork the file again to add macOS or change the check to isApple or something
  3. Andrew Coates is OOF, so maybe @chiaramooney ?

LGTM from me =)

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47734944

christophpurrer added a commit to christophpurrer/react-native-macos that referenced this pull request Jul 25, 2023
Summary:
Pull Request resolved: facebook#38601

This file is forked in out of tree platforms as:

- macOS > https://github.com/microsoft/react-native-macos/blob/main/Libraries/Components/SafeAreaView/SafeAreaView.js#L28-L33
- Windows > https://github.com/microsoft/react-native-windows/blob/0.71-stable/vnext/src/Libraries/Components/SafeAreaView/SafeAreaView.windows.js#L30-L36

The change here removes the need of forking this file

Changelog:
[General] [Fixed] - [SafeAreaView] Remove need for platform overrides in SafeAreaView.js

Differential Revision: D47734944

fbshipit-source-id: 5726f357182c9f54807bb5a63bbf535e4dcda3fd
@christophpurrer
Copy link
Contributor Author

@Saadnajmi > Ideally with this (and other changes to come) we would aim to limit the number of forked files 'out of tree platforms' have to maintain.

However for the macOS use case you mentioned you might still need one for the foreseeable future

christophpurrer added a commit to christophpurrer/react-native-macos that referenced this pull request Jul 25, 2023
Summary:
Pull Request resolved: facebook#38601

This file is forked in out of tree platforms as:

- macOS > https://github.com/microsoft/react-native-macos/blob/main/Libraries/Components/SafeAreaView/SafeAreaView.js#L28-L33
- Windows > https://github.com/microsoft/react-native-windows/blob/0.71-stable/vnext/src/Libraries/Components/SafeAreaView/SafeAreaView.windows.js#L30-L36

The change here removes the need of forking this file

Changelog:
[General] [Fixed] - [SafeAreaView] Remove need for platform overrides in SafeAreaView.js

Differential Revision: D47734944

fbshipit-source-id: cb9627b618582b4389d7b2870747e712e8d19e86
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47734944

@analysis-bot
Copy link

analysis-bot commented Jul 25, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,843,398 -45
android hermes armeabi-v7a 8,152,551 -47
android hermes x86 9,349,181 -54
android hermes x86_64 9,191,916 -40
android jsc arm64-v8a 9,456,874 +11
android jsc armeabi-v7a 8,637,996 -1
android jsc x86 9,539,955 +5
android jsc x86_64 9,783,245 -3

Base commit: e64756a
Branch: main

Summary:
Pull Request resolved: facebook#38601

This file is forked in out of tree platforms as:

- macOS > https://github.com/microsoft/react-native-macos/blob/main/Libraries/Components/SafeAreaView/SafeAreaView.js#L28-L33
- Windows > https://github.com/microsoft/react-native-windows/blob/0.71-stable/vnext/src/Libraries/Components/SafeAreaView/SafeAreaView.windows.js#L30-L36

The change here removes the need of forking this file

Changelog:
[General] [Fixed] - [SafeAreaView] Remove need for platform overrides in SafeAreaView.js

Reviewed By: NickGerleman

Differential Revision: D47734944

fbshipit-source-id: 27ba3399339c90c9b324c77b178407488d7fe18e
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D47734944

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 80f531f.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jul 26, 2023
billnbell pushed a commit to billnbell/react-native that referenced this pull request Jul 29, 2023
Summary:
Pull Request resolved: facebook#38601

This file is forked in out of tree platforms as:

- macOS > https://github.com/microsoft/react-native-macos/blob/main/Libraries/Components/SafeAreaView/SafeAreaView.js#L28-L33
- Windows > https://github.com/microsoft/react-native-windows/blob/0.71-stable/vnext/src/Libraries/Components/SafeAreaView/SafeAreaView.windows.js#L30-L36

The change here removes the need of forking this file

Changelog:
[General] [Fixed] - [SafeAreaView] Remove need for platform overrides in SafeAreaView.js

Reviewed By: NickGerleman

Differential Revision: D47734944

fbshipit-source-id: 84249a3b3e7e3807b3d5ee4bfa4b1cb140541b8b
billnbell pushed a commit to billnbell/react-native that referenced this pull request Jul 29, 2023
Summary:
Pull Request resolved: facebook#38601

This file is forked in out of tree platforms as:

- macOS > https://github.com/microsoft/react-native-macos/blob/main/Libraries/Components/SafeAreaView/SafeAreaView.js#L28-L33
- Windows > https://github.com/microsoft/react-native-windows/blob/0.71-stable/vnext/src/Libraries/Components/SafeAreaView/SafeAreaView.windows.js#L30-L36

The change here removes the need of forking this file

Changelog:
[General] [Fixed] - [SafeAreaView] Remove need for platform overrides in SafeAreaView.js

Reviewed By: NickGerleman

Differential Revision: D47734944

fbshipit-source-id: 84249a3b3e7e3807b3d5ee4bfa4b1cb140541b8b
billnbell pushed a commit to billnbell/react-native that referenced this pull request Jul 29, 2023
Summary:
Pull Request resolved: facebook#38601

This file is forked in out of tree platforms as:

- macOS > https://github.com/microsoft/react-native-macos/blob/main/Libraries/Components/SafeAreaView/SafeAreaView.js#L28-L33
- Windows > https://github.com/microsoft/react-native-windows/blob/0.71-stable/vnext/src/Libraries/Components/SafeAreaView/SafeAreaView.windows.js#L30-L36

The change here removes the need of forking this file

Changelog:
[General] [Fixed] - [SafeAreaView] Remove need for platform overrides in SafeAreaView.js

Reviewed By: NickGerleman

Differential Revision: D47734944

fbshipit-source-id: 84249a3b3e7e3807b3d5ee4bfa4b1cb140541b8b
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. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants