Modal Status Bar Translucent #7157

jemise111 · 2016-04-22T14:23:10Z

Currently the Modal component on Android is rendered below the Status Bar, which changes it's color to grey, and in the UIExplorer example the backdrop is just formatted to look the same color. In some scenarios users may want to preserve the color of their status bar and make it look as though the modal is appearing on top. This PR allows for that.

This GIF shows current behavior and new behavior with the translucentStatusBar prop set to true.

I've updated the UIExplorer app to demonstrate and the docs as shown below

Thanks!

facebook-github-bot · 2016-04-22T14:24:29Z

By analyzing the blame information on this pull request, we identified @dmmiller and @gabelevi to be potential reviewers.

dmmiller · 2016-04-22T14:59:45Z

Libraries/Modal/Modal.js

@@ -20,6 +20,8 @@ const View = require('View');
 const requireNativeComponent = require('requireNativeComponent');
 const RCTModalHostView = requireNativeComponent('RCTModalHostView', null);

+const STATUS_BAR_HEIGHT = 24;


Is this fixed on Android?

You can use StatusBar.currentHeight to get the actual height from native.

@janicduplessis thanks for the tip, fixed!

facebook-github-bot · 2016-04-22T19:20:18Z

@jemise111 updated the pull request.

eslint-bot · 2016-04-22T20:13:20Z

Libraries/Modal/Modal.js

@@ -20,6 +20,8 @@ const View = require('View');
 const requireNativeComponent = require('requireNativeComponent');
 const RCTModalHostView = requireNativeComponent('RCTModalHostView', null);

+const STATUS_BAR_HEIGHT = StatusBar.currentHeight;
+


no-undef: 'StatusBar' is not defined.

identifier StatusBar Could not resolve name

facebook-github-bot · 2016-04-22T20:38:13Z

@jemise111 updated the pull request.

janicduplessis · 2016-04-26T07:48:11Z

Should we add a prop for this? Feels like it should always appear on top of the status bar. It would probably still be possible to achieve the old behavior using the StatusBar module and changing the status bar color when the modal is opened.

Just checked out a few apps and dialogs always appear on top of the status bar.

cc @mkonicek @dmmiller

janicduplessis · 2016-04-26T07:54:21Z

Have you tested on older android version that do not support the TRANSLUCENT window flag?

ghost · 2016-04-28T22:27:18Z

@jemise111 updated the pull request.

ghost · 2016-04-28T22:41:15Z

@jemise111 updated the pull request.

jemise111 · 2016-04-28T22:43:48Z

@janicduplessis Fixed so the top of the modal is only pushed down on the appropriate API level on android.

Nothing breaks in the Java code for older versions.

Could you please clarify your other comment? Thanks!

janicduplessis · 2016-04-29T00:10:19Z

I'm wondering if the modal should always render over the status bar (like when you set Translucent Statusbar to true in the example). I think it looks a lot better and is what pretty much every app does.

satya164 · 2016-04-29T07:10:22Z

I think this should be the default behaviour instead of adding a prop.

mkonicek · 2016-04-29T11:39:49Z

Thanks for looking @janicduplessis and @grabbou. Let's make this default.

jemise111 · 2016-04-29T12:11:22Z

@mkonicek great, will do! Think it's necessary to keep a prop for rendering the dialogue the original way? Something like 'persistsStatusBar={true}'?

satya164 · 2016-04-29T13:31:42Z

@jemise111 I don't think anybody really wants that. Do you think people need it?

jemise111 · 2016-04-29T13:44:36Z

@satya164 no, I agree with you. Just wanted to check.

ghost · 2016-05-01T15:40:17Z

@jemise111 updated the pull request.

jemise111 · 2016-05-01T15:42:14Z

@mkonicek done!

eslint-bot · 2016-05-01T15:45:10Z

Libraries/Modal/Modal.js

@@ -91,6 +103,18 @@ class Modal extends React.Component {
  }


eol-last: Newline required at end of file but not found.

janicduplessis · 2016-05-05T04:34:51Z

Libraries/Modal/Modal.js

@@ -91,6 +103,18 @@ class Modal extends React.Component {
  }
 }

+Modal.propTypes = {


Why are these prop types added here? It's already defined in the class as a static member.

…jemise111/react-native into js-modal-status-bar-translucent

jemise111 · 2016-05-05T13:47:45Z

@janicduplessis done!

ghost · 2016-05-05T13:59:12Z

@jemise111 updated the pull request.

dmmiller · 2016-05-05T14:39:07Z

@facebook-github-bot shipit

ghost · 2016-05-05T14:49:34Z

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

dmmiller · 2016-05-06T09:36:17Z

This is failing some internal tests. I am working to get those fixed and have this landed.

As a bonus, this pull request fixed an internal issue that someone posted about yesterday. I told them to try this patch and it solved the issue. That's pretty awesome!

jemise111 · 2016-05-06T10:50:40Z

@dmmiller thanks! Let me know if I can help in any way.

And that's really cool! Glad to hear it.

dmmiller · 2016-05-06T12:00:09Z

I'm working on open sourcing the test that is failing so you can look into it :) And I'll keep looking at it as well.

dmmiller · 2016-05-09T12:54:10Z

@jemise111 So the problem is that by introducing a dependency on StatusBar, we have to update ReactPackage to include StatusBarManager which some of our internal apps don't do. Instead what I've done is expose a StatusBarHeight constant on Modal to use. I should be landing it today with that change and you can see.

jemise111 · 2016-05-09T13:42:55Z

Awesome @dmmiller. Thanks for the update.

Summary: Currently the Modal component on Android is rendered below the Status Bar, which changes it's color to grey, and in the UIExplorer example the backdrop is just formatted to look the same color. In some scenarios users may want to preserve the color of their status bar and make it look as though the modal is appearing on top. This PR allows for that. This GIF shows current behavior and new behavior with the translucentStatusBar prop set to true. ![](http://g.recordit.co/BSX5g9obRC.gif) I've updated the UIExplorer app to demonstrate and the docs as shown below ![image](https://cloud.githubusercontent.com/assets/4265163/14742854/500e1292-086c-11e6-9275-71808b0cbed7.png) Thanks! Closes facebook#7157 Differential Revision: D3264497 Pulled By: dmmiller fb-gh-sync-id: 61346d99414d331d3420f44a4c5f6341b0973be6 fbshipit-source-id: 61346d99414d331d3420f44a4c5f6341b0973be6

patrick-ngo · 2018-02-12T16:57:42Z

Is this behaviour achievable currently?

jemise111 added 3 commits April 22, 2016 10:16

add translucentStatusBar prop to Modal for android

4b2a90f

taps to spaces

e4dfc8a

small cleanup

e607b2d

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Apr 22, 2016

dmmiller reviewed Apr 22, 2016
View reviewed changes

use StatusBar current height instead of fixed

ae66097

eslint-bot reviewed Apr 22, 2016
View reviewed changes

require statusbar

dbe56b1

Merge branch 'master' into js-modal-status-bar-translucent

7c499a7

dont change top value for older api versions

01939a3

jemise111 added 2 commits May 1, 2016 11:37

remove translucentStatusBar prop, make it default behavior

92adea5

fix conflicts with master

6d91f09

eslint-bot reviewed May 1, 2016
View reviewed changes

janicduplessis reviewed May 5, 2016
View reviewed changes

janicduplessis added GH Review: needs-revision and removed GH Review: review-needed labels May 5, 2016

jemise111 added 3 commits May 5, 2016 09:46

remove non static proptypes and defaultprops

2903c7e

Merge branch 'master' into js-modal-status-bar-translucent

0dae882

Merge branch 'js-modal-status-bar-translucent' of https://github.com/…

9ba161e

…jemise111/react-native into js-modal-status-bar-translucent

ghost added GH Review: review-needed and removed GH Review: needs-revision labels May 5, 2016

ghost added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels May 5, 2016

ghost added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels May 6, 2016

ghost closed this in 191d278 May 9, 2016

nihgwu mentioned this pull request Jul 8, 2016

Using Modal in an app with status bar hidden = true results in status bar being shown #8627

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modal Status Bar Translucent #7157

Modal Status Bar Translucent #7157

jemise111 commented Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

dmmiller Apr 22, 2016

janicduplessis Apr 22, 2016

jemise111 Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

eslint-bot Apr 22, 2016

eslint-bot Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

janicduplessis commented Apr 26, 2016 •

edited

Loading

janicduplessis commented Apr 26, 2016

ghost commented Apr 28, 2016

ghost commented Apr 28, 2016

jemise111 commented Apr 28, 2016

janicduplessis commented Apr 29, 2016

satya164 commented Apr 29, 2016

mkonicek commented Apr 29, 2016

jemise111 commented Apr 29, 2016

satya164 commented Apr 29, 2016

jemise111 commented Apr 29, 2016

ghost commented May 1, 2016

jemise111 commented May 1, 2016

eslint-bot May 1, 2016

janicduplessis May 5, 2016

jemise111 commented May 5, 2016

ghost commented May 5, 2016

dmmiller commented May 5, 2016

ghost commented May 5, 2016

dmmiller commented May 6, 2016

jemise111 commented May 6, 2016

dmmiller commented May 6, 2016

dmmiller commented May 9, 2016

jemise111 commented May 9, 2016

patrick-ngo commented Feb 12, 2018

Modal Status Bar Translucent #7157

Modal Status Bar Translucent #7157

Conversation

jemise111 commented Apr 22, 2016

facebook-github-bot commented Apr 22, 2016

dmmiller Apr 22, 2016

Choose a reason for hiding this comment

janicduplessis Apr 22, 2016

Choose a reason for hiding this comment

jemise111 Apr 22, 2016

Choose a reason for hiding this comment

facebook-github-bot commented Apr 22, 2016

eslint-bot Apr 22, 2016

Choose a reason for hiding this comment

eslint-bot Apr 22, 2016

Choose a reason for hiding this comment

facebook-github-bot commented Apr 22, 2016

janicduplessis commented Apr 26, 2016 • edited Loading

janicduplessis commented Apr 26, 2016

ghost commented Apr 28, 2016

ghost commented Apr 28, 2016

jemise111 commented Apr 28, 2016

janicduplessis commented Apr 29, 2016

satya164 commented Apr 29, 2016

mkonicek commented Apr 29, 2016

jemise111 commented Apr 29, 2016

satya164 commented Apr 29, 2016

jemise111 commented Apr 29, 2016

ghost commented May 1, 2016

jemise111 commented May 1, 2016

eslint-bot May 1, 2016

Choose a reason for hiding this comment

janicduplessis May 5, 2016

Choose a reason for hiding this comment

jemise111 commented May 5, 2016

ghost commented May 5, 2016

dmmiller commented May 5, 2016

ghost commented May 5, 2016

dmmiller commented May 6, 2016

jemise111 commented May 6, 2016

dmmiller commented May 6, 2016

dmmiller commented May 9, 2016

jemise111 commented May 9, 2016

patrick-ngo commented Feb 12, 2018

janicduplessis commented Apr 26, 2016 •

edited

Loading