Fix RCTAlertController not showing when using SceneDelegate on iOS 13.0+ #35716

ouabing · 2022-12-25T04:47:23Z

Summary

On iOS 13.0+, app may use SceneDelegate for multiple windows support or CarPlay support. RCTAlertController can't find the correct root vc in such scene based apps.

Changelog

[iOS] [Fixed] - Fix RCTAlertController not showing when using SceneDelegate on iOS 13.0+.

Test Plan

facebook-github-bot · 2022-12-25T04:47:28Z

Hi @ouabing!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

analysis-bot · 2022-12-25T05:14:22Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,494,185	+0
android	hermes	armeabi-v7a	7,814,776	+0
android	hermes	x86	8,970,115	+0
android	hermes	x86_64	8,828,201	+0
android	jsc	arm64-v8a	9,679,692	+0
android	jsc	armeabi-v7a	8,413,897	+0
android	jsc	x86	9,744,208	+0
android	jsc	x86_64	10,221,524	+0

Base commit: 9c57a7f
Branch: main

analysis-bot · 2022-12-25T05:45:39Z

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

Base commit: 9c57a7f
Branch: main

pull-bot · 2022-12-25T05:56:31Z

PR build artifact for e468621 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

pull-bot · 2022-12-25T06:00:13Z

PR build artifact for e468621 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

facebook-github-bot · 2022-12-25T06:36:43Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

facebook-github-bot · 2022-12-27T20:06:13Z

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

cipolleschi

The code seems ok to me, I left a couple of questions to understand it better and see whether we can simplify it a little bit.

Also, can we extract lone 24 to 34 into a private function? Something like:

- (UIWindow *)getUIWindowFromScene
{
  // ...moved code...
}

This will make it easier to read the code and to eventually change/remove the code once we don't need all the checks.

What do you think?

React/CoreModules/RCTAlertController.m

cipolleschi · 2022-12-30T08:27:39Z

React/CoreModules/RCTAlertController.m

+    __IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_13_0
+    if (@available(iOS 13.0, *)) {
+      for (UIScene *scene in RCTSharedApplication().connectedScenes) {
+        if (scene.activationState == UISceneActivationStateForegroundActive && [scene isKindOfClass:[UIWindowScene class]]) {


Why the check [scene isKindOfClass:[UIWindowScene class]] is needed? Can scene be of something different from a UIWindowScene class?

For app supports CarPlay, RCTSharedApplication().connectedScenes will include CarPlay scenes such as CPTemplateApplicationScene instance, which may also in active state.

pull-bot · 2022-12-31T04:49:40Z

PR build artifact for 50273c0 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

pull-bot · 2022-12-31T04:53:40Z

PR build artifact for 50273c0 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

pull-bot · 2022-12-31T06:01:52Z

PR build artifact for cb78e7e is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

pull-bot · 2022-12-31T06:03:23Z

PR build artifact for cb78e7e is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

facebook-github-bot · 2023-01-03T21:29:00Z

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

makovkastar · 2023-01-03T23:54:38Z

@ouabing can you please rebase this PR on top of the latest commit? There are some merge conflicts.

pull-bot · 2023-01-04T03:14:11Z

PR build artifact for 03de6eb is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

pull-bot · 2023-01-04T03:16:51Z

PR build artifact for 03de6eb is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

ouabing · 2023-01-04T04:03:20Z

@makovkastar done, thank you all

facebook-github-bot · 2023-01-04T18:32:24Z

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

facebook-github-bot · 2023-01-04T19:43:21Z

@makovkastar merged this pull request in 0c53420.

….0+ (#35716) Summary: On iOS 13.0+, app may use SceneDelegate for multiple windows support or CarPlay support. RCTAlertController can't find the correct root vc in such scene based apps. ## Changelog  [iOS] [Fixed] - Fix RCTAlertController not showing when using SceneDelegate on iOS 13.0+. Pull Request resolved: #35716 Reviewed By: cipolleschi Differential Revision: D42253653 Pulled By: makovkastar fbshipit-source-id: ae4e833abca2af7af8028f3af9bd8d3f60ebd392

….0+ (facebook#35716) Summary: On iOS 13.0+, app may use SceneDelegate for multiple windows support or CarPlay support. RCTAlertController can't find the correct root vc in such scene based apps. ## Changelog  [iOS] [Fixed] - Fix RCTAlertController not showing when using SceneDelegate on iOS 13.0+. Pull Request resolved: facebook#35716 Reviewed By: cipolleschi Differential Revision: D42253653 Pulled By: makovkastar fbshipit-source-id: ae4e833abca2af7af8028f3af9bd8d3f60ebd392

see facebook/react-native#35716

* fix: unbalanced calls to begin/end appearance transitions * fix: update how toastWindow is obtained see facebook/react-native#35716

…n iOS 13.0+ (facebook#35716)" This reverts commit 0c53420.

ouabing force-pushed the main branch from 6eda0f5 to e468621 Compare December 25, 2022 04:53

react-native-bot added Bug Platform: iOS iOS applications. labels Dec 25, 2022

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 Dec 25, 2022

cipolleschi requested changes Dec 30, 2022

View reviewed changes

ouabing requested a review from cipolleschi December 31, 2022 04:23

ouabing force-pushed the main branch from 50273c0 to cb78e7e Compare December 31, 2022 05:32

cipolleschi approved these changes Jan 3, 2023

View reviewed changes

ouabing added 2 commits January 4, 2023 10:33

Fix RCTAlertController not showing when using SceneDelegate on iOS 13.0+

23d0ade

Extract scene related logic into function in RCTAlertController

03de6eb

ouabing force-pushed the main branch from cb78e7e to 03de6eb Compare January 4, 2023 02:41

facebook-github-bot closed this in 0c53420 Jan 4, 2023

facebook-github-bot added the Merged This PR has been merged. label Jan 4, 2023

Saadnajmi mentioned this pull request Jan 27, 2023

[Upstream] fix (iOS): Only try to make the alert window key if the app recognizes it #35942

Closed

snyk-bot mentioned this pull request Feb 10, 2023

[Snyk] Upgrade react-native from 0.63.3 to 0.71.1 NOUIY/react-native-macos#975

Open

vonovak added a commit to vonovak/react-native-simple-toast that referenced this pull request Aug 1, 2023

fix: update how toastWindow is obtained

eaf6a83

see facebook/react-native#35716

vonovak mentioned this pull request Aug 1, 2023

fix: update how toastWindow is obtained vonovak/react-native-simple-toast#58

Merged

vonovak added a commit to vonovak/react-native-simple-toast that referenced this pull request Aug 1, 2023

fix: update how toastWindow is obtained (#58)

5b8defb

* fix: unbalanced calls to begin/end appearance transitions * fix: update how toastWindow is obtained see facebook/react-native#35716

cipolleschi mentioned this pull request Aug 16, 2023

cipolleschi/add maven polling #39039

Merged

dnamrata0805 added a commit to dnamrata0805/react-native-fork that referenced this pull request Sep 20, 2023

Revert "Fix RCTAlertController not showing when using SceneDelegate o…

eff32ce

…n iOS 13.0+ (facebook#35716)" This reverts commit 0c53420.

cipolleschi mentioned this pull request Sep 21, 2023

[LOCAL] Copy hermes ruby files to hermes folder in local E2E test #39577

Merged

cipolleschi mentioned this pull request Oct 11, 2023

cipolleschi/fix CI #40793

Merged

cipolleschi mentioned this pull request Nov 1, 2023

[RN][CI]Update node installation on debian (0.71) #41277

Merged

This was referenced Jan 8, 2024

[0.71][RN][General] Migrate boost download url away from JFrog #42177

Merged

[RN][iOS] Fix symbol not found _jump_fcontext with use_frameworks! #42229

Merged

cipolleschi mentioned this pull request Mar 6, 2024

[RN][iOS][0.71] Fix flipper for Xcode 15.3 #43345

Merged

cipolleschi mentioned this pull request Apr 5, 2024

cipolleschi/fix yoga archive 0.71 #43895

Closed

cipolleschi mentioned this pull request Apr 26, 2024

[iOS][Privacy Manifest] Set up privacy manifest for React Native 0.71 #44281

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix RCTAlertController not showing when using SceneDelegate on iOS 13.0+ #35716

Fix RCTAlertController not showing when using SceneDelegate on iOS 13.0+ #35716

ouabing commented Dec 25, 2022

facebook-github-bot commented Dec 25, 2022

analysis-bot commented Dec 25, 2022 •

edited

Loading

analysis-bot commented Dec 25, 2022 •

edited

Loading

pull-bot commented Dec 25, 2022

pull-bot commented Dec 25, 2022

facebook-github-bot commented Dec 25, 2022

facebook-github-bot commented Dec 27, 2022

cipolleschi left a comment

cipolleschi Dec 30, 2022

ouabing Dec 31, 2022 •

edited

Loading

pull-bot commented Dec 31, 2022

pull-bot commented Dec 31, 2022

pull-bot commented Dec 31, 2022

pull-bot commented Dec 31, 2022

facebook-github-bot commented Jan 3, 2023

makovkastar commented Jan 3, 2023

pull-bot commented Jan 4, 2023

pull-bot commented Jan 4, 2023

ouabing commented Jan 4, 2023

facebook-github-bot commented Jan 4, 2023

facebook-github-bot commented Jan 4, 2023

Fix RCTAlertController not showing when using SceneDelegate on iOS 13.0+ #35716

Fix RCTAlertController not showing when using SceneDelegate on iOS 13.0+ #35716

Conversation

ouabing commented Dec 25, 2022

Summary

Changelog

Test Plan

facebook-github-bot commented Dec 25, 2022

Action Required

Process

analysis-bot commented Dec 25, 2022 • edited Loading

analysis-bot commented Dec 25, 2022 • edited Loading

pull-bot commented Dec 25, 2022

pull-bot commented Dec 25, 2022

facebook-github-bot commented Dec 25, 2022

facebook-github-bot commented Dec 27, 2022

cipolleschi left a comment

Choose a reason for hiding this comment

cipolleschi Dec 30, 2022

Choose a reason for hiding this comment

ouabing Dec 31, 2022 • edited Loading

Choose a reason for hiding this comment

pull-bot commented Dec 31, 2022

pull-bot commented Dec 31, 2022

pull-bot commented Dec 31, 2022

pull-bot commented Dec 31, 2022

facebook-github-bot commented Jan 3, 2023

makovkastar commented Jan 3, 2023

pull-bot commented Jan 4, 2023

pull-bot commented Jan 4, 2023

ouabing commented Jan 4, 2023

facebook-github-bot commented Jan 4, 2023

facebook-github-bot commented Jan 4, 2023

analysis-bot commented Dec 25, 2022 •

edited

Loading

analysis-bot commented Dec 25, 2022 •

edited

Loading

ouabing Dec 31, 2022 •

edited

Loading