Skip to content

Scene support for alerts #917

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

Merged
merged 2 commits into from
Dec 2, 2021

Conversation

HeyImChris
Copy link

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

We hit a bug downstream with the existing implementation where we use Scenes in our app which require alerts to be presented from a window associated with the app's UIWindowScene.

Currently without this fix, when we attempt to create a new UIWindow and UIViewController to present a modal alert, that UIWindow isn't associated with any of the app's scenes which results in the app silently failing to show the alert.

The fix is to more or less revert the incriminating part of this change. This is the least intrusive way to make a safe fix, but once this is in, I'd like to make a fuller fix where we find a reasonable way to pair the new UIWindow with the app's scene and continue using that instead.

Changelog

[iOS] [Alert] - Fix modal alert issue using UIScenes

Test Plan

Tested in a downstream app and this is required to show an alert in response to an action sheet closing in an app leveraging UIScenes.

@HeyImChris HeyImChris requested a review from amgleitman December 1, 2021 23:52
@HeyImChris HeyImChris self-assigned this Dec 1, 2021
@HeyImChris HeyImChris requested a review from alloy as a code owner December 1, 2021 23:52
@pull-bot
Copy link

pull-bot commented Dec 1, 2021

Warnings
⚠️

❔ Base Branch - The base branch for this PR is something other than master. Are you sure you want to merge these changes into a stable release? If you are interested in backporting updates to an older release, the suggested approach is to land those changes on master first and then cherry-pick the commits into the branch for that release. The Releases Guide has more information.

Messages
📖

📋 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • macOS
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against d02ace5

@HeyImChris HeyImChris merged commit b7752ec into microsoft:0.64-stable Dec 2, 2021
HeyImChris added a commit to HeyImChris/react-native-macos that referenced this pull request Dec 2, 2021
* scene support for alerts

* move alert changes
HeyImChris added a commit that referenced this pull request Dec 17, 2021
* android

* Scene support for alerts (#917)

* scene support for alerts

* move alert changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants