Add StatusBar height constant and improve implementation

[janicduplessis]

This adds a HEIGHT constant on StatusBar on Android. I needed only this for now but I will work on a better status bar dimensions API later (see TODO).

It also improves the implementation to fix a bug that happened when multiple StatusBar components get updated in the same frame as well as remove useless calls to the StatusBarModule when values did not change.

Instead of calling the StatusBarManager immediately when the component gets updated and relying on the order of the calls that get dispatched to native we now wait at the end of the frame to send the calls to the StatusBarManager using setImmediate. To make this work properly we need to change the data structure of the props stack a little bit to store the desired transition/animation too for each value.

Finally this updates the example to only show the ones that work for the current platform.

Test plan
In the UIExplorer Example, in the 'StatusBar dimensions' section it should show 25 for the height of the status bar.
All the StatusBar props should still work as they were before but the StatusBarManager should only get called once per frame and only for the values that changed.

[facebook-github-bot]

By analyzing the blame information on this pull request, we identified @janicduplessis to be a potential reviewer.

[janicduplessis]

Also this depends on the fix in #6192

[janicduplessis]

cc @nicklockwood

[bestander]

fyi buck build is broken

[satya164]

Why @Nullable? Doesn't it always return a Map?

[janicduplessis]

The method in the base class is @Nullable, I just kept the same signature.

[ide]

You should be able to leave off the @Nullable assuming the return types are covariant (think I have that right)

[janicduplessis]

@bestander Thanks for being the gatekeeper of the buck build, I need to stop forgetting about testing that :)

[facebook-github-bot]

@janicduplessis updated the pull request.

[brentvatne]

@facebook-github-bot shipit

[facebook-github-bot]

Thanks for importing. If you are an FB employee go to Phabricator to review.

[ide]

Heads up that on iOS the status bar height isn't a constant and is dynamic.

[janicduplessis]

@ide Yes, right now it is Android only, I'll try to think of a good API when I get some time that works for both. I was thinking something like an EventEmitter to subscribe to height changes, a function to get the current height (it would return 0 if the status bar is currently hidden for example) and a function to get the 'normal' status bar height that would never change (always ~20 on iOS, ~25 on Android).

[satya164]

IMO we should not have it as a constant if it's not a constant on both platforms. I agree about having an event and a function returning a promise for the height.

[janicduplessis]

Yea, it should be something like StatusBar.getCurrentHeight(): Promise<number> and StatusBar.HEIGHT.

[satya164]

@janicduplessis oops, seems I made a typo in my comment. I meant that we shouldn't have a constant. It makes clear that it can change.

[janicduplessis]

I think we should have both, the constant has the advantage of being sync so you don't have to rerender a view after receiving the value of the status bar height. And then we can use something like an event listener to update the value of the height if it does change after.

[satya164]

@janicduplessis It also makes it super easy to use incorrectly. Say someone uses it in SyleSheet.create, and then later the height changes, it means bad UX. And we should try to make it difficult to do bad UX.

The current dimensions APIs fall into similar category. They are synchronous and constants, and super easy to forget to update. We should not repeat the same mistakes.

If the user needs to access it synchronously, he can always store it in a variable. So we're not really limiting anything.

[janicduplessis]

Totally agree but I'm not sure if there is a way to keep the convenience of having the value as a constant while forcing users to make sure they update it properly. Even if it's a promise returning the current value, it can still cause issues when the value changes while the view is displayed so it doesn't 100% fix the issue.

[satya164]

I guess the difference in naming is good enough, STATUSBAR_HEIGHT vs getCurrentHeight , to signify that it's just the current value, and not a constant value. It also means that the function will always give the current value since it can always ask the Native code for current value, as opposed to a constant, which will store an old cached value in memory.

It also allows lazy evaluation, without having to set the constant if nobody is using it. That's how most of the RN modules are defined. They aren't initialised until needed, but that's another topic.

I think the asynchronous value encourages people to store it in the component state which makes it much easier to update, even though they still have to listen to the events manually, as opposed using it in SyleSheet directly.

[ide]

A promise might not be the right API -- as you guys mentioned, the asynchronous aspect is annoying to work with. We could instead offer an API like this:

// A read-only property that synchronously provides the last-known height of the status bar
StatusBar.currentHeight
// Maybe we want to provide a richer API:
StatusBar.currentSize.{width, height}

// Get notified when the status bar height changes
StatusBar.addListener('change', ({
  size: { width, height },
  position: { x, y },
  orientation: StatusBar.Orientation.{ LANDSCAPE, PORTRAIT },
  animationStyle: StatusBar.AnimationStyle.{ FADE, SLIDE }, // 
  animationDuration: number,
}) => { ... });

As with this diff, we'd send the initial height of the status bar from native to JS via the StatusBar module's constants, and StatusBar.js would subscribe to changes in the status bar and constantly update the properties like StatusBar.currentHeight -- this should have little perf overhead because the status bar doesn't change that much over the course of an app's lifetime.

[satya164]

Yeah, currentHeight is a better name instead of just HEIGHT as it communicates the fact that it's just the current height.

I think @foghina had some suggestions about the Dimensions API, so ccing him.

[ide]

Good call about the Dimensions API -- it's not too different from the status bar in a lot of ways. [On that note, we should separate the notions of "physical screen dimensions" (actual device size) from "logical screen dimensions" (device size minus the software nav bar on Android) from "root size" (size of the root React view, which could be any size).]

All of these may change over the course of an app's lifetime, especially with multi-screen devices or multi-window iPads, and having a consistent API across all of these would be pretty sweet.

[janicduplessis]

@ide I like the API you proposed, I try to get a prototype implementation going soonish :)

[facebook-github-bot]

@janicduplessis updated the pull request.

[facebook-github-bot]

@janicduplessis updated the pull request.

[fungilation]

After a year, still no update on how to deal with in-call status bar height changes on iOS? That should be a fairly common use case that needs to be addressed, and right now I don't see a clear way to detect and catch status bar height change on iOS.

I looked at https://github.com/jgkim/react-native-status-bar-size which hasn't been updated in a year and that relies on StatusBarIOS, which seems to be deprecated in RN now? A pointer on what's current best practice in dealing with dynamic status bar on iOS would be appreciated, if this is a NOFIX.

[dantman]

Something else to consider, Android N's Multi-Window/Split-Screen mode may also change how the status bar is handled.
https://developer.android.com/guide/topics/ui/multi-window.html