You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Lazza <gi...@git.apache.org> on 2017/02/26 19:21:53 UTC

[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

GitHub user Lazza opened a pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77

    CB-10879: (android) Enable overlaysWebView on Android API 21+

    ### Platforms affected
    
    Android (API 21+).
    
    ### What does this PR do?
    
    This patch enables devices running Android API 21+ to have the status bar overlaying the WebView, i.e. `StatusBar.overlaysWebView(true)`. It lets any Android version call `StatusBar.overlaysWebView(false)` to disable the overlay, which is actually the default behavior on that platform.
    
    ### What testing has been done on this change?
    
    It has been tested on emulators running several android versions, both before and after API 21.
    
    ### Checklist
    - [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
    - [x] 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.
    - [x] Added automated test coverage as appropriate for this change.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Lazza/cordova-plugin-statusbar patch-1

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-plugin-statusbar/pull/77.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #77
    
----
commit de4c0efa3a763807f9fbbf128ac165d2a43eb538
Author: Andrea Lazzarotto <an...@gmail.com>
Date:   2017-02-26T19:11:07Z

    Enable overlaysWebView on Android API 21+
    
    This patch enables devices running Android API 21+ to have the status bar overlaying the WebView, i.e. `StatusBar.overlaysWebView(true)`. It lets any Android version call `StatusBar.overlaysWebView(false)` to disable the overlay, which is actually the default behavior on that platform.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar issue #77: CB-10879: (android) Enable overlaysWebVi...

Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on the issue:

    https://github.com/apache/cordova-plugin-statusbar/pull/77
  
    FYI I filed an issue for 'removal of reflection' from this plugin in [CB-12732](https://issues.apache.org/jira/browse/CB-12732).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by Lazza <gi...@git.apache.org>.
Github user Lazza commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77#discussion_r103163253
  
    --- Diff: src/android/StatusBar.java ---
    @@ -142,6 +142,23 @@ public void run() {
                 return true;
             }
     
    +        if ("overlaysWebView".equals(action)) {
    +            if (Build.VERSION.SDK_INT >= 21) {
    --- End diff --
    
    This one is used to check the return value of the method, the one on line 186 is used to isolate API 21+ code, so that the compiler handles it correctly and doesn't complain about it when building for older versions of Android.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cordova-plugin-statusbar/pull/77


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar issue #77: CB-10879: (android) Enable overlaysWebVi...

Posted by infil00p <gi...@git.apache.org>.
Github user infil00p commented on the issue:

    https://github.com/apache/cordova-plugin-statusbar/pull/77
  
    BTW: I agree with the original author about reflection not being in Android.  I think that adding reflection for compile-time bugs is one of the worst things ever, like drowning puppies or kicking kittens.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by macdonst <gi...@git.apache.org>.
Github user macdonst commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77#discussion_r103260428
  
    --- Diff: src/android/StatusBar.java ---
    @@ -142,6 +142,23 @@ public void run() {
                 return true;
             }
     
    +        if ("overlaysWebView".equals(action)) {
    +            if (Build.VERSION.SDK_INT >= 21) {
    --- End diff --
    
    I'm just saying you don't need to check it twice.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by macdonst <gi...@git.apache.org>.
Github user macdonst commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77#discussion_r103261864
  
    --- Diff: src/android/StatusBar.java ---
    @@ -164,4 +181,21 @@ private void setStatusBarBackgroundColor(final String colorPref) {
                 }
             }
         }
    +
    +    private void setStatusBarTransparent(final boolean transparent) {
    +        if (Build.VERSION.SDK_INT >= 21) {
    +            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);
    --- End diff --
    
    Yeah, I hate reflection on Android too. It is junk. The reason I suggested following the same code style as `setStatusBarBackgroundColor` is sometimes our users build with older versions of the Android API. I know that's wrong but it's hard to stop them.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by macdonst <gi...@git.apache.org>.
Github user macdonst commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77#discussion_r103127274
  
    --- Diff: src/android/StatusBar.java ---
    @@ -164,4 +181,21 @@ private void setStatusBarBackgroundColor(final String colorPref) {
                 }
             }
         }
    +
    +    private void setStatusBarTransparent(final boolean transparent) {
    +        if (Build.VERSION.SDK_INT >= 21) {
    +            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);
    --- End diff --
    
    This method was only introduced in API level 21 so you should use the same approach as `setStatusBarBackgroundColor` and call it via reflection. In fact you'd be better off refactoring `setStatusBarBackgroundColor` so you don't end up repeating a bunch of code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by Lazza <gi...@git.apache.org>.
Github user Lazza commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77#discussion_r103163815
  
    --- Diff: src/android/StatusBar.java ---
    @@ -164,4 +181,21 @@ private void setStatusBarBackgroundColor(final String colorPref) {
                 }
             }
         }
    +
    +    private void setStatusBarTransparent(final boolean transparent) {
    +        if (Build.VERSION.SDK_INT >= 21) {
    +            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);
    --- End diff --
    
    Reflection is not the approach used by Android Studio in handling any of the platform specific code. In fact, reflection has other drawbacks such as being slow and making debugging harder.
    
    The code uses a version check which is recognized by the standard Android build process and it works fine also on e.g. Android API 19 and other versions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar issue #77: CB-10879: (android) Enable overlaysWebVi...

Posted by Lazza <gi...@git.apache.org>.
Github user Lazza commented on the issue:

    https://github.com/apache/cordova-plugin-statusbar/pull/77
  
    I don't understand... why is the repository empty now?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by macdonst <gi...@git.apache.org>.
Github user macdonst commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77#discussion_r103258589
  
    --- Diff: src/android/StatusBar.java ---
    @@ -142,6 +142,23 @@ public void run() {
                 return true;
             }
     
    +        if ("overlaysWebView".equals(action)) {
    --- End diff --
    
    Sorry, I was reviewing this while doing 5 other things. Ignore this comment.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar issue #77: CB-10879: (android) Enable overlaysWebVi...

Posted by cordova-qa <gi...@git.apache.org>.
Github user cordova-qa commented on the issue:

    https://github.com/apache/cordova-plugin-statusbar/pull/77
  
    Cordova CI Build has completed successfully.
    
    **Commit**     - [Link](https://github.com/apache/cordova-plugin-statusbar/pull/77/commits/de4c0efa3a763807f9fbbf128ac165d2a43eb538)
    **Dashboard** - [Link](http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45/)
    
    | Builder Name  | Console Output | Test Report | Device Logs  |
    |     :---:     |     :---:      |   :---:     |     :---:    |
    | [Windows 10  Store]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=windows-10-store/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=windows-10-store/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=windows-10-store/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=windows-10-store/artifact/) |
    | [iOS 9.3]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=ios-9.3/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=ios-9.3/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=ios-9.3/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=ios-9.3/artifact/) |
    | [iOS 10.0]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=ios-10.0/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=ios-10.0/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=ios-10.0/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=ios-10.0/artifact/) |
    | [Android 4.4]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=android-4.4/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=android-4.4/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=android-4.4/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=android-4.4/artifact/) |
    | [Android 5.1]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=android-5.1/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=android-5.1/console) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=android-5.1/testReport/) | [Link]( http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-statusbar-pr/45//PLATFORM=android-5.1/artifact/) |
     



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by Lazza <gi...@git.apache.org>.
Github user Lazza commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77#discussion_r103162781
  
    --- Diff: src/android/StatusBar.java ---
    @@ -142,6 +142,23 @@ public void run() {
                 return true;
             }
     
    +        if ("overlaysWebView".equals(action)) {
    --- End diff --
    
    I am not sure what you mean here. The method has the same name of the equivalent one already existing for the iOS plug-in. Calling the method from JS works fine and has been tested on multiple Android versions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar issue #77: CB-10879: (android) Enable overlaysWebVi...

Posted by macdonst <gi...@git.apache.org>.
Github user macdonst commented on the issue:

    https://github.com/apache/cordova-plugin-statusbar/pull/77
  
    This PR is good to merge and we should look at removing reflection from the rest of the plugin.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by macdonst <gi...@git.apache.org>.
Github user macdonst commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77#discussion_r103126865
  
    --- Diff: src/android/StatusBar.java ---
    @@ -142,6 +142,23 @@ public void run() {
                 return true;
             }
     
    +        if ("overlaysWebView".equals(action)) {
    --- End diff --
    
    You haven't exposed this method in the Javascript interface.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar issue #77: CB-10879: (android) Enable overlaysWebVi...

Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on the issue:

    https://github.com/apache/cordova-plugin-statusbar/pull/77
  
    There was an infrastructure issue between GitHub's readonly clones and Apache git servers. I _think_ it's resolved now?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-statusbar pull request #77: CB-10879: (android) Enable overla...

Posted by macdonst <gi...@git.apache.org>.
Github user macdonst commented on a diff in the pull request:

    https://github.com/apache/cordova-plugin-statusbar/pull/77#discussion_r103126947
  
    --- Diff: src/android/StatusBar.java ---
    @@ -142,6 +142,23 @@ public void run() {
                 return true;
             }
     
    +        if ("overlaysWebView".equals(action)) {
    +            if (Build.VERSION.SDK_INT >= 21) {
    --- End diff --
    
    This line is redundant with line 186.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org