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

wants to merge 1 commit into from

Conversation

breautek
Copy link
Contributor

Platforms affected

Android

Motivation and Context

Statusbar would override background color to set transparent on overlaysWebView(true) and therefore overlaysWebView(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 on overlaysWebview(true) will hide the statusbar, as it always did before. The overlay background color can be set by supplying a #AARRGGBB hex format to StatusBarBackgroundColor preference.

Testing

npm test and manual testing.

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

timbru31
timbru31 requested changes Dec 11, 2019
View reviewed changes
Copy link
Member

@timbru31 timbru31 left a 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. :)

src/android/StatusBar.java
@@ -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.

src/android/StatusBar.java
@@ -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.

src/android/StatusBar.java
_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?

src/android/StatusBar.java
} 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 ;)

src/android/StatusBar.java
} 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

src/android/StatusBar.java

// 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 ...

@NiklasMerz NiklasMerz added this to the 3.0 milestone Mar 23, 2020
@breautek breautek changed the title (android) Remember statusbar color during overlay state changes WIP: (android) Remember statusbar color during overlay state changes May 11, 2020
@SchnWalter SchnWalter mentioned this pull request May 11, 2020
Open
3 tasks
@timbru31
Copy link
Member

timbru31 commented Jan 8, 2021

Ping @breautek any chance you want to "re-visit" this PR? :)

@breautek
Copy link
Contributor Author

breautek commented Jan 8, 2021

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?

@timbru31
Copy link
Member

timbru31 commented Jan 8, 2021

Nope, I was just going through open issues and PRs and Niklas added the 3.0 milestone some time ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SchnWalter SchnWalter SchnWalter left review comments

@timbru31 timbru31 timbru31 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.

[Android] Statusbar doesn't remember background color when overlay mode changes
4 participants
@breautek @timbru31 @SchnWalter @NiklasMerz