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

Add statusBarBackgroundColor to DrawerLayoutAndroid to allow it to open over the status bar #6218

Closed
wants to merge 3 commits into from

Conversation

janicduplessis
Copy link
Contributor

The navigation drawer of most apps on android opens over the status bar, this adds an option to do so. It implements a similar API to the native DrawerLayout by adding a statusBarBackgroundColor to DrawerLayoutAndroid.

Without statusBarBackgroundColor:
image

With statusBarBackgroundColor:
image

This PR depends on the changes in #6195 to add the StatusBar.HEIGHT constant I just want to put it out there now to see if this looks good. To test without the other PR just change StatusBar.HEIGHT for 25.

It is implemented by making the native status bar translucent and making its background color transparent so we can draw a view of the same height as the status bar under it as a child of the DrawerLayoutAndroid. Then we can draw a semi-transparent gray View inside the drawer view to make it look like the drawer is inside the status bar.

There was just one issue that needed some less obvious code, we need to update the StatusBar backgroundColor prop one frame after rendering the background view of the drawer otherwise there was a white flicker when killing the app and reopening it.

Test plan
Tested in the UIExplorer app using the main app drawer. The drawer should open over the status bar and the status bar should not flicker at any time.

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @ericvicenti, @mkonicek and @janicduplessis to be potential reviewers.

@facebook-github-bot 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 Mar 1, 2016
@mkonicek
Copy link
Contributor

mkonicek commented Mar 4, 2016

Whoa, is this something people do in Android apps?

@janicduplessis
Copy link
Contributor Author

Pretty much every google app does it on Android, it is also supported natively by DrawerLayout, I just did it in js since it was easier/cleaner :)

http://developer.android.com/reference/android/support/v4/widget/DrawerLayout.html#setStatusBarBackground(android.graphics.drawable.Drawable)

screen shot 2016-03-04 at 10 22 01 am

@terrysahaidak
Copy link

It would really cool if it will be merged!

@brentvatne
Copy link
Collaborator

This looks good to me.

@brentvatne
Copy link
Collaborator

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

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

@janicduplessis
Copy link
Contributor Author

@brentvatne We'll need to merge #6195 too for this to work properly otherwise the height constant is not defined (it won't break anything though).

@mkonicek
Copy link
Contributor

mkonicek commented Mar 8, 2016

OK, removing the accepted label to remember not to merge this one until #6195 lands. I try to re-ship all PRs that have the accepted label and failed to merge.

@facebook-github-bot
Copy link
Contributor

@janicduplessis updated the pull request.

@facebook-github-bot
Copy link
Contributor

@janicduplessis updated the pull request.

@mkonicek
Copy link
Contributor

Looks like this is still waiting for #6195, commented there.

@facebook-github-bot
Copy link
Contributor

@janicduplessis updated the pull request.

@janicduplessis
Copy link
Contributor Author

Rebased on master to make sure all tests are good.

@janicduplessis
Copy link
Contributor Author

@mkonicek #6195 landed and CI is green :)

@facebook-github-bot
Copy link
Contributor

@janicduplessis updated the pull request.

@mkonicek
Copy link
Contributor

Thanks @janicduplessis, just asked @foghina to take a look too and he says it's good to go! 👍

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

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

@ghost ghost closed this in 039924e Mar 30, 2016
zebulgar pushed a commit to nightingale/react-native that referenced this pull request Jun 18, 2016
…en over the status bar

Summary:The navigation drawer of most apps on android opens over the status bar, this adds an option to do so. It implements a similar API to the native DrawerLayout by adding a statusBarBackgroundColor to DrawerLayoutAndroid.

Without statusBarBackgroundColor:
![image](https://cloud.githubusercontent.com/assets/2677334/13414490/50ebcdf4-df21-11e5-974f-c6a1343c2a4e.png)

With statusBarBackgroundColor:
![image](https://cloud.githubusercontent.com/assets/2677334/13414459/1fdc4086-df21-11e5-9658-bd47bfdb925f.png)

This PR depends on the changes in facebook#6195 to add the `StatusBar.HEIGHT` constant I just want to put it out there now to see if this looks good. To test without the other PR just change `StatusBar.HEIGHT` for `25`.

It is implemented by making the native status bar translucent and making its background color transparent so we can draw a view of the same height as the status bar under it as a child of the DrawerLayoutAndroid. Then we can draw a semi-transparent gray View inside the drawer view to make it
Closes facebook#6218

Differential Revision: D3017444

Pulled By: bestander

fb-gh-sync-id: ca48a47a20a2feecae360a76f3e2c9bbe6a37700
fbshipit-source-id: ca48a47a20a2feecae360a76f3e2c9bbe6a37700
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants