From ed36ea6c955c1b352f7b4e53a57a961a9087d685 Mon Sep 17 00:00:00 2001
From: Jeff DeCew <jeffdq@google.com>
Date: Wed, 15 Jun 2022 21:08:58 +0000
Subject: [PATCH] Code cleanup and visual polish for hybrid views

These views (the single line children of a group when it is collapsed) were missing some pretty basic polish pieces.  This CL:
* Removes unnecessary styles (the alternative theme was removed years ago) that also required an extra context allocation.
* Apply the themed text colors to the text views.  Leave colors alone (e.g. use the un-themed defaults) if theme colors cannot be resolved.
* Ensure the "conversation sender name" text view translates vertically with the title and text when it fades out.

Test: manual expansion
Bug: 236252445
Change-Id: Ib578eeffa5c78a0016ba818fa0273daa54bee748
---
 .../hybrid_conversation_notification.xml      |  6 +-
 .../res/layout/hybrid_notification.xml        |  9 +-
 packages/SystemUI/res/values/attrs.xml        |  6 --
 packages/SystemUI/res/values/styles.xml       | 24 -----
 .../HybridConversationNotificationView.java   |  4 +
 .../notification/row/HybridGroupManager.java  | 17 +---
 .../row/HybridNotificationView.java           | 96 +++++++++++++------
 7 files changed, 84 insertions(+), 78 deletions(-)

diff --git a/packages/SystemUI/res/layout/hybrid_conversation_notification.xml b/packages/SystemUI/res/layout/hybrid_conversation_notification.xml
index 43b16618d615c..a313833e2a66c 100644
--- a/packages/SystemUI/res/layout/hybrid_conversation_notification.xml
+++ b/packages/SystemUI/res/layout/hybrid_conversation_notification.xml
@@ -57,7 +57,8 @@
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:singleLine="true"
-        style="?attr/hybridNotificationTextStyle"
+        android:paddingEnd="4dp"
+        style="@*android:style/Widget.DeviceDefault.Notification.Text"
     />
 
     <TextView
@@ -65,6 +66,7 @@
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
         android:singleLine="true"
-        style="?attr/hybridNotificationTextStyle"
+        android:paddingEnd="4dp"
+        style="@*android:style/Widget.DeviceDefault.Notification.Text"
     />
 </com.android.systemui.statusbar.notification.row.HybridConversationNotificationView>
diff --git a/packages/SystemUI/res/layout/hybrid_notification.xml b/packages/SystemUI/res/layout/hybrid_notification.xml
index e8d77511e53cc..9ea7be50adec9 100644
--- a/packages/SystemUI/res/layout/hybrid_notification.xml
+++ b/packages/SystemUI/res/layout/hybrid_notification.xml
@@ -20,19 +20,22 @@
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
     android:gravity="bottom|start"
-    style="?attr/hybridNotificationStyle">
+    android:paddingStart="@*android:dimen/notification_content_margin_start"
+    android:paddingEnd="12dp">
     <TextView
         android:id="@+id/notification_title"
         android:layout_width="wrap_content"
         android:layout_height="wrap_content"
         android:singleLine="true"
-        style="?attr/hybridNotificationTitleStyle"
+        android:textAppearance="@*android:style/TextAppearance.DeviceDefault.Notification.Title"
+        android:paddingEnd="4dp"
     />
     <TextView
         android:id="@+id/notification_text"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
         android:singleLine="true"
-        style="?attr/hybridNotificationTextStyle"
+        android:paddingEnd="4dp"
+        style="@*android:style/Widget.DeviceDefault.Notification.Text"
     />
 </com.android.systemui.statusbar.notification.row.HybridNotificationView>
\ No newline at end of file
diff --git a/packages/SystemUI/res/values/attrs.xml b/packages/SystemUI/res/values/attrs.xml
index 3228c3c9d37fe..beda2a7c50b02 100644
--- a/packages/SystemUI/res/values/attrs.xml
+++ b/packages/SystemUI/res/values/attrs.xml
@@ -108,12 +108,6 @@
         <attr name="android:layout" />
     </declare-styleable>
 
-    <declare-styleable name="HybridNotificationTheme">
-        <attr name="hybridNotificationStyle" format="reference" />
-        <attr name="hybridNotificationTitleStyle" format="reference" />
-        <attr name="hybridNotificationTextStyle" format="reference" />
-    </declare-styleable>
-
     <declare-styleable name="PluginInflateContainer">
         <attr name="viewType" format="string" />
     </declare-styleable>
diff --git a/packages/SystemUI/res/values/styles.xml b/packages/SystemUI/res/values/styles.xml
index 0c25f54734167..5cbfa78256ce9 100644
--- a/packages/SystemUI/res/values/styles.xml
+++ b/packages/SystemUI/res/values/styles.xml
@@ -17,30 +17,6 @@
 <resources xmlns:android="http://schemas.android.com/apk/res/android"
            xmlns:androidprv="http://schemas.android.com/apk/prv/res/android">
 
-    <!-- HybridNotification themes and styles -->
-
-    <style name="HybridNotification">
-        <item name="hybridNotificationStyle">@style/hybrid_notification</item>
-        <item name="hybridNotificationTitleStyle">@style/hybrid_notification_title</item>
-        <item name="hybridNotificationTextStyle">@style/hybrid_notification_text</item>
-    </style>
-
-    <style name="hybrid_notification">
-        <item name="android:paddingStart">@*android:dimen/notification_content_margin_start</item>
-        <item name="android:paddingEnd">12dp</item>
-    </style>
-
-    <style name="hybrid_notification_title">
-        <item name="android:paddingEnd">4dp</item>
-        <item name="android:textAppearance">@*android:style/TextAppearance.DeviceDefault.Notification.Title</item>
-    </style>
-
-    <style name="hybrid_notification_text"
-           parent="@*android:style/Widget.DeviceDefault.Notification.Text">
-        <item name="android:paddingEnd">4dp</item>
-    </style>
-
-
     <style name="TextAppearance.StatusBar.Clock" parent="@*android:style/TextAppearance.StatusBar.Icon">
         <item name="android:textSize">@dimen/status_bar_clock_size</item>
         <item name="android:fontFamily">@*android:string/config_headlineFontFamilyMedium</item>
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridConversationNotificationView.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridConversationNotificationView.java
index c66140822d927..99a24cb3e9ca6 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridConversationNotificationView.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridConversationNotificationView.java
@@ -67,6 +67,7 @@ public class HybridConversationNotificationView extends HybridNotificationView {
         mConversationIconView = requireViewById(com.android.internal.R.id.conversation_icon);
         mConversationFacePile = requireViewById(com.android.internal.R.id.conversation_face_pile);
         mConversationSenderName = requireViewById(R.id.conversation_notification_sender);
+        applyTextColor(mConversationSenderName, mSecondaryTextColor);
         mFacePileSize = getResources()
                 .getDimensionPixelSize(R.dimen.conversation_single_line_face_pile_size);
         mFacePileAvatarSize = getResources()
@@ -75,6 +76,9 @@ public class HybridConversationNotificationView extends HybridNotificationView {
                 .getDimensionPixelSize(R.dimen.conversation_single_line_avatar_size);
         mFacePileProtectionWidth = getResources().getDimensionPixelSize(
                 R.dimen.conversation_single_line_face_pile_protection_width);
+        mTransformationHelper.setCustomTransformation(
+                new FadeOutAndDownWithTitleTransformation(mConversationSenderName),
+                mConversationSenderName.getId());
         mTransformationHelper.addViewTransformingToSimilar(mConversationIconView);
         mTransformationHelper.addTransformedView(mConversationSenderName);
     }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridGroupManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridGroupManager.java
index 56f8e087d64dc..39c845df49186 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridGroupManager.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridGroupManager.java
@@ -22,7 +22,6 @@ import android.content.Context;
 import android.content.res.Resources;
 import android.service.notification.StatusBarNotification;
 import android.util.TypedValue;
-import android.view.ContextThemeWrapper;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -55,10 +54,8 @@ public class HybridGroupManager {
         mOverflowNumberPadding = res.getDimensionPixelSize(R.dimen.group_overflow_number_padding);
     }
 
-    private HybridNotificationView inflateHybridViewWithStyle(int style,
-            View contentView, ViewGroup parent) {
-        LayoutInflater inflater = new ContextThemeWrapper(mContext, style)
-                .getSystemService(LayoutInflater.class);
+    private HybridNotificationView inflateHybridView(View contentView, ViewGroup parent) {
+        LayoutInflater inflater = LayoutInflater.from(mContext);
         int layout = contentView instanceof ConversationLayout
                 ? R.layout.hybrid_conversation_notification
                 : R.layout.hybrid_notification;
@@ -91,16 +88,8 @@ public class HybridGroupManager {
     public HybridNotificationView bindFromNotification(HybridNotificationView reusableView,
             View contentView, StatusBarNotification notification,
             ViewGroup parent) {
-        return bindFromNotificationWithStyle(reusableView, contentView, notification,
-                R.style.HybridNotification, parent);
-    }
-
-    private HybridNotificationView bindFromNotificationWithStyle(
-            HybridNotificationView reusableView, View contentView,
-            StatusBarNotification notification,
-            int style, ViewGroup parent) {
         if (reusableView == null) {
-            reusableView = inflateHybridViewWithStyle(style, contentView, parent);
+            reusableView = inflateHybridView(contentView, parent);
         }
         CharSequence titleText = resolveTitle(notification.getNotification());
         CharSequence contentText = resolveText(notification.getNotification());
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridNotificationView.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridNotificationView.java
index c0d85a6a16efc..fc9d9e8b736c2 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridNotificationView.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/HybridNotificationView.java
@@ -16,13 +16,18 @@
 
 package com.android.systemui.statusbar.notification.row;
 
+import static android.app.Notification.COLOR_INVALID;
+
 import android.annotation.Nullable;
 import android.content.Context;
+import android.content.res.TypedArray;
 import android.text.TextUtils;
 import android.util.AttributeSet;
 import android.view.View;
 import android.widget.TextView;
 
+import androidx.annotation.ColorInt;
+
 import com.android.keyguard.AlphaOptimizedLinearLayout;
 import com.android.systemui.R;
 import com.android.systemui.statusbar.CrossFadeHelper;
@@ -40,6 +45,8 @@ public class HybridNotificationView extends AlphaOptimizedLinearLayout
     protected final ViewTransformationHelper mTransformationHelper = new ViewTransformationHelper();
     protected TextView mTitleView;
     protected TextView mTextView;
+    protected int mPrimaryTextColor = COLOR_INVALID;
+    protected int mSecondaryTextColor = COLOR_INVALID;
 
     public HybridNotificationView(Context context) {
         this(context, null);
@@ -69,42 +76,37 @@ public class HybridNotificationView extends AlphaOptimizedLinearLayout
     @Override
     protected void onFinishInflate() {
         super.onFinishInflate();
+        resolveThemeTextColors();
         mTitleView = findViewById(R.id.notification_title);
         mTextView = findViewById(R.id.notification_text);
+        applyTextColor(mTitleView, mPrimaryTextColor);
+        applyTextColor(mTextView, mSecondaryTextColor);
         mTransformationHelper.setCustomTransformation(
-                new ViewTransformationHelper.CustomTransformation() {
-                    @Override
-                    public boolean transformTo(TransformState ownState, TransformableView notification,
-                            float transformationAmount) {
-                        // We want to transform to the same y location as the title
-                        TransformState otherState = notification.getCurrentState(
-                                TRANSFORMING_VIEW_TITLE);
-                        CrossFadeHelper.fadeOut(mTextView, transformationAmount);
-                        if (otherState != null) {
-                            ownState.transformViewVerticalTo(otherState, transformationAmount);
-                            otherState.recycle();
-                        }
-                        return true;
-                    }
-
-                    @Override
-                    public boolean transformFrom(TransformState ownState,
-                            TransformableView notification, float transformationAmount) {
-                        // We want to transform from the same y location as the title
-                        TransformState otherState = notification.getCurrentState(
-                                TRANSFORMING_VIEW_TITLE);
-                        CrossFadeHelper.fadeIn(mTextView, transformationAmount, true /* remap */);
-                        if (otherState != null) {
-                            ownState.transformViewVerticalFrom(otherState, transformationAmount);
-                            otherState.recycle();
-                        }
-                        return true;
-                    }
-                }, TRANSFORMING_VIEW_TEXT);
+                new FadeOutAndDownWithTitleTransformation(mTextView),
+                TRANSFORMING_VIEW_TEXT);
         mTransformationHelper.addTransformedView(TRANSFORMING_VIEW_TITLE, mTitleView);
         mTransformationHelper.addTransformedView(TRANSFORMING_VIEW_TEXT, mTextView);
     }
 
+    protected void applyTextColor(TextView textView, @ColorInt int textColor) {
+        if (textColor != COLOR_INVALID) {
+            textView.setTextColor(textColor);
+        }
+    }
+
+    private void resolveThemeTextColors() {
+        try (TypedArray ta = mContext.getTheme().obtainStyledAttributes(
+                android.R.style.Theme_DeviceDefault_DayNight, new int[]{
+                        android.R.attr.textColorPrimary,
+                        android.R.attr.textColorSecondary
+                })) {
+            if (ta != null) {
+                mPrimaryTextColor = ta.getColor(0, mPrimaryTextColor);
+                mSecondaryTextColor = ta.getColor(1, mSecondaryTextColor);
+            }
+        }
+    }
+
     public void bind(@Nullable CharSequence title, @Nullable CharSequence text,
             @Nullable View contentView) {
         mTitleView.setText(title);
@@ -152,4 +154,40 @@ public class HybridNotificationView extends AlphaOptimizedLinearLayout
 
     @Override
     public void setNotificationFaded(boolean faded) {}
+
+    protected static class FadeOutAndDownWithTitleTransformation extends
+            ViewTransformationHelper.CustomTransformation {
+
+        private final View mView;
+
+        public FadeOutAndDownWithTitleTransformation(View view) {
+            mView = view;
+        }
+
+        @Override
+        public boolean transformTo(TransformState ownState, TransformableView notification,
+                float transformationAmount) {
+            // We want to transform to the same y location as the title
+            TransformState otherState = notification.getCurrentState(TRANSFORMING_VIEW_TITLE);
+            CrossFadeHelper.fadeOut(mView, transformationAmount);
+            if (otherState != null) {
+                ownState.transformViewVerticalTo(otherState, transformationAmount);
+                otherState.recycle();
+            }
+            return true;
+        }
+
+        @Override
+        public boolean transformFrom(TransformState ownState,
+                TransformableView notification, float transformationAmount) {
+            // We want to transform from the same y location as the title
+            TransformState otherState = notification.getCurrentState(TRANSFORMING_VIEW_TITLE);
+            CrossFadeHelper.fadeIn(mView, transformationAmount, true /* remap */);
+            if (otherState != null) {
+                ownState.transformViewVerticalFrom(otherState, transformationAmount);
+                otherState.recycle();
+            }
+            return true;
+        }
+    }
 }