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

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

Closed
wants to merge 2 commits into from

Conversation

Saadnajmi
Copy link
Contributor

@Saadnajmi Saadnajmi commented Jan 24, 2023

Summary

This is a change we made in our fork (React Native macOS) that we are now upstreaming to reduce the number of diffs between React Native Core and React Native macOS. Also.. one less crash 🥳!

Resolves microsoft#1675

Original PR Notes:

In iOS 14, if we call -[UIWindow makeKeyAndVisible] from a UIWindow that isn't part of the app, the current key window gets made non-key. This can result in RCTKeyWindow(), and thusly, RCTPresentedViewController(), to come back as nil, which means that the alert never shows up.

The fix here is to ensure that we don't try to make the alert window key unless it's actually tracked by the application.

Changelog

[IOS] [FIXED] - Only try to make the alert window key if the app recognizes it

Test Plan

Build should pass. This change has been running in our fork in production for a while so we're fairly confident of it.

@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: Microsoft Partner: Microsoft Partner labels Jan 24, 2023
@analysis-bot
Copy link

analysis-bot commented Jan 24, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,462,911 -19
android hermes armeabi-v7a 7,783,489 +208
android hermes x86 8,936,172 +272
android hermes x86_64 8,794,233 +425
android jsc arm64-v8a 9,648,873 -14
android jsc armeabi-v7a 8,383,370 +207
android jsc x86 9,711,124 +262
android jsc x86_64 10,188,137 +422

Base commit: 6f7428e
Branch: main

@Saadnajmi
Copy link
Contributor Author

This may be a duplicate of #35716

@Saadnajmi
Copy link
Contributor Author

Closing as there is already a similar fix merged.

@Saadnajmi Saadnajmi closed this Jan 31, 2023
@Saadnajmi Saadnajmi deleted the alert branch April 6, 2023 23:28
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. p: Microsoft Partner: Microsoft Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upstream RCTAlertController fix for multi window iOS scenarios
3 participants