GitToTheHub commented on code in PR #1939:
URL: https://github.com/apache/cordova-android/pull/1939#discussion_r3303369312
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -129,13 +127,10 @@ private void setStatusBarBackgroundColor(JSONArray
argbVals) {
int r = argbVals.getInt(1);
int g = argbVals.getInt(2);
int b = argbVals.getInt(3);
- String hexColor = String.format("#%02X%02X%02X%02X", a, r, g, b);
- int parsedColor = parseColorFromString(hexColor);
- if (parsedColor == INVALID_COLOR) return;
+ overrideStatusBarBackgroundColor =
String.format("#%02X%02X%02X%02X", a, r, g, b);
Review Comment:
Previously `overrideStatusBarBackgroundColor` did get the parsed color here.
I would change `overrideStatusBarBackgroundColor` to Integer and set the
parsed color directly here, e.g.:
```java
overrideStatusBarBackgroundColor =
parseColorFromString(String.format("#%02X%02X%02X%02X", a, r, g, b));
```
So it's the same behaviour as before and is null if the parsing failed
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -145,30 +140,34 @@ private void setStatusBarBackgroundColor(JSONArray
argbVals) {
* Attempt to update all system bars (status, navigation and gesture bars)
in various points
* of the apps life cycle.
* For example:
- * 1. Device configurations between (E.g. between dark and light mode)
- * 2. User resumes the app
- * 3. App transitions from SplashScreen Theme to App's Theme
+ * 1. Device configurations between (E.g. between dark and light mode)
+ * 2. User resumes the app
+ * 3. App transitions from SplashScreen Theme to App's Theme
*/
private void updateSystemBars() {
// Update Root View Background Color
- int rootViewBackgroundColor = getPreferenceBackgroundColor();
- if (rootViewBackgroundColor == INVALID_COLOR) {
+ Integer rootViewBackgroundColor = getPreferenceBackgroundColor();
+ if (rootViewBackgroundColor == null) {
rootViewBackgroundColor = canEdgeToEdge ? Color.TRANSPARENT :
getUiModeColor();
}
updateRootView(rootViewBackgroundColor);
// Update StatusBar Background Color
- int statusBarBackgroundColor;
- if (overrideStatusBarBackgroundColor != INVALID_COLOR) {
- statusBarBackgroundColor = overrideStatusBarBackgroundColor;
+ Integer statusBarBackgroundColor;
+ if (overrideStatusBarBackgroundColor != null) {
+ statusBarBackgroundColor =
parseColorFromString(overrideStatusBarBackgroundColor);
} else if (preferences.contains("StatusBarBackgroundColor")) {
statusBarBackgroundColor = getPreferenceStatusBarBackgroundColor();
- } else if(preferences.contains("BackgroundColor")){
- statusBarBackgroundColor = rootViewBackgroundColor;
+ } else if (preferences.contains("BackgroundColor")) {
+ statusBarBackgroundColor = rootViewBackgroundColor;
} else {
statusBarBackgroundColor = canEdgeToEdge ? Color.TRANSPARENT :
getUiModeColor();
}
+ if (statusBarBackgroundColor == null) {
+ statusBarBackgroundColor = canEdgeToEdge ? Color.TRANSPARENT :
getUiModeColor();
+ }
+
Review Comment:
When `overrideStatusBarBackgroundColor` is an `Integer` this can be removed.
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -262,33 +261,32 @@ private static boolean isColorLight(int color) {
/**
* Returns the StatusBarBackgroundColor preference value.
- * If the value is missing or fails to parse, it will attempt to try to
guess the background
- * color by extracting from the apps R.color.cdv_background_color or
determine from the uiModes.
- * If all fails, the color normally used in light mode is returned.
+ * If the value is missing or fails to parse, null is returned.
*
- * @return int
+ * @return Integer|null
*/
- private int getPreferenceStatusBarBackgroundColor() {
+ private Integer getPreferenceStatusBarBackgroundColor() {
String colorString = preferences.getString("StatusBarBackgroundColor",
null);
- int parsedColor = parseColorFromString(colorString);
- if (parsedColor != INVALID_COLOR) return parsedColor;
-
- return getUiModeColor(); // fallback
Review Comment:
Why did you removed the fallback `getUiModeColor()`?
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -129,13 +127,10 @@ private void setStatusBarBackgroundColor(JSONArray
argbVals) {
int r = argbVals.getInt(1);
int g = argbVals.getInt(2);
int b = argbVals.getInt(3);
- String hexColor = String.format("#%02X%02X%02X%02X", a, r, g, b);
- int parsedColor = parseColorFromString(hexColor);
- if (parsedColor == INVALID_COLOR) return;
+ overrideStatusBarBackgroundColor =
String.format("#%02X%02X%02X%02X", a, r, g, b);
- overrideStatusBarBackgroundColor = parsedColor;
- updateStatusBar(overrideStatusBarBackgroundColor);
+ updateSystemBars();
Review Comment:
When you change `overrideStatusBarBackgroundColor` to an `Integer`, you can
call `updateStatusBar(overrideStatusBarBackgroundColor)` again instead of
`updateSystemBars()`. Or is there any other reason to call `updateSystemBars()`
instead of `updateStatusBar()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
