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

Support the default value of UIViewControllerBasedStatusBarAppearance (true) #11710

Closed
steipete opened this issue Jan 3, 2017 · 32 comments
Closed
Labels
Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.

Comments

@steipete
Copy link

steipete commented Jan 3, 2017

Description

React Native currently requires that UIViewControllerBasedStatusBarAppearance is set to NO. This is an app-global setting and Apple offers this to help bringing old (pre-iOS 7) apps over to newer OS versions. It's not recommended that this is set for new apps. The methods for manual status bar control are already deprecated and Apple explicitly discourages using them:

To opt out of the view controller-based status bar appearance behavior, you must add the UIViewControllerBasedStatusBarAppearance key with a value of false to your app’s Info.plist file, but doing so is not recommended.
https://developer.apple.com/reference/uikit/uiapplication/1622923-setstatusbarstyle#discussion

The check was added in 2015:
https://github.com/facebook/react-native/blame/64a4c6070df7e711e7fd01c490f369bbd0d0fb28/React/Modules/RCTStatusBarManager.m#L109-L111

At PSPDFKit we offer a React Native module. We also have a very similar check that warns if the value of UIViewControllerBasedStatusBarAppearance is still set to the old behavior.
https://pspdfkit.com/blog/2016/react-native-module/

Here's our FAQ article about it:
https://pspdfkit.com/guides/ios/current/faq/uiviewcontrollerbasedstatusbarappearance/

Other platforms (Xamarin) are switching to the new style as well:
xamarin/Xamarin.Forms#463

Reproduction

See the code check for UIViewControllerBasedStatusBarAppearance. React should either support both or just default to the non-deprecated style. Currently any modification of the status bar requires this key to be set to false (true is the default)

Solution

This is tricky. See the related discussion on Twitter with Nick who wrote the original check:
https://twitter.com/nicklockwood/status/816334935422337025

I realize that React doesn't own the parent View Controller. There are still quite a few ways how the parent VC can be accessed and how hooks could be installed to both have the current flexibility (so we do not break any API) and offer compatibility with the new way of controlling the status bar.

I am willing to help and work on this but I'd first love to hear if there are any discussions around this that I missed while searching for it and what the mid- to long-term plans here are. I know that everything related to the status bar on iOS is painful, so I'm apologizing in advance for bringing up this issue.

Additional Information

  • React Native version: 0.39
  • Platform: iOS
  • Operating System: MacOS
@ericvicenti
Copy link
Contributor

@ide, this may be related to your team's work on exponent view for iOS. Maybe you can tag whoever has the most context

@ide
Copy link
Contributor

ide commented Jan 4, 2017

@ericvicenti We'd probably want to come up with whatever API makes sense for RCTRootView and then can expose a similarly structured API through ExponentView.

@steipete I haven't heard of any discussions about this since the original method works fine in most cases but I do think it's a good idea to be ready for when they finally do remove these APIs. There is a class called RCTRootViewDelegate that the programmer could hook up to the owning VC and we could add a method like reactRootViewDidRequestStatusBarStyleChange.

The tricky part is that an app can have multiple RCTRootViews per RCTBridge (the true core of React Native). I think the naive solution is for calls to StatusBar.setStyle(whatever) to be fed through the bridge to all RCTRootViews and up through each of their delegates, i.e. broadcast the status bar change to all owning VCs. I believe this would mostly match the semantics of the current API and globally change the status bar.

We probably do want to keep the deprecated API for now in case one of Facebook's apps uses it, if we break a FB app the commit is just going to be reverted.

@sophiebits
Copy link
Contributor

StatusBar.setStyle

Didn't Apple introduce VC-based status bar control since global imperative calls to set the status bar state were too unwieldy? :) Maybe what we need is a RCTRootViewController for full-screen views and a declarative way to request a status bar appearance in JS where it's clear what takes precedence in the case of a conflict.

@ide
Copy link
Contributor

ide commented Jan 4, 2017

We still need an imperative way to change the status bar style for the current VC, whether that's an RCTRootViewController or just any UIViewController that conforms to RCTRootViewDelegate. I prefer the latter to keep the API surface area of RN more flexible.

Most RN apps are housed under one RCTRootView (and therefore one VC) so we do need a way to request status bar changes. They have to be imperative at the Obj-C level I think because all bridge calls are imperative. Navigation libraries can then wrap the imperative call in a declarative API in user space.

@ericvicenti
Copy link
Contributor

Yep, at some point every API needs to be imperative. I never really understood why we decided to make a status bar component- because the behavior can be totally unexpected when there are multiple StatusBar components in the hierarchy but only one global status bar to control. I think it is a pretty safe bet to treat a app-global API like a global API for our JS app.

@ide
Copy link
Contributor

ide commented Jan 4, 2017

@ericvicenti I agree, I find the current StatusBar component short-sighted. Thinking about tab bars and drawers and other navigation paradigms, there are many cases where there are two scene components mounted on-screen at the same time and you can toggle between which one is "active". React has no notion of the "active" screen but the navigation API does -> react-navigation is the right level of abstraction to expose a status bar component or prop.

@ericvicenti
Copy link
Contributor

React has no notion of the "active" screen but the navigation API does -> react-navigation is the right level of abstraction to expose a status bar component or prop.

Totally agree!

I have the exact same gripes with the <Modal> component

@sophiebits
Copy link
Contributor

Well, it would be nice long-term if we can clearly delineate which are best avoided in structured applications – or else people will surely use the imperative one even when they should be using a navigation-based one. You know how people are…

@SudoPlz
Copy link
Contributor

SudoPlz commented Jun 21, 2017

Any news on that?

@hramos hramos added the Icebox label Aug 24, 2017
@hramos
Copy link
Contributor

hramos commented Aug 24, 2017

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

  • Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
  • If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
  • Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

@hramos hramos closed this as completed Aug 24, 2017
@steipete
Copy link
Author

@hramos This is a conceptual issue and I have not seen a fix for it yet - please re-open this ticket.

@sophiebits sophiebits reopened this Aug 24, 2017
@hramos hramos added Type: Discussion Long running discussion. and removed Icebox labels Aug 25, 2017
@hramos
Copy link
Contributor

hramos commented Aug 25, 2017

I've applied the "For Discussion" label, which should prevent this issue from getting closed in the future due to inactivity.

@steipete
Copy link
Author

We've got another ping from a customer that uses RCTStatusBarManager which requires UIViewControllerBasedStatusBarAppearance to be set to NO. Are there any plans to convert RCTStatusBarManager to work with the root view controller? If RN is all view-based, shouldn't that be relatively easy to convert (forgive me my ignorance if I got that wrong)

@PvanHengel
Copy link

I agree that react native should at least support both modes, its the direction apple seems to recommend. There are plugins that expect this setting to be YES, and throw an error when it is set to NO like RN likes. I have upgraded to all the latest versions and still see the issue. I'm not all that familiar with the inner workings of what this change implies, but clearly its worth someone proposing a backward compatible "fix" rather than just parking this as a discussion topic.

@steipete
Copy link
Author

steipete commented Sep 27, 2017

Here's another plugin that requires UIViewControllerBasedStatusBarAppearance to be set to YES.
Nubescope/react-native-facebook-account-kit#21

react-native-navigation supports both modes:
https://github.com/wix/react-native-navigation/blob/master/ios/RCCViewController.m

@steipete
Copy link
Author

steipete commented Nov 3, 2017

Is nobody else running into this issue?

@Ragnar-H
Copy link

@steipete I'm running into this issue with a combination of react-native-navigation and storybook

The problem being that we're using UIViewControllerBasedStatusBarAppearance YES in the actual app (supported by react-native-navigation), but then storybook won't run.

Dunno how much help this is...guess this is just a wordy "+1" 😅

@FlaviooLima
Copy link

@steipete I'm getting this error too.
I don't know if you know, but as long you put the value to YES and remove anything calling StatusBar the error disappear. Don't forget the libraries :)
Not a solution but...

@derrh
Copy link

derrh commented May 9, 2018

@Ragnar-H, did you come up with a work-around? I'm running into the same issue (rnn + storybook).

In the short term I'd love a suggestion to suppress this error:

screen shot 2018-05-09 at 11 15 48 am

I have tried componentDidCatch in the parent component that is rendering the Storybook root component (which I presume is causing the error), but that does not seem to work. Perhaps since the error is happening in the native code?

@FlaviooLima
Copy link

@derrh @Ragnar-H
In Storybook on page "storybook/app/react-native/src/preview/components/OnDeviceUI/index.js" it's calling the StatusBar component from react-native.

import { Dimensions, View, TouchableWithoutFeedback, Image, Text, StatusBar } from 'react-native';
You can solve your problem by removing the import of statusBar on that page.
I advise you to create a fork and maintain or ask to remove the need for statusBar.
I don't understand why some libraries insist on taking care of statusBar, the only person who should change the statusBar should be the developer.

@steipete
Copy link
Author

I don't understand why some libraries insist on taking care of statusBar, the only person who should change the statusBar should be the developer.

On iOS, the status bar should always match the topmost view controller, and Apple recognized and changed this behavior in iOS 7. deprecating the previous behavior, which basically were global toggles that any could could call at any time - which often was very challenging when view controllers pushed that to store the status bar style and then reset it.

Since React Native doesn't use the view controller concept and simply manages views, it makes sense to have such a status bar manager. There's still one rootViewController, and it should be possible to make this root controller manage and forward the status bar management, so React apps can move over to the modern status bar management without any functional regressions. (I am not that deep in React to verify this, potentially it's enough to add that to https://github.com/facebook/react-native/blob/master/React/Views/RCTWrapperViewController.m)

@FlaviooLima
Copy link

@steipete I'm not saying that a framework like react-native shouldn't have control over statusBar.
I'm saying that packages like react-native-elements and storybook, shouldn't call the statusBar component from react-native. The only person to dictate the statusBar should be the developer and not be forced by some component package.
But hey this is just my two cents :)

@Ragnar-H
Copy link

Sorry if this is derailing the main discussion of this issue.

@derrh we turned off onDeviceUI for our use case.

Look for getStorybookUI and set to false:
const StorybookUIRoot = getStorybookUI({ port: 7007, onDeviceUI: false });

@derrh
Copy link

derrh commented May 21, 2018

@steipete, this is one of the big impedance mismatches with react-native on iOS. Much of the API for UIKit is in UIViewController et al. It is not safe to assume that all users of React Native will always have a single RCTWrapperViewController. Apps using wix/react-native-navigation or embedding RCTViews in their own view controller will have none.

@FlaviooLima, I agree. React Native libraries should not touch the StatusBar api. I'll fork storybook and work with them to remedy. Sorry for derailing.

@Ragnar-H, thanks! onDeviceUI: false solved the issue for my immediate use case.

@derrh
Copy link

derrh commented May 24, 2018

One possible solution would be to use the modern non-deprecated API for status bar appearance and have the StatusBar api interface with RCTWrapperViewController which would need to be updated to adopt the status bar appearance API. This would allow consistent behavior for users who are doing a typical React Native app, but also prevent errors for those who are embedding RN in their custom view controllers or using a 3rd party nav solution such as wix/rnn.

@sophiebits
Copy link
Contributor

I am not sure but I believe most people using React Native don't go through RCTWrapperViewController which makes this a more complex conversation than just fixing the status bar API unfortunately. :( I know this is an awkward mismatch but it doesn't seem like it is easy to rectify.

@sryze
Copy link
Contributor

sryze commented Nov 1, 2018

What is the suggested way to deal with view controllers from third-party SDKs that want to set their own status bar style?

For example, I don't use the StatusBar module anywhere in my app, but if I set UIViewControllerBasedStatusBarAppearance to YES I get the error mentioned above. I guess some third party module tries to use StatusBar.

If I want to use let's say the Facebook Login SDK, it shows its own view controller (for login) with a light navigation bar, t looks bad when the status bar style is also set to light.

facebook-github-bot pushed a commit that referenced this issue Jan 22, 2019
…21206)

Summary:
This PR exposes three static methods (`pushStackEntry`, `popStackEntry`, and `replaceStackEntry`) on StatusBar that enable imperative manipulation of the StatusBar style within the stack established by mounted StatusBar components.

Motivation:
----------

The StatusBar **component** provides a sensible API for manipulating that StatusBar style: every time a StatusBar component is mounted, its props are pushed onto a stack, and the props from the most recently mounted component are applied.

However, there are some scenarios where you may need to manipulate the StatusBar style from imperative code — particularly when invoking imperative third-party APIs that cause UI to appear. (For example, a user feedback utility or bug reporter that launches a full-screen modal.)

In modern iOS development, `UIViewControllerBasedStatusBarAppearance` is typically set to `YES`, which allows the third-party UIViewController to specify its preferred status bar style. However, as has been discussed at length in #11710, React Native has disabled this setting, which means that either the app's code or the third-party's React Native wrapper needs to manually manipulate React Native's StatusBar API to achieve the desired outcome.

The existing imperative StatusBar APIs are not a good fit for these needs because they simply overwrite the existing StatusBar styles, and provide no means of reverting StatusBar style changes when the third-party UI is dismissed.

To improve upon this situation, this PR makes it possible to call `StatusBar.pushStackEntry` before launching the third-party UI, wait for the UI to dismiss, and then call `StatusBar.popStackEntry` (supplying the token returned from the push call).

I've featured the new stack-based imperative methods in the documentation, but stopped short of explicitly deprecating the older imperative methods — though I can think of no reason not to deprecate them. Feedback is welcome on this point.

Release Notes:
--------------

[GENERAL] [ENHANCEMENT] [StatusBar] - Add static methods to manipulate StatusBar stack imperatively

Pull Request resolved: #21206

Differential Revision: D9945247

Pulled By: cpojer

fbshipit-source-id: ec118268cff5b47e87be81d0b9e1728ecc3a9b02
@stale
Copy link

stale bot commented Feb 2, 2019

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Feb 2, 2019
@fbartho
Copy link
Contributor

fbartho commented Feb 2, 2019

As far as I know, this is still an unsolved conceptual issue!

@steipete
Copy link
Author

steipete commented Feb 2, 2019

This issue is still unsolved. React Native needs to move to view controller based status bar management to be compatible with 3rd-party SDKs.

matt-oakes pushed a commit to matt-oakes/react-native that referenced this issue Feb 7, 2019
…acebook#21206)

Summary:
This PR exposes three static methods (`pushStackEntry`, `popStackEntry`, and `replaceStackEntry`) on StatusBar that enable imperative manipulation of the StatusBar style within the stack established by mounted StatusBar components.

Motivation:
----------

The StatusBar **component** provides a sensible API for manipulating that StatusBar style: every time a StatusBar component is mounted, its props are pushed onto a stack, and the props from the most recently mounted component are applied.

However, there are some scenarios where you may need to manipulate the StatusBar style from imperative code — particularly when invoking imperative third-party APIs that cause UI to appear. (For example, a user feedback utility or bug reporter that launches a full-screen modal.)

In modern iOS development, `UIViewControllerBasedStatusBarAppearance` is typically set to `YES`, which allows the third-party UIViewController to specify its preferred status bar style. However, as has been discussed at length in facebook#11710, React Native has disabled this setting, which means that either the app's code or the third-party's React Native wrapper needs to manually manipulate React Native's StatusBar API to achieve the desired outcome.

The existing imperative StatusBar APIs are not a good fit for these needs because they simply overwrite the existing StatusBar styles, and provide no means of reverting StatusBar style changes when the third-party UI is dismissed.

To improve upon this situation, this PR makes it possible to call `StatusBar.pushStackEntry` before launching the third-party UI, wait for the UI to dismiss, and then call `StatusBar.popStackEntry` (supplying the token returned from the push call).

I've featured the new stack-based imperative methods in the documentation, but stopped short of explicitly deprecating the older imperative methods — though I can think of no reason not to deprecate them. Feedback is welcome on this point.

Release Notes:
--------------

[GENERAL] [ENHANCEMENT] [StatusBar] - Add static methods to manipulate StatusBar stack imperatively

Pull Request resolved: facebook#21206

Differential Revision: D9945247

Pulled By: cpojer

fbshipit-source-id: ec118268cff5b47e87be81d0b9e1728ecc3a9b02
@stale
Copy link

stale bot commented Feb 9, 2019

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

@stale stale bot closed this as completed Feb 9, 2019
@hramos hramos reopened this Feb 9, 2019
@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Feb 9, 2019
@hramos hramos added the Platform: iOS iOS applications. label Feb 9, 2019
@cpojer
Copy link
Contributor

cpojer commented Feb 15, 2019

This issue has been moved to rnc-archive/react-native-statusbar#5.

@cpojer cpojer closed this as completed Feb 15, 2019
@facebook facebook locked as resolved and limited conversation to collaborators Feb 15, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.
Projects
None yet
Development

No branches or pull requests