[iOS] Moving towards UIWindowScene support #25425

radex · 2019-06-28T09:21:47Z

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 RootTagContext and passing it to NativeModules so that they can identify correctly which window they refer to. Here I'm just laying groundwork.

Alert and ActionSheetIOS take an optional rootTag argument that will cause them to appear on the correct window
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: ☂️ Supporting Xcode 11 and iOS 13 Beta #25181 (comment)
setNetworkActivityIndicatorVisible is deprecated in iOS 13
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 #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

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

…o per-uiscene-statusbar

shergin · 2019-07-31T16:46:39Z

I believe,

We should land this.
We should decouple all of this components from the core.
We should figure out a better API for this and migrate over it.

Speaking about the better API, I think we have to use some kind of reference to any view to specify a concrete surface, not a root one (so, the root one can be inferred from a view). Now we can use reactTag for classic react, but later we have to migrate to some non-numerical reference to the view (compare implementation of UIManager.measure in Paper and Fabric for reference).

@radex Radek, I would prefer if we fix this PR to use a view reference (not a root tag), and then land. But I am also okay with landing this as is now. LMK which approach you prefer.

hramos · 2019-07-31T17:14:05Z

Thanks for taking a look, Valentin. @radex, I can help land this after the feedback is taken into account.

radex · 2019-08-02T07:45:57Z

Speaking about the better API, I think we have to use some kind of reference to any view to specify a concrete surface, not a root one (so, the root one can be inferred from a view). Now we can use reactTag for classic react, but later we have to migrate to some non-numerical reference to the view (compare implementation of UIManager.measure in Paper and Fabric for reference).

That makes a lot of sense to me! Let me make those changes :)

Libraries/Alert/Alert.js

radex · 2019-08-02T09:08:30Z

@hramos I took Valentin's feedback into account and revised the PR.

Also added some examples to RNTester & fixed up Flow types where I noticed they were wrong

radex · 2019-08-08T07:59:52Z

@hramos did you have a chance to take a look at the PR? Let me know what I can do to get it landed

facebook-github-bot

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

radex · 2019-09-09T10:30:21Z

@hramos hey, this seems stuck on import for 18 days… is there something I can do to push this along?

…sbar

facebook-github-bot

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

jpshelley · 2019-11-26T20:33:14Z

Whats the status of this PR? Its now quite out of date, I see its landed internally but not been released yet. What can we do to help? With the iOS requirements approaching, this seems vital

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: 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: f9d36ecdb3a11b137b36234049e9e256b88b45ac

hramos · 2020-02-14T00:33:20Z

I apologize for the huge delay here. I'm working on getting this merged.

hramos · 2020-02-14T06:40:09Z

I resolved all conflicts and fixed some Flow issues. I still need to go through our internal apps and update some call sites. You can find an early copy of the internal diff with conflicts resolved in #28058. When the internal diff gets merged, it should attribute the change back to PR #25425.

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

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

Summary: {emoji:26a0} This is a follow up to #25425 -- which isn't merged yet… See https://github.com/facebook/react-native/pull/25919/files/2a286257a6553a80a34e2b1f1ad94fc7bae36ea3..125aedbedc234c65c8d1b2133b79e926ad6cf145 for actual diff Currently, StatusBar native module manages the status bar on iOS globally, using `UIApplication.` APIs. This is bad because: - those APIs have been deprecated for 4 years - Apple really, really wants you to have an explicitly defined view controller, and control the status bar there - it [breaks external native components](#25181 (comment)) - it's [not compatible with iPadOS 13 multi window support](#25181 (comment)) for those reasons I we should transition towards view controller-based status bar management. With that, there is a need to introduce a default React Native root view controller, so I added `RCTRootViewController`. Using it is completely opt-in and there is no breaking change here. However I believe this should be a part of the template for new RN iOS apps. Additionally, I added `RCTRootViewControllerProtocol` with hooks needed for RCTStatusBarManager to control the status bar. This means apps that want to have total control over their view controller can still opt in to react native VC-based status bar by conforming their root view controller to this protocol. ## Changelog [iOS] [Added] - Added `RCTRootViewController` and `RCTRootViewControllerProtocol` [iOS] [Fixed] - `UIViewControllerBasedStatusBarAppearance=YES` no longer triggers an error as long as you use `RCTRootViewController` [iOS] [Fixed] - Status bar style is now correctly changed in multi-window iPadOS 13 apps if you use `RCTRootViewController` and set `UIViewControllerBasedStatusBarAppearance=YES` Pull Request resolved: #25919 Test Plan: - Open RNTester → StatusBar → and check that no features broke Reviewed By: fkgozali Differential Revision: D16957766 Pulled By: hramos fbshipit-source-id: 9ae1384ee20a06933053c4404b8237810f1e7c2c

hramos · 2020-03-05T21:02:42Z

💔 We had to back 80e6d67 out. Some of our own applications ran into issues with the changes; nothing that can't be fixed. I'll work on getting this change into the codebase again ASAP.

facebook-github-bot · 2020-03-06T03:46:07Z

@hramos merged this pull request in b58e176.

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

Summary: {emoji:26a0} This is a follow up to facebook#25425 -- which isn't merged yet… See https://github.com/facebook/react-native/pull/25919/files/2a286257a6553a80a34e2b1f1ad94fc7bae36ea3..125aedbedc234c65c8d1b2133b79e926ad6cf145 for actual diff Currently, StatusBar native module manages the status bar on iOS globally, using `UIApplication.` APIs. This is bad because: - those APIs have been deprecated for 4 years - Apple really, really wants you to have an explicitly defined view controller, and control the status bar there - it [breaks external native components](facebook#25181 (comment)) - it's [not compatible with iPadOS 13 multi window support](facebook#25181 (comment)) for those reasons I we should transition towards view controller-based status bar management. With that, there is a need to introduce a default React Native root view controller, so I added `RCTRootViewController`. Using it is completely opt-in and there is no breaking change here. However I believe this should be a part of the template for new RN iOS apps. Additionally, I added `RCTRootViewControllerProtocol` with hooks needed for RCTStatusBarManager to control the status bar. This means apps that want to have total control over their view controller can still opt in to react native VC-based status bar by conforming their root view controller to this protocol. ## Changelog [iOS] [Added] - Added `RCTRootViewController` and `RCTRootViewControllerProtocol` [iOS] [Fixed] - `UIViewControllerBasedStatusBarAppearance=YES` no longer triggers an error as long as you use `RCTRootViewController` [iOS] [Fixed] - Status bar style is now correctly changed in multi-window iPadOS 13 apps if you use `RCTRootViewController` and set `UIViewControllerBasedStatusBarAppearance=YES` Pull Request resolved: facebook#25919 Test Plan: - Open RNTester → StatusBar → and check that no features broke Reviewed By: fkgozali Differential Revision: D16957766 Pulled By: hramos fbshipit-source-id: 9ae1384ee20a06933053c4404b8237810f1e7c2c

[Modal] Modernize Modal to use RootTagContext

9cc5b5e

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 28, 2019

react-native-bot added Component: StatusBar Platform: iOS iOS applications. labels Jun 28, 2019

radex added 5 commits June 28, 2019 12:28

[StatusBar] Support UIWindowScene APIs - WIP

7c3913c

[ios] RCTPresentedViewController - make window explicit

55a8a68

[ios] pass rootTag to Alert, ActionSheetIOS

4601e22

Don't rely on UIAppDelegate.window

43259a4

[ios] clean up

7b329fb

radex changed the title ~~[iOS] Per-UIScene StatusBar~~ [iOS] Initial UIWindowScene support Jun 28, 2019

radex marked this pull request as ready for review June 28, 2019 14:51

radex requested a review from shergin as a code owner June 28, 2019 14:51

Merge branch 'master' of https://github.com/facebook/react-native int…

41db410

…o per-uiscene-statusbar

hramos mentioned this pull request Jul 2, 2019

☂️ Supporting Xcode 11 and iOS 13 Beta #25181

Closed

3 tasks

hramos added the iOS 13 label Jul 3, 2019

hramos self-assigned this Jul 31, 2019

Merge branch 'master' into per-uiscene-statusbar

5938026

radex force-pushed the per-uiscene-statusbar branch from 9e378fa to 5938026 Compare August 2, 2019 07:51

radex changed the title ~~[iOS] Initial UIWindowScene support~~ [iOS] Moving towards UIWindowScene support Aug 2, 2019

radex added 4 commits August 2, 2019 10:24

[uiscene] rootTag -> any surface (root tag, host object, component ref)

5724e11

fix imports

303e7a7

fix potential crash

e3dd976

Add RNTester examples of passing surface

47e2475

radex commented Aug 2, 2019

View reviewed changes

Libraries/Alert/Alert.js Outdated Show resolved Hide resolved

fix types

dc44848

fix types - take 2

1b90cb8

fix types - take 3

2a28625

radex mentioned this pull request Aug 2, 2019

[iOS] UIViewController-based status bar management #25919

Closed

radex added 2 commits August 2, 2019 13:15

fix types - take 4

4ca185d

i give up, flow

e699526

Merge branch 'master' into per-uiscene-statusbar

76cc618

facebook-github-bot reviewed Aug 22, 2019

View reviewed changes

radex added 5 commits September 27, 2019 09:09

unbreak merge?

ccfb397

Merge remote-tracking branch 'facebook/master' into per-uiscene-statu…

b45fec4

…sbar

Alert - make options read-only

2267b91

fix some flow issues

1800be9

fix flow issues

e6f5e93

facebook-github-bot reviewed Sep 27, 2019

View reviewed changes

hramos mentioned this pull request Feb 13, 2020

Moving towards UIWindowScene support (#25425) #28058

Closed

4 tasks

facebook-github-bot closed this in b58e176 Mar 4, 2020

facebook-github-bot added the Merged This PR has been merged. label Mar 6, 2020

xiao99xiao mentioned this pull request Apr 1, 2020

0.62.0 Discussion react-native-community/releases#179

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[iOS] Moving towards UIWindowScene support #25425

[iOS] Moving towards UIWindowScene support #25425

radex commented Jun 28, 2019 •

edited

Loading

shergin commented Jul 31, 2019

hramos commented Jul 31, 2019

radex commented Aug 2, 2019

radex commented Aug 2, 2019

radex commented Aug 8, 2019

facebook-github-bot left a comment

radex commented Sep 9, 2019

facebook-github-bot left a comment

jpshelley commented Nov 26, 2019

hramos commented Feb 14, 2020

hramos commented Feb 14, 2020

hramos commented Mar 5, 2020

facebook-github-bot commented Mar 6, 2020

[iOS] Moving towards UIWindowScene support #25425

[iOS] Moving towards UIWindowScene support #25425

Conversation

radex commented Jun 28, 2019 • edited Loading

Summary

Changelog

Test Plan

shergin commented Jul 31, 2019

hramos commented Jul 31, 2019

radex commented Aug 2, 2019

radex commented Aug 2, 2019

radex commented Aug 8, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

radex commented Sep 9, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

jpshelley commented Nov 26, 2019

hramos commented Feb 14, 2020

hramos commented Feb 14, 2020

hramos commented Mar 5, 2020

facebook-github-bot commented Mar 6, 2020

radex commented Jun 28, 2019 •

edited

Loading