GitToTheHub commented on code in PR #1939:
URL: https://github.com/apache/cordova-android/pull/1939#discussion_r3310306762
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -129,12 +127,9 @@ 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 =
parseColorFromString(String.format("#%02X%02X%02X%02X", a, r, g, b));
- overrideStatusBarBackgroundColor = parsedColor;
updateStatusBar(overrideStatusBarBackgroundColor);
Review Comment:
We must check `overrideStatusBarBackgroundColor` if it's not null before
calling `updateStatusBar`, otherwise this would throw a `NullPointerException`
because `updateStatusBar` takes a primitive `int` which cannot be null.
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -129,12 +127,9 @@ 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);
Review Comment:
You can remove the whitespace
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -262,33 +257,33 @@ 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
+ Integer parsedColor = parseColorFromString(colorString);
+ return parsedColor != null ? parsedColor :
getUiModeColor();//getUiModeColor is used as a fallback.
Review Comment:
This can be simplified to:
```java
return Objects.requireNonNullElse(parseColorFromString(colorString),
getUiModeColor());
```
but needs to import `java.util.Objects`:
```java
import java.util.Objects;
```
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -129,12 +127,9 @@ 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 =
parseColorFromString(String.format("#%02X%02X%02X%02X", a, r, g, b));
Review Comment:
You can remove the whitespace
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -262,33 +257,33 @@ 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.
Review Comment:
This comment has to be updated:
```javadoc
/**
* Returns the StatusBarBackgroundColor preference value or {@link
#getUiModeColor()} as fallback.
*
* @return Integer
*/
```
##########
framework/src/org/apache/cordova/SystemBarPlugin.java:
##########
@@ -262,33 +257,33 @@ 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);
Review Comment:
The whitespace can be removed
--
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]
