Fixed an issue where the media notification wouldn't have contrast
Because we were relying on the output to go in the right direction
already, this could be wrong. We're now only following the given
lightness direction if it is even possible to satisfy contrast.
Test: runtest -x core/tests/coretests/src/android/app/NotificationTest.java
Change-Id: I06d934a6b5c328ebbf0cf707030b0d707ccb5ab4
Fixes: 38182819
diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java
index 4294eab..4a07694 100644
--- a/core/java/android/app/Notification.java
+++ b/core/java/android/app/Notification.java
@@ -16,6 +16,8 @@
package android.app;
+import static com.android.internal.util.NotificationColorUtil.satisfiesTextContrast;
+
import android.annotation.ColorInt;
import android.annotation.DrawableRes;
import android.annotation.IntDef;
@@ -42,7 +44,6 @@
import android.media.session.MediaSession;
import android.net.Uri;
import android.os.BadParcelableException;
-import android.os.Binder;
import android.os.Build;
import android.os.Bundle;
import android.os.IBinder;
@@ -72,6 +73,7 @@
import android.widget.RemoteViews;
import com.android.internal.R;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.NotificationColorUtil;
import com.android.internal.util.Preconditions;
@@ -2670,6 +2672,19 @@
private static final boolean USE_ONLY_TITLE_IN_LOW_PRIORITY_SUMMARY =
SystemProperties.getBoolean("notifications.only_title", true);
+ /**
+ * The lightness difference that has to be added to the primary text color to obtain the
+ * secondary text color when the background is light.
+ */
+ private static final int LIGHTNESS_TEXT_DIFFERENCE_LIGHT = 20;
+
+ /**
+ * The lightness difference that has to be added to the primary text color to obtain the
+ * secondary text color when the background is dark.
+ * A bit less then the above value, since it looks better on dark backgrounds.
+ */
+ private static final int LIGHTNESS_TEXT_DIFFERENCE_DARK = -10;
+
private Context mContext;
private Notification mN;
private Bundle mUserExtras = new Bundle();
@@ -3835,11 +3850,26 @@
contentView.setTextColor(id, mPrimaryTextColor);
}
- private int getPrimaryTextColor() {
+ /**
+ * @return the primary text color
+ * @hide
+ */
+ @VisibleForTesting
+ public int getPrimaryTextColor() {
ensureColors();
return mPrimaryTextColor;
}
+ /**
+ * @return the secondary text color
+ * @hide
+ */
+ @VisibleForTesting
+ public int getSecondaryTextColor() {
+ ensureColors();
+ return mSecondaryTextColor;
+ }
+
private int getActionBarColor() {
ensureColors();
return mActionBarColor;
@@ -3879,16 +3909,21 @@
double textLum = NotificationColorUtil.calculateLuminance(mForegroundColor);
double contrast = NotificationColorUtil.calculateContrast(mForegroundColor,
backgroundColor);
- boolean textDark = backLum > textLum;
+ // We only respect the given colors if worst case Black or White still has
+ // contrast
+ boolean backgroundLight = backLum > textLum
+ && satisfiesTextContrast(backgroundColor, Color.BLACK)
+ || backLum <= textLum
+ && !satisfiesTextContrast(backgroundColor, Color.WHITE);
if (contrast < 4.5f) {
- if (textDark) {
+ if (backgroundLight) {
mSecondaryTextColor = NotificationColorUtil.findContrastColor(
mForegroundColor,
backgroundColor,
true /* findFG */,
4.5f);
mPrimaryTextColor = NotificationColorUtil.changeColorLightness(
- mSecondaryTextColor, -20);
+ mSecondaryTextColor, -LIGHTNESS_TEXT_DIFFERENCE_LIGHT);
} else {
mSecondaryTextColor =
NotificationColorUtil.findContrastColorAgainstDark(
@@ -3897,16 +3932,17 @@
true /* findFG */,
4.5f);
mPrimaryTextColor = NotificationColorUtil.changeColorLightness(
- mSecondaryTextColor, 10);
+ mSecondaryTextColor, -LIGHTNESS_TEXT_DIFFERENCE_DARK);
}
} else {
mPrimaryTextColor = mForegroundColor;
mSecondaryTextColor = NotificationColorUtil.changeColorLightness(
- mPrimaryTextColor, textDark ? 10 : -20);
+ mPrimaryTextColor, backgroundLight ? LIGHTNESS_TEXT_DIFFERENCE_LIGHT
+ : LIGHTNESS_TEXT_DIFFERENCE_DARK);
if (NotificationColorUtil.calculateContrast(mSecondaryTextColor,
backgroundColor) < 4.5f) {
// oh well the secondary is not good enough
- if (textDark) {
+ if (backgroundLight) {
mSecondaryTextColor = NotificationColorUtil.findContrastColor(
mSecondaryTextColor,
backgroundColor,
@@ -3921,7 +3957,9 @@
4.5f);
}
mPrimaryTextColor = NotificationColorUtil.changeColorLightness(
- mSecondaryTextColor, textDark ? -20 : 10);
+ mSecondaryTextColor, backgroundLight
+ ? -LIGHTNESS_TEXT_DIFFERENCE_LIGHT
+ : -LIGHTNESS_TEXT_DIFFERENCE_DARK);
}
}
}
diff --git a/core/java/com/android/internal/util/NotificationColorUtil.java b/core/java/com/android/internal/util/NotificationColorUtil.java
index cd41f9e..1ba92bf 100644
--- a/core/java/com/android/internal/util/NotificationColorUtil.java
+++ b/core/java/com/android/internal/util/NotificationColorUtil.java
@@ -533,6 +533,10 @@
return ColorUtilsFromCompat.calculateContrast(foregroundColor, backgroundColor);
}
+ public static boolean satisfiesTextContrast(int backgroundColor, int foregroundColor) {
+ return NotificationColorUtil.calculateContrast(backgroundColor, foregroundColor) >= 4.5;
+ }
+
/**
* Composite two potentially translucent colors over each other and returns the result.
*/
@@ -540,6 +544,10 @@
return ColorUtilsFromCompat.compositeColors(foreground, background);
}
+ public static boolean isColorLight(int backgroundColor) {
+ return calculateLuminance(backgroundColor) > 0.5f;
+ }
+
/**
* Framework copy of functions needed from android.support.v4.graphics.ColorUtils.
*/
diff --git a/core/tests/coretests/src/android/app/NotificationTest.java b/core/tests/coretests/src/android/app/NotificationTest.java
new file mode 100644
index 0000000..ac1ac19
--- /dev/null
+++ b/core/tests/coretests/src/android/app/NotificationTest.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package android.app;
+
+import static com.android.internal.util.NotificationColorUtil.satisfiesTextContrast;
+
+import static org.junit.Assert.assertTrue;
+
+import android.content.Context;
+import android.media.session.MediaSession;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.filters.SmallTest;
+import android.support.test.runner.AndroidJUnit4;
+
+import com.android.internal.util.NotificationColorUtil;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(AndroidJUnit4.class)
+@SmallTest
+public class NotificationTest {
+
+ private Context mContext;
+
+ @Before
+ public void setUp() {
+ mContext = InstrumentationRegistry.getContext();
+ }
+
+ @Test
+ public void testColorSatisfiedWhenBgDarkTextDarker() {
+ Notification.Builder builder = getMediaNotification();
+ builder.build();
+
+ // An initial guess where the foreground color is actually darker than an already dark bg
+ int backgroundColor = 0xff585868;
+ int initialForegroundColor = 0xff505868;
+ builder.setColorPalette(backgroundColor, initialForegroundColor);
+ int primaryTextColor = builder.getPrimaryTextColor();
+ assertTrue(satisfiesTextContrast(primaryTextColor, backgroundColor));
+ int secondaryTextColor = builder.getSecondaryTextColor();
+ assertTrue(satisfiesTextContrast(secondaryTextColor, backgroundColor));
+ }
+
+ private Notification.Builder getMediaNotification() {
+ MediaSession session = new MediaSession(mContext, "test");
+ return new Notification.Builder(mContext, "color")
+ .setSmallIcon(com.android.internal.R.drawable.emergency_icon)
+ .setContentTitle("Title")
+ .setContentText("Text")
+ .setStyle(new Notification.MediaStyle().setMediaSession(session.getSessionToken()));
+ }
+}
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/MediaNotificationProcessor.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/MediaNotificationProcessor.java
index 52c053f..d0a6fcf 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/MediaNotificationProcessor.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/MediaNotificationProcessor.java
@@ -27,6 +27,7 @@
import android.support.v7.graphics.Palette;
import android.util.LayoutDirection;
+import com.android.internal.util.NotificationColorUtil;
import com.android.systemui.R;
import java.util.List;
@@ -110,7 +111,7 @@
paletteBuilder.addFilter(mBlackWhiteFilter);
palette = paletteBuilder.generate();
int foregroundColor;
- if (ColorUtils.calculateLuminance(backgroundColor) > 0.5) {
+ if (NotificationColorUtil.isColorLight(backgroundColor)) {
Palette.Swatch first = palette.getDarkVibrantSwatch();
Palette.Swatch second = palette.getVibrantSwatch();
if (first != null && second != null) {