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.

Sign up for GitHub

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

Jump to bottom

WIP: (android) Remember statusbar color during overlay state changes #172

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP: (android) Remember statusbar color during overlay state changes #172

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions src/android/StatusBar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@

public class StatusBar extends CordovaPlugin {
private static final String TAG = "StatusBar";
private String _bgColor;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO these variables should not begin with an underscore

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My habits leaked into the PR, even though it's not consistent with Apache's codebase 😁

I'll remove the underscores when I come back to this task.

private String _overlayBgColor;

/**
* Sets the context of the Command. This can then be used to do things like
Expand All @@ -54,13 +56,30 @@ public void initialize(final CordovaInterface cordova, CordovaWebView webView) {
this.cordova.getActivity().runOnUiThread(new Runnable() {
@Override
public void run() {

// Clear flag FLAG_FORCE_NOT_FULLSCREEN which is set initially
// by the Cordova.
Window window = cordova.getActivity().getWindow();
window.clearFlags(WindowManager.LayoutParams.FLAG_FORCE_NOT_FULLSCREEN);

// Read 'StatusBarBackgroundColor' from config.xml, default is #000000.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not your code, but this comment is useless and could be removed.

setStatusBarBackgroundColor(preferences.getString("StatusBarBackgroundColor", "#000000"));
_bgColor = preferences.getString("StatusBarBackgroundColor", "#000000");
setStatusBarBackgroundColor(_bgColor);

int color;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be moved to the try block?

try {
color = Color.parseColor(_bgColor);
// If `StatusBarBackgroundColor` contains alpha other than 1, then it
// is reasonable to assume that the color is intended for overlayed statusbar
if (Color.alpha(color) != 1) {
_overlayBgColor = _bgColor;
} else {
_overlayBgColor = "#00000000"; //Transparent
}
} catch (IllegalArgumentException e) {
LOG.w(TAG, "Invalid color " + _bgColor + ". Defaulting to #00000000.");
_overlayBgColor = "#00000000";
}

// Read 'StatusBarStyle' from config.xml, default is 'lightcontent'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above with the comment ;)

setStatusBarStyle(preferences.getString("StatusBarStyle", "lightcontent"));
Expand Down Expand Up @@ -210,6 +229,17 @@ private void setStatusBarBackgroundColor(final String colorPref) {
if (Build.VERSION.SDK_INT >= 21) {
if (colorPref != null && !colorPref.isEmpty()) {
final Window window = cordova.getActivity().getWindow();

// This is so that we can return to the last set statusbar colours if overlay state changes.
int uiOptions = window.getDecorView().getSystemUiVisibility();
boolean isOverlayed = ((uiOptions | View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN) == uiOptions);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to take SYSTEM_UI_FLAG_LAYOUT_STABLE into account, too, or is SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN enough?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that SYSTEM_UI_FLAG_LAYOUT_STABLE will help a lot. In our code, we've delayed the rendering of the map for a harcoded time of 160ms (10 frames at 60fps), just to get around this issue. If we don't add that delay, the map will be resized immediately after it finishes rendering for the first time, and it doesn't look good.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I think that this method should have a different name, It's name is a bit misleading; You can see that it's used for the overlaysWebView action, but it's name doesn't suggest anything on those lines.

        if ("overlaysWebView".equals(action)) {
            // ...
        }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try the SYSTEM_UI_FLAG_LAYOUT_STABLE, however it is now deprecated... along with the other system ui flag constants. But it looks like the replacement is only going to be available in the API 30 (unreleased, under API R), soooo we will have no choice but to continue using them for the next foreseeable future.

In our code, we've delayed the rendering of the map for a harcoded time of 160ms (10 frames at 60fps), just to get around this issue. If we don't add that delay, the map will be resized immediately after it finishes rendering for the first time, and it doesn't look good.

Makes me wonder if this will correct #158 ...

if (isOverlayed) {
_overlayBgColor = colorPref;
} else {
_bgColor = colorPref;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls remove one newline


// Method and constants not available on all SDKs but we want to be able to compile this code with any SDK
window.clearFlags(0x04000000); // SDK 19: WindowManager.LayoutParams.FLAG_TRANSLUCENT_STATUS);
window.addFlags(0x80000000); // SDK 21: WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS);
Expand All @@ -233,12 +263,13 @@ private void setStatusBarTransparent(final boolean transparent) {
window.getDecorView().setSystemUiVisibility(
View.SYSTEM_UI_FLAG_LAYOUT_STABLE
| View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN);
window.setStatusBarColor(Color.TRANSPARENT);
setStatusBarBackgroundColor(_overlayBgColor);
}
else {
window.getDecorView().setSystemUiVisibility(
View.SYSTEM_UI_FLAG_LAYOUT_STABLE
| View.SYSTEM_UI_FLAG_VISIBLE);
setStatusBarBackgroundColor(_bgColor);
}
}
}
Expand Down