CB-14238: Make transparency of statusbar persistent when hiding and showing statusbar #99

aszmyd · 2018-07-18T11:28:26Z

Platforms affected

Android

What does this PR do?

It makes statusbar transparency persisten. Earlier when you call methods in this order: overlaysWebView(true) , hide() , show() the status bar gets shown without transparency and pushes webview to bottom. And because we wanted to have statusbar transparent (overlaysWebView) from this point the show/hide should toggle the statusbar visibility and it should always show transparent.
Support for StatusBarOverlaysWebView in Android when its pre-defined in project config.xml

What testing has been done on this change?

Manual testing on Android device

Checklist

Reported an issue in the JIRA database
Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
Added automated test coverage as appropriate for this change.

janpio · 2018-07-18T12:07:23Z

src/android/StatusBar.java

            final Window window = cordova.getActivity().getWindow();
            if (transparent) {
                window.getDecorView().setSystemUiVisibility(
                        View.SYSTEM_UI_FLAG_LAYOUT_STABLE
                                | View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN);
-                window.setStatusBarColor(Color.TRANSPARENT);
+
+                if (Build.VERSION.SDK_INT >= 21) {


Why did this SDK version check get moved inside here?
Does the other stuff also work on earlier versions?

As far as i've checked setSystemUiVisibility has been added in API 11 https://developer.android.com/reference/android/view/View.html#setSystemUiVisibility(int) so it seems it should work everywhere

janpio · 2018-07-18T12:07:59Z

src/android/StatusBar.java

@@ -227,20 +236,22 @@ private void setStatusBarBackgroundColor(final String colorPref) {
    }

    private void setStatusBarTransparent(final boolean transparent) {
-        if (Build.VERSION.SDK_INT >= 21) {


Code indentation of the code inside this removed if should be adapted to match the added this.transparent = transparent;, right?

Right. Fixes

janpio · 2018-07-18T12:08:42Z

src/android/StatusBar.java

@@ -96,7 +101,11 @@ public void run() {
                    // use KitKat here to be aligned with "Fullscreen"  preference
                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
                        int uiOptions = window.getDecorView().getSystemUiVisibility();
-                        uiOptions &= ~View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN;
+                        if (transparent) {


Shouldn't this be this.transparent?

Good catch. Fixes

aszmyd · 2018-07-23T05:01:01Z

@janpio whats needed to get this merged? I've pushed fixes to the way i accessed transparent from another thread as this.transparent did not work during build.

janpio · 2018-07-23T17:31:10Z

Someone of the committers/maintainers with enough Android experience has to be convinced enough that he hits the merge button. I am not, as I have only superficial knowledge of Android. Enough to leave some comments on code, but not enough to actually be sure the code will be ok in production.

aszmyd · 2018-07-31T09:06:56Z

@janpio do you have anyone specific in mind? Maybe @infil00p can jump in here? I see he merged other Android codes in this repo.

janpio · 2018-07-31T09:25:44Z

Unfortunately not. You could try emailing the Dev mailing list, see https://cordova.apache.org/contact/.

janpio · 2018-07-31T12:42:17Z

@aszmyd Some feedback from other commiters I got when asking for a review of this:

doesn't have JIRA issue 👎
just saying we have hundreds of unattended PRs and I won't give priority to the ones that don't follow the rules

Could you please create said issue and add it to the PR? Open a new one to get the "usual" template for a PR. Thanks.

(If changing the commit message is to involved for you, we can fix that when/if merging)

aszmyd · 2018-08-01T03:47:22Z

@janpio I've created JIRA issue: https://issues.apache.org/jira/browse/CB-14238 and renamed PR name and description. Not sure if i should also rename commit name and force git push? It could confuse others that already pulled this branch.

brodycj · 2018-08-01T03:58:58Z

@aszmyd it should be fine to update by force push (we do this kind of thing all the time). Alternative solution which I think is not necessary is to simply open a new PR. Even fancier is to open a new PR with text like "resolves #99" or "closes #99" in the description (https://help.github.com/articles/closing-issues-using-keywords/).

As an aside you may want to maintain your own special fork version until we get a chance to deal with this one. Maybe something like "cordova-plugin-statusbar-transparent" or "cordova-plugin-persistent-statusbar"?

aszmyd · 2018-08-01T04:06:45Z

@brodybits thanks. I've pushed clean one commit with name changed.

brodycj · 2018-08-01T04:16:33Z

A couple minor points FYI:

First line should generally be within 50 characters. If you need more then you should add a blank line then keep remaining lines within 70 characters. This is very well documented, for example: https://chris.beams.io/posts/git-commit/. AFAICT we generally take care of this formatting issue if needed.
Unless there is good reason to keep multiple original commits we will very likely do a "squash merge", which means that the master branch would actually diverge from your proposed android-persistent-transparency branch.

Thanks for the contribution, I am sure it will help others in the future.

aszmyd · 2018-08-01T04:45:56Z

I do not know why travis build failed for safari browser.

janpio · 2018-08-01T07:51:53Z

I restarted the failed Travis jobs manually.

aszmyd mentioned this pull request Jul 18, 2018

Persistent transparency + getStatusBarHeight method #96

Closed

janpio reviewed Jul 18, 2018

View reviewed changes

aszmyd changed the title ~~Make transparency of statusbar persistent when hiding and showing sta…~~ Make transparency of statusbar persistent when hiding and showing statusbar Aug 1, 2018

aszmyd changed the title ~~Make transparency of statusbar persistent when hiding and showing statusbar~~ CB-14238: Make transparency of statusbar persistent when hiding and showing statusbar Aug 1, 2018

aszmyd force-pushed the android-persistent-transparency branch from 138b0ed to 65a8631 Compare August 1, 2018 04:06

CB-14238: Fixed transparency of statusbar

515d683

aszmyd force-pushed the android-persistent-transparency branch from 65a8631 to 515d683 Compare August 1, 2018 04:24

janpio added the platform: android label Sep 16, 2018

janpio added the bug label Sep 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CB-14238: Make transparency of statusbar persistent when hiding and showing statusbar #99

CB-14238: Make transparency of statusbar persistent when hiding and showing statusbar #99

aszmyd commented Jul 18, 2018 •

edited

Loading

janpio Jul 18, 2018

aszmyd Jul 18, 2018

janpio Jul 18, 2018

aszmyd Jul 18, 2018

janpio Jul 18, 2018

aszmyd Jul 18, 2018

aszmyd commented Jul 23, 2018

janpio commented Jul 23, 2018

aszmyd commented Jul 31, 2018

janpio commented Jul 31, 2018

janpio commented Jul 31, 2018 •

edited

Loading

aszmyd commented Aug 1, 2018

brodycj commented Aug 1, 2018

aszmyd commented Aug 1, 2018

brodycj commented Aug 1, 2018

aszmyd commented Aug 1, 2018

janpio commented Aug 1, 2018

CB-14238: Make transparency of statusbar persistent when hiding and showing statusbar #99

Are you sure you want to change the base?

CB-14238: Make transparency of statusbar persistent when hiding and showing statusbar #99

Conversation

aszmyd commented Jul 18, 2018 • edited Loading

Platforms affected

What does this PR do?

What testing has been done on this change?

Checklist

janpio Jul 18, 2018

Choose a reason for hiding this comment

aszmyd Jul 18, 2018

Choose a reason for hiding this comment

janpio Jul 18, 2018

Choose a reason for hiding this comment

aszmyd Jul 18, 2018

Choose a reason for hiding this comment

janpio Jul 18, 2018

Choose a reason for hiding this comment

aszmyd Jul 18, 2018

Choose a reason for hiding this comment

aszmyd commented Jul 23, 2018

janpio commented Jul 23, 2018

aszmyd commented Jul 31, 2018

janpio commented Jul 31, 2018

janpio commented Jul 31, 2018 • edited Loading

aszmyd commented Aug 1, 2018

brodycj commented Aug 1, 2018

aszmyd commented Aug 1, 2018

brodycj commented Aug 1, 2018

aszmyd commented Aug 1, 2018

janpio commented Aug 1, 2018

aszmyd commented Jul 18, 2018 •

edited

Loading

janpio commented Jul 31, 2018 •

edited

Loading