Skip to content

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

Closed
Saadnajmi wants to merge 2 commits into
react:mainfrom
Saadnajmi:alert
Closed

[Upstream] fix (iOS): Only try to make the alert window key if the app recognizes it#35942
Saadnajmi wants to merge 2 commits into
react:mainfrom
Saadnajmi:alert

Conversation

@Saadnajmi

@SaadnajmiSaadnajmi commented Jan 24, 2023

Copy link
Copy Markdown
Contributor

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 🥳!

Resolvesmicrosoft#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-botfacebook-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

analysis-bot commented Jan 24, 2023

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,462,911-19
androidhermesarmeabi-v7a7,783,489+208
androidhermesx868,936,172+272
androidhermesx86_648,794,233+425
androidjscarm64-v8a9,648,873-14
androidjscarmeabi-v7a8,383,370+207
androidjscx869,711,124+262
androidjscx86_6410,188,137+422

Base commit: 6f7428e
Branch: main

@Saadnajmi

Copy link
Copy Markdown
ContributorAuthor

This may be a duplicate of #35716

@Saadnajmi

Copy link
Copy Markdown
ContributorAuthor

Closing as there is already a similar fix merged.

@Saadnajmi
Saadnajmi deleted the alert branch April 6, 2023 23:28
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.p: MicrosoftPartner: MicrosoftPartner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upstream RCTAlertController fix for multi window iOS scenarios

3 participants

@Saadnajmi@analysis-bot@facebook-github-bot