-
Notifications
You must be signed in to change notification settings - Fork 481
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
WIP: (android) Remember statusbar color during overlay state changes #172
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got some minor change requests. :)
@@ -38,6 +38,8 @@ | |||
|
|||
public class StatusBar extends CordovaPlugin { | |||
private static final String TAG = "StatusBar"; | |||
private String _bgColor; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
@@ -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. |
There was a problem hiding this comment.
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.
_bgColor = preferences.getString("StatusBarBackgroundColor", "#000000"); | ||
setStatusBarBackgroundColor(_bgColor); | ||
|
||
int color; |
There was a problem hiding this comment.
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?
} catch (IllegalArgumentException e) { | ||
LOG.w(TAG, "Invalid color " + _bgColor + ". Defaulting to #00000000."); | ||
_overlayBgColor = "#00000000"; | ||
} | ||
|
||
// Read 'StatusBarStyle' from config.xml, default is 'lightcontent'. |
There was a problem hiding this comment.
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 ;)
} else { | ||
_bgColor = colorPref; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls remove one newline
|
||
// 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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)) {
// ...
}
There was a problem hiding this comment.
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 ...
Ping @breautek any chance you want to "re-visit" this PR? :) |
Want to... yes Will have I the time to... that might be a different story... Is this potentially blocking something? |
Nope, I was just going through open issues and PRs and Niklas added the 3.0 milestone some time ago. |
Platforms affected
Android
Motivation and Context
Statusbar would override background color to set transparent on
overlaysWebView(true)
and thereforeoverlaysWebView(false)
will make the statusbar turn black.This PR keeps track of the background color, in both states so that
Fixes #159
Description
Two variables were added to keep the background color in memory, one for the solid statusbar and another for overlayed statusbar.
Calling on
backgroundColorByHexString
will determine if the statusbar is overlayed and will update the respective variables.setStatusBarTransparent
has been changed so that it now doesn't hard code the statusbar colour and instead will use the variables to change the statusbar color.By default, the overlay background color defaults to
#00000000
(in otherwords, fully transparent). This makes it so, calling onoverlaysWebview(true)
will hide the statusbar, as it always did before. The overlay background color can be set by supplying a #AARRGGBB hex format toStatusBarBackgroundColor
preference.Testing
npm test
and manual testing.Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)