Moving towards UIWindowScene support (#25425) #28058
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D16957751 |
|
|
RNTester (Android/hermes/arm64-v8a): 3276800 bytes |
4 tasks
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D16957751 |
hramos
pushed a commit
to hramos/react-native
that referenced
this pull request
Feb 14, 2020
Summary: Pull Request resolved: facebook#28058 I'm taking the first step towards supporting iOS 13 UIScene APIs and modernizing React Native not to assume an app only has a single window. See discussion here: facebook#25181 (comment) The approach I'm taking is to take advantage of `RootTagContext` and passing it to NativeModules so that they can identify correctly which window they refer to. Here I'm just laying groundwork. - [x] `Alert` and `ActionSheetIOS` take an optional `rootTag` argument that will cause them to appear on the correct window - [x] `StatusBar` methods also have `rootTag` argument added, but it's not fully hooked up on the native side — this turns out to require some more work, see: facebook#25181 (comment) - [x] `setNetworkActivityIndicatorVisible` is deprecated in iOS 13 - [x] `RCTPerfMonitor`, `RCTProfile` no longer assume `UIApplicationDelegate` has a `window` property (no longer the best practice) — they now just render on the key window Next steps: Add VC-based status bar management (if I get the OK on facebook#25181 (comment) ), add multiple window demo to RNTester, deprecate Dimensions in favor of a layout context, consider adding hook-based APIs for native modules such as Alert that automatically know which rootTag to pass ## Changelog [Internal] [Changed] - Modernize Modal to use RootTagContext [iOS] [Changed] - `Alert`, `ActionSheetIOS`, `StatusBar` methods now take an optional `surface` argument (for future iPadOS 13 support) [Internal] [Changed] - Do not assume `UIApplicationDelegate` has a `window` property Pull Request resolved: facebook#25425 Test Plan: - Open RNTester and: - go to Modal and check if it still works - Alert → see if works - ACtionSheetIOS → see if it works - StatusBar → see if it works - Share → see if it works Reviewed By: PeteTheHeat Differential Revision: D16957751 Pulled By: hramos fbshipit-source-id: 4dae1a8126822038891e3bc3e0aa9640b86dfe66
hramos
force-pushed
the
export-D16957751
branch
from
0244522 to
8ed4e4f
Compare
|
RNTester (Android/hermes/arm64-v8a): 3289088 bytes |
Summary: Pull Request resolved: facebook#28058 I'm taking the first step towards supporting iOS 13 UIScene APIs and modernizing React Native not to assume an app only has a single window. See discussion here: facebook#25181 (comment) The approach I'm taking is to take advantage of `RootTagContext` and passing it to NativeModules so that they can identify correctly which window they refer to. Here I'm just laying groundwork. - [x] `Alert` and `ActionSheetIOS` take an optional `rootTag` argument that will cause them to appear on the correct window - [x] `StatusBar` methods also have `rootTag` argument added, but it's not fully hooked up on the native side — this turns out to require some more work, see: facebook#25181 (comment) - [x] `setNetworkActivityIndicatorVisible` is deprecated in iOS 13 - [x] `RCTPerfMonitor`, `RCTProfile` no longer assume `UIApplicationDelegate` has a `window` property (no longer the best practice) — they now just render on the key window Next steps: Add VC-based status bar management (if I get the OK on facebook#25181 (comment) ), add multiple window demo to RNTester, deprecate Dimensions in favor of a layout context, consider adding hook-based APIs for native modules such as Alert that automatically know which rootTag to pass ## Changelog [Internal] [Changed] - Modernize Modal to use RootTagContext [iOS] [Changed] - `Alert`, `ActionSheetIOS`, `StatusBar` methods now take an optional `surface` argument (for future iPadOS 13 support) [Internal] [Changed] - Do not assume `UIApplicationDelegate` has a `window` property Pull Request resolved: facebook#25425 Test Plan: - Open RNTester and: - go to Modal and check if it still works - Alert → see if works - ACtionSheetIOS → see if it works - StatusBar → see if it works - Share → see if it works Reviewed By: PeteTheHeat Differential Revision: D16957751 Pulled By: hramos fbshipit-source-id: 7bbe694c17a490a7ba7b03e5ed31e679e2777b68
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D16957751 |
hramos
force-pushed
the
export-D16957751
branch
from
8ed4e4f to
3371772
Compare
|
RNTester (Android/hermes/arm64-v8a): 3289088 bytes |
Collaborator
|
This pull request was successfully merged by @radex in b58e176. When will my fix make it into a release? | Upcoming Releases |
4 tasks
osdnk
pushed a commit
to osdnk/react-native
that referenced
this pull request
Mar 9, 2020
Summary: Pull Request resolved: facebook#28058 I'm taking the first step towards supporting iOS 13 UIScene APIs and modernizing React Native not to assume an app only has a single window. See discussion here: facebook#25181 (comment) The approach I'm taking is to take advantage of `RootTagContext` and passing it to NativeModules so that they can identify correctly which window they refer to. Here I'm just laying groundwork. - [x] `Alert` and `ActionSheetIOS` take an optional `rootTag` argument that will cause them to appear on the correct window - [x] `StatusBar` methods also have `rootTag` argument added, but it's not fully hooked up on the native side — this turns out to require some more work, see: facebook#25181 (comment) - [x] `setNetworkActivityIndicatorVisible` is deprecated in iOS 13 - [x] `RCTPerfMonitor`, `RCTProfile` no longer assume `UIApplicationDelegate` has a `window` property (no longer the best practice) — they now just render on the key window Next steps: Add VC-based status bar management (if I get the OK on facebook#25181 (comment) ), add multiple window demo to RNTester, deprecate Dimensions in favor of a layout context, consider adding hook-based APIs for native modules such as Alert that automatically know which rootTag to pass ## Changelog [Internal] [Changed] - Modernize Modal to use RootTagContext [iOS] [Changed] - `Alert`, `ActionSheetIOS`, `StatusBar` methods now take an optional `surface` argument (for future iPadOS 13 support) [iOS] [Changed] - RCTPresentedViewController now takes a nullable `window` arg [Internal] [Changed] - Do not assume `UIApplicationDelegate` has a `window` property Pull Request resolved: facebook#25425 Test Plan: - Open RNTester and: - go to Modal and check if it still works - Alert → see if works - ACtionSheetIOS → see if it works - StatusBar → see if it works - Share → see if it works Reviewed By: PeteTheHeat Differential Revision: D16957751 Pulled By: hramos fbshipit-source-id: ae2a4478e2e7f8d2be3022c9c4861561ec244a26
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a duplicate of #25425! Do not merge.
This duplicate contains additional changes made to #25425. It's duplicated here for testing purposes.
Summary:
I'm taking the first step towards supporting iOS 13 UIScene APIs and modernizing React Native not to assume an app only has a single window. See discussion here: #25181 (comment)
The approach I'm taking is to take advantage of
RootTagContextand passing it to NativeModules so that they can identify correctly which window they refer to. Here I'm just laying groundwork.AlertandActionSheetIOStake an optionalrootTagargument that will cause them to appear on the correct windowStatusBarmethods also haverootTagargument added, but it's not fully hooked up on the native side — this turns out to require some more work, see: ☂️ Supporting Xcode 11 and iOS 13 Beta #25181 (comment)setNetworkActivityIndicatorVisibleis deprecated in iOS 13RCTPerfMonitor,RCTProfileno longer assumeUIApplicationDelegatehas awindowproperty (no longer the best practice) — they now just render on the key windowNext steps: Add VC-based status bar management (if I get the OK on #25181 (comment) ), add multiple window demo to RNTester, deprecate Dimensions in favor of a layout context, consider adding hook-based APIs for native modules such as Alert that automatically know which rootTag to pass
Changelog
[Internal] [Changed] - Modernize Modal to use RootTagContext
[iOS] [Changed] -
Alert,ActionSheetIOS,StatusBarmethods now take an optionalsurfaceargument (for future iPadOS 13 support)[Internal] [Changed] - Do not assume
UIApplicationDelegatehas awindowpropertyPull Request resolved: #25425
Test Plan:
Reviewed By: PeteTheHeat
Differential Revision: D16957751
Pulled By: hramos