apache / cordova-plugin-statusbar

Apache Cordova Status Bar Plugin
https://cordova.apache.org/
Apache License 2.0
618 stars 480 forks source link

refactor(android): simplify window & activity #249

Closed erisu closed 1 year ago

erisu commented 1 year ago

Platforms affected

Android

Motivation and Context

Declare class properties activity and window to shorten lines and remove in method defines.

Description

Created class properties and defined them plugin initialize.

Testing

build with AS

Checklist

erisu commented 1 year ago

I think this PR should be the first one instead of the last one, since on the PR that changes the if to a switch is already using activity instead of this.cordova.getActivity().

I did forget to revert all changes from the switch case PR, but I had defined above the switch case statement this:

final Activity activity = this.cordova.getActivity();
final Window window = activity.getWindow();

This was a bit of a step towards the final goal of this PR.

Since I refactored the PRs for readability and remove stacking of PRs, this ended up becoming last.

Also, I wouldn't remove the array just yet, because we are going to need a new dark theme or system theme or similar, at the moment we have a PR to add dark theme, but not sure if it's the correct approach. The problem is we use default = dark and light = light. But on iOS default = match the system theme (switches automatically based on the user theme) So we should probably add a dark theme, a "system" theme or similar naming and deprecate default (in this case we will still need the array since it would do the same as dark mode) Or add dark theme and change the default behavior back to the native "default" value.

I think we can add back the array if necessary.

IMO, clean up when possible is good, even if part of it is readded. I am unaware of the progress of the development of that feature or ETA. Since it might take time, and maybe even the development becomes stale, I thought it would be better to remove clutter when possible.

Also based on the comment that the default value can be either dark or light, depending on the setting of the system's theme, I think the value default would not be in either array. That means there might be still only one value in each array if we add back STYLE_DARK_CONTENT and treat default as using system setting.

Seems the id condition would be better in that case, avoid looping an array, and remove extra import.

I feel like the code would become something like this:

private void setStatusBarStyle(final String style) {
  if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && !style.isEmpty()) {
    View decorView = window.getDecorView();
    WindowInsetsControllerCompat windowInsetsControllerCompat = WindowCompat.getInsetsController(window, decorView);

    if (style.equals(STYLE_DEFAULT)) {
      windowInsetsControllerCompat.setAppearanceLightStatusBars(getIsNightModeActive());
    } else if (style.equals(STYLE_DARK_CONTENT)) {
      windowInsetsControllerCompat.setAppearanceLightStatusBars(true);
    } else if (style.equals(STYLE_LIGHT_CONTENT)) {
      windowInsetsControllerCompat.setAppearanceLightStatusBars(false);
    } else {
      LOG.e(TAG, "Invalid style, must be either 'default' or 'lightcontent'");
    }
  }
}

private boolean getIsNightModeActive() {
  Configuration configuration = activity.getResources().getConfiguration();

  switch (configuration.uiMode & Configuration.UI_MODE_NIGHT_MASK) {
    case Configuration.UI_MODE_NIGHT_YES:
    return true;
    case Configuration.UI_MODE_NIGHT_NO:
    default:
    return false;
  }
}

resources.configuration.isNightModeActive may also be a better method to use, but it requires minSDK of 30.

Anyways, I do not know the end goal for the style values.

As for this PR, it has been rebased and now focus on the creating and hoisting the class variable for window and activity.

erisu commented 1 year ago

@jcesarmobile sorry, I accidentally click on the request re-review button without knowing that you already approved it 25 minutes ago. I thought it was still in the request-change mode.