-
-
Notifications
You must be signed in to change notification settings - Fork 518
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
Fix user interface style in native stack view controllers #481
Fix user interface style in native stack view controllers #481
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, left a few minor comments. Also wanted to clarify if I understand this correctly: this only applies to VC's presented modally and does not affect pushed screens? If that's the case can you update description to indicate that the problem was only with modals and hence this fix only makes changes to the logic responsible for presenting modals?
ios/RNSScreenStack.m
Outdated
|
||
if (@available(iOS 13.0, *)) { | ||
// Inherit UI style from its parent - solves an issue with incorrect style being applied to some UIKit views like date picker or segmented control. | ||
next.overrideUserInterfaceStyle = previous.overrideUserInterfaceStyle; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should make any difference in your case but I think it'd be more intuitive if presented controllers would inherit styles from their container and not from the sibling modal. If so I'd copy the property from _controller.overrideUserInterfaceStyle
here and not from previous
@@ -285,6 +285,12 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers | |||
for (NSUInteger i = changeRootIndex; i < controllers.count; i++) { | |||
UIViewController *next = controllers[i]; | |||
BOOL lastModal = (i == controllers.count - 1); | |||
|
|||
if (@available(iOS 13.0, *)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also does not matter I think as soon appstore will stop accepting builds made with prev versions of xcode (if that isn't the case already) but for consistency can we add #ifdef __IPHONE_13_0
to wrap this call much like we do in other cases where we do version checks
# Why Followup software-mansion/react-native-screens#481 # How Copied my changes I made to `react-native-screens` into our codebase and backported them to SDK37. I also had to force `EXAppViewController`s child to use parent's interface style as well. Previously I though this issue happens only for view controllers that are being presented modally. # Test Plan Tested against https://github.com/brentvatne/modal-example
# Why Followup software-mansion/react-native-screens#481 # How Copied my changes I made to `react-native-screens` into our codebase and backported them to SDK37. I also had to force `EXAppViewController`s child to use parent's interface style as well. Previously I though this issue happens only for view controllers that are being presented modally. # Test Plan Tested against https://github.com/brentvatne/modal-example
Why
In Expo client, which supports both light and dark styles, you can open an experience that has strict setting to light or dark mode. In that case experience's root view controller is set to the style specified by the user, however view controllers presented over it or added as a child have
overrideUserInterfaceStyle
set toUIUserInterfaceStyleUnspecified
which causes the view controller to use the style that is set in system settings so we get some UIKit's views rendered with incorrect interface style 😞 This only affects view controllers presented modally, thus doing the same for pushed screens is not necessary.@brentvatne prepared reproducible example: https://github.com/brentvatne/modal-example
How
Native stack's view controllers will inherit
overrideUserInterfaceStyle
from presenting view controller.Test Plan
Example https://github.com/brentvatne/modal-example works as expected in Expo client with style set to Dark in system settings. *This also requires one small change in the client itself, so it won't work in the current version in the app store.