From 5dae211842a3c40da922e93728a384e74bd6acaa Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Wed, 28 Jul 2021 15:30:40 -0400 Subject: [PATCH] Fix bug removing semantic colors of CallStyle notification actions. * When we changed the default color of emphasized actions to an accent color instead of the notification background color, I didn't split the two into different locals, and this meant the accent color was used to enforce the color contrast of the custom color. This led to color desaturation in an attempt to meet contrast (when in dark mode) and a failure to ensure sufficient contrast (in light mode). * A piece of this that applies specifically to colorized notifications is that we used the 'night mode' state as a shortcut for whether the background was light or dark. This led to 1) different foreground colors for the same background color, depending on dark mode state, and 2) frequently a failure to meet contrast (and/or extreme desaturation) in one of those modes. * Also adding different night-mode colors for CallStyle actions, to meet spec. Test: validated sufficient contrast of default and various colorized background/button combos using the accessibility scanner. Bug: 194935539 Change-Id: I3a69eadc89f1d9d98a816c44a36873c7615a177e --- core/java/android/app/Notification.java | 34 ++++++++++++++++++------- core/res/res/values-night/colors.xml | 3 +++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index 9d149cf340e29..4b054f49d9104 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -6122,28 +6122,29 @@ public class Notification implements Parcelable // change the background bgColor CharSequence title = action.title; ColorStateList[] outResultColor = new ColorStateList[1]; - int background = getColors(p).getSecondaryAccentColor(); + int buttonFillColor = getColors(p).getSecondaryAccentColor(); if (isLegacy()) { title = ContrastColorUtil.clearColorSpans(title); } else { - title = ensureColorSpanContrast(title, background, outResultColor); + int notifBackgroundColor = getColors(p).getBackgroundColor(); + title = ensureColorSpanContrast(title, notifBackgroundColor, outResultColor); } button.setTextViewText(R.id.action0, processTextSpans(title)); boolean hasColorOverride = outResultColor[0] != null; if (hasColorOverride) { // There's a span spanning the full text, let's take it and use it as the // background color - background = outResultColor[0].getDefaultColor(); + buttonFillColor = outResultColor[0].getDefaultColor(); } final int textColor = ContrastColorUtil.resolvePrimaryColor(mContext, - background, mInNightMode); + buttonFillColor, mInNightMode); button.setTextColor(R.id.action0, textColor); // We only want about 20% alpha for the ripple final int rippleColor = (textColor & 0x00ffffff) | 0x33000000; button.setColorStateList(R.id.action0, "setRippleColor", ColorStateList.valueOf(rippleColor)); button.setColorStateList(R.id.action0, "setButtonBackground", - ColorStateList.valueOf(background)); + ColorStateList.valueOf(buttonFillColor)); if (p.mCallStyleActions) { button.setImageViewIcon(R.id.action0, action.getIcon()); boolean priority = action.getExtras().getBoolean(CallStyle.KEY_ACTION_PRIORITY); @@ -6176,8 +6177,8 @@ public class Notification implements Parcelable * there exists a full length color span. * @return the contrasted charSequence */ - private CharSequence ensureColorSpanContrast(CharSequence charSequence, int background, - ColorStateList[] outResultColor) { + private static CharSequence ensureColorSpanContrast(CharSequence charSequence, + int background, ColorStateList[] outResultColor) { if (charSequence instanceof Spanned) { Spanned ss = (Spanned) charSequence; Object[] spans = ss.getSpans(0, ss.length(), Object.class); @@ -6197,8 +6198,9 @@ public class Notification implements Parcelable int[] colors = textColor.getColors(); int[] newColors = new int[colors.length]; for (int i = 0; i < newColors.length; i++) { + boolean isBgDark = isColorDark(background); newColors[i] = ContrastColorUtil.ensureLargeTextContrast( - colors[i], background, mInNightMode); + colors[i], background, isBgDark); } textColor = new ColorStateList(textColor.getStates().clone(), newColors); @@ -6217,8 +6219,9 @@ public class Notification implements Parcelable } else if (resultSpan instanceof ForegroundColorSpan) { ForegroundColorSpan originalSpan = (ForegroundColorSpan) resultSpan; int foregroundColor = originalSpan.getForegroundColor(); + boolean isBgDark = isColorDark(background); foregroundColor = ContrastColorUtil.ensureLargeTextContrast( - foregroundColor, background, mInNightMode); + foregroundColor, background, isBgDark); if (fullLength) { outResultColor[0] = ColorStateList.valueOf(foregroundColor); resultSpan = null; @@ -6237,6 +6240,19 @@ public class Notification implements Parcelable return charSequence; } + /** + * Determines if the color is light or dark. Specifically, this is using the same metric as + * {@link ContrastColorUtil#resolvePrimaryColor(Context, int, boolean)} and peers so that + * the direction of color shift is consistent. + * + * @param color the color to check + * @return true if the color has higher contrast with white than black + */ + private static boolean isColorDark(int color) { + // as per ContrastColorUtil.shouldUseDark, this uses the color contrast midpoint. + return ContrastColorUtil.calculateLuminance(color) <= 0.17912878474; + } + /** * @return Whether we are currently building a notification from a legacy (an app that * doesn't create material notifications by itself) app. diff --git a/core/res/res/values-night/colors.xml b/core/res/res/values-night/colors.xml index 816ddd41ccd36..2e4578c394309 100644 --- a/core/res/res/values-night/colors.xml +++ b/core/res/res/values-night/colors.xml @@ -29,5 +29,8 @@ #FFFFFF #FFFFFF + #E66A5E + #5DBA80 + #8AB4F8