fix: RCTAlertController's UserInterfaceStyle to follow root window

[vonovak]

Summary

The motivation of this PR is for the Alert to follow the same style override (overrideUserInterfaceStyle being light/dark) as the one used by the root window (UIApplication.sharedApplication.delegate.window).

This is something that has worked previously because RCTPResentedViewController() was used to present the Alert (the behavior has changed in vonovak@f319ff3). With the former approach, the alert would "inherit" the userInterfaceStyle of the view controller it was presented within (and that one, in turn, would "inherit" from UIApplication.sharedApplication.delegate.window).

With the current approach, the "style inheritance" does not work with the view controller being created here.

Because this viewcontroller instance does not have where to "inherit" the styling from, the styling might be different from the rest of the app. This PR fixes that.

Changelog

[iOS] [Fixed] - fix: RCTAlertController's UserInterfaceStyle to follow root window

Test Plan

Instead of

self.overrideUserInterfaceStyle = UIApplication.sharedApplication.delegate.window.overrideUserInterfaceStyle;

you can do

self.overrideUserInterfaceStyle = UIUserInterfaceStyleDark;

and observe the result. So if the override is set, it'll manifest itself. If it's not set, the value of UIApplication.sharedApplication.delegate.window.overrideUserInterfaceStyle will be UIUserInterfaceStyleUnspecified, and it'll have no effect.

screenshot

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,633,226	-65
android	hermes	armeabi-v7a	7,045,220	-50
android	hermes	x86	7,935,184	-33
android	hermes	x86_64	7,907,093	-53
android	jsc	arm64-v8a	9,510,925	-64
android	jsc	armeabi-v7a	8,286,278	-53
android	jsc	x86	9,450,609	-41
android	jsc	x86_64	10,041,879	-55

Base commit: 7909b91
Branch: main

[analysis-bot]

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

Base commit: e6ef083
Branch: main

[cipolleschi]

Thanks for opening this PR. 👏 👏
I left a feedback and a question!

[cipolleschi]

Instead of UIApplication.sharedApplication, can yo use initWithFrame:RCTSharedApplication()?
The UIApplication is not always available (in extensions, for example).

What happens if UIApplication.sharedApplication.delegate.window.overrideUserInterfaceStyle; returns nil?

[vonovak]

@cipolleschi I have refactored the code to use RCTSharedApplication()

What happens if ...overrideUserInterfaceStyle; returns nil?

I don't believe that should be happening; it'd rather be UIUserInterfaceStyleUnspecified. But to be on the safe side of things, I made the assignment default to UIUserInterfaceStyleUnspecified in case RCTSharedApplication() were to return nil.

[cipolleschi]

Unfortunately, if any part of the chain is nil, it can happen due to opional chaining - this is in Swift, but it works in the same way for Objective-C.

If RCTSharedApplication return null, the final object returned will be null, for example. In Objective-C, you can call any method on a null instance and get a null in return! 😄

[vonovak]

Unfortunately, if any part of the chain is nil, it can happen

yes, I'm aware of that 🙂 and it's why I made the assignment default to UIUserInterfaceStyleUnspecified.

maybe I don't understand extensions enough, I just thought this code wouldn't be called in an extension, but I can be wrong 😄
there is also this part which I thought would handle the nil case more explicitly if there was a risk of having nil RCTSharedApplication(). Is that part okay from that point of view? Thanks.

react-native/React/CoreModules/RCTAlertController.m

Line 23 in 6fcfe2e

_alertWindow = [[UIWindow alloc] initWithFrame:RCTSharedApplication().keyWindow.bounds];

[cipolleschi]

Thank you for the changes!

[facebook-github-bot]

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

[cipolleschi]

Hi! There are some failure in the CI, but I think they will go away after a rebase. Could you please try @vonovak? 🙏

[vonovak]

Hi! There are some failure in the CI, but I think they will go away after a rebase. Could you please try @vonovak? 🙏

done.

the failures I see seem unrelated...

[cipolleschi]

Hi @vonovak, sorry to ask this, but could you rebase it again.. 😅

[facebook-github-bot]

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

[react-native-bot]

This pull request was successfully merged by @vonovak in 18542b6.

^{When will my fix make it into a release? | Upcoming Releases}