From df7223b8e0850b9265987181983dd7be3b39ea80 Mon Sep 17 00:00:00 2001
From: Jeff DeCew <jeffdq@google.com>
Date: Mon, 10 May 2021 17:43:28 -0400
Subject: [PATCH] Simplify SmartReplyView padding layout logic

The horizontal padding used to be dynamic based on the number of lines, but that hasn't been true for a while.  The new goal is for padding to always be 16dp, except that the action icon is in the middle of that.

Fixes: 78876518
Test: manual visual inspection with notify
Test: atest SmartReplyViewTest
Change-Id: I11400a552cab99d9f0ae31024d5973772e8ea300
---
 .../res/layout/smart_action_button.xml        |  2 +
 .../res/layout/smart_reply_button.xml         |  2 +
 .../SystemUI/res/layout/smart_reply_view.xml  |  2 -
 packages/SystemUI/res/values/attrs.xml        |  2 -
 packages/SystemUI/res/values/dimens.xml       |  5 +-
 .../statusbar/policy/SmartReplyView.java      | 87 ++++---------------
 .../statusbar/policy/SmartReplyViewTest.java  | 23 -----
 7 files changed, 22 insertions(+), 101 deletions(-)

diff --git a/packages/SystemUI/res/layout/smart_action_button.xml b/packages/SystemUI/res/layout/smart_action_button.xml
index 271603479482b..488be3a4479e3 100644
--- a/packages/SystemUI/res/layout/smart_action_button.xml
+++ b/packages/SystemUI/res/layout/smart_action_button.xml
@@ -29,6 +29,8 @@
         android:textSize="@dimen/smart_reply_button_font_size"
         android:lineSpacingExtra="@dimen/smart_reply_button_line_spacing_extra"
         android:textColor="@color/smart_reply_button_text"
+        android:paddingLeft="@dimen/smart_reply_button_action_padding_left"
+        android:paddingRight="@dimen/smart_reply_button_padding_horizontal"
         android:drawablePadding="@dimen/smart_action_button_icon_padding"
         android:textStyle="normal"
         android:ellipsize="none"/>
diff --git a/packages/SystemUI/res/layout/smart_reply_button.xml b/packages/SystemUI/res/layout/smart_reply_button.xml
index 9faed18287180..ddf16e0afed77 100644
--- a/packages/SystemUI/res/layout/smart_reply_button.xml
+++ b/packages/SystemUI/res/layout/smart_reply_button.xml
@@ -31,5 +31,7 @@
         android:textSize="@dimen/smart_reply_button_font_size"
         android:lineSpacingExtra="@dimen/smart_reply_button_line_spacing_extra"
         android:textColor="@color/smart_reply_button_text"
+        android:paddingLeft="@dimen/smart_reply_button_padding_horizontal"
+        android:paddingRight="@dimen/smart_reply_button_padding_horizontal"
         android:textStyle="normal"
         android:ellipsize="none"/>
diff --git a/packages/SystemUI/res/layout/smart_reply_view.xml b/packages/SystemUI/res/layout/smart_reply_view.xml
index 9fffc72bcf6db..9d4d1db1ef285 100644
--- a/packages/SystemUI/res/layout/smart_reply_view.xml
+++ b/packages/SystemUI/res/layout/smart_reply_view.xml
@@ -24,8 +24,6 @@
     android:layout_height="wrap_content"
     android:layout_width="wrap_content"
     systemui:spacing="@dimen/smart_reply_button_spacing"
-    systemui:singleLineButtonPaddingHorizontal="@dimen/smart_reply_button_padding_horizontal_single_line"
-    systemui:doubleLineButtonPaddingHorizontal="@dimen/smart_reply_button_padding_horizontal_double_line"
     systemui:buttonStrokeWidth="@dimen/smart_reply_button_stroke_width">
     <!-- smart_reply_button(s) will be added here. -->
 </com.android.systemui.statusbar.policy.SmartReplyView>
diff --git a/packages/SystemUI/res/values/attrs.xml b/packages/SystemUI/res/values/attrs.xml
index eb724425b5d8b..067d56f3d157c 100644
--- a/packages/SystemUI/res/values/attrs.xml
+++ b/packages/SystemUI/res/values/attrs.xml
@@ -131,8 +131,6 @@
 
     <declare-styleable name="SmartReplyView">
         <attr name="spacing" format="dimension" />
-        <attr name="singleLineButtonPaddingHorizontal" format="dimension" />
-        <attr name="doubleLineButtonPaddingHorizontal" format="dimension" />
         <attr name="buttonStrokeWidth" format="dimension" />
     </declare-styleable>
 
diff --git a/packages/SystemUI/res/values/dimens.xml b/packages/SystemUI/res/values/dimens.xml
index 24a3baf07e157..a97eb13fa7076 100644
--- a/packages/SystemUI/res/values/dimens.xml
+++ b/packages/SystemUI/res/values/dimens.xml
@@ -1147,9 +1147,8 @@
     <!-- Smart reply button. Total height 48dp, visible height 32dp. -->
     <dimen name="smart_reply_button_spacing">8dp</dimen>
     <dimen name="smart_reply_button_padding_vertical">14dp</dimen>
-    <!-- Note: The following two paddings need to be different until b/78876518 is fixed. -->
-    <dimen name="smart_reply_button_padding_horizontal_single_line">16dp</dimen>
-    <dimen name="smart_reply_button_padding_horizontal_double_line">15dp</dimen>
+    <dimen name="smart_reply_button_padding_horizontal">16dp</dimen>
+    <dimen name="smart_reply_button_action_padding_left">8dp</dimen>
     <dimen name="smart_reply_button_min_height">48dp</dimen>
     <dimen name="smart_reply_button_stroke_width">1dp</dimen>
     <dimen name="smart_reply_button_font_size">14sp</dimen>
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/SmartReplyView.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/SmartReplyView.java
index 9e1ae8b89fc31..41b1dd12639a4 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/SmartReplyView.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/SmartReplyView.java
@@ -23,6 +23,7 @@ import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
 import android.widget.Button;
+import android.widget.TextView;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.ContrastColorUtil;
@@ -58,15 +59,6 @@ public class SmartReplyView extends ViewGroup {
     /** Spacing to be applied between views. */
     private final int mSpacing;
 
-    /** Horizontal padding of smart reply buttons if all of them use only one line of text. */
-    private final int mSingleLineButtonPaddingHorizontal;
-
-    /** Horizontal padding of smart reply buttons if at least one of them uses two lines of text. */
-    private final int mDoubleLineButtonPaddingHorizontal;
-
-    /** Increase in width of a smart reply button as a result of using two lines instead of one. */
-    private final int mSingleToDoubleLineButtonWidthIncrease;
-
     private final BreakIterator mBreakIterator;
 
     private PriorityQueue<Button> mCandidateButtonQueueForSqueezing;
@@ -114,8 +106,6 @@ public class SmartReplyView extends ViewGroup {
                 mDefaultBackgroundColor);
 
         int spacing = 0;
-        int singleLineButtonPaddingHorizontal = 0;
-        int doubleLineButtonPaddingHorizontal = 0;
         int strokeWidth = 0;
 
         final TypedArray arr = context.obtainStyledAttributes(attrs, R.styleable.SmartReplyView,
@@ -125,10 +115,6 @@ public class SmartReplyView extends ViewGroup {
             int attr = arr.getIndex(i);
             if (attr == R.styleable.SmartReplyView_spacing) {
                 spacing = arr.getDimensionPixelSize(i, 0);
-            } else if (attr == R.styleable.SmartReplyView_singleLineButtonPaddingHorizontal) {
-                singleLineButtonPaddingHorizontal = arr.getDimensionPixelSize(i, 0);
-            } else if (attr == R.styleable.SmartReplyView_doubleLineButtonPaddingHorizontal) {
-                doubleLineButtonPaddingHorizontal = arr.getDimensionPixelSize(i, 0);
             } else if (attr == R.styleable.SmartReplyView_buttonStrokeWidth) {
                 strokeWidth = arr.getDimensionPixelSize(i, 0);
             }
@@ -137,10 +123,6 @@ public class SmartReplyView extends ViewGroup {
 
         mStrokeWidth = strokeWidth;
         mSpacing = spacing;
-        mSingleLineButtonPaddingHorizontal = singleLineButtonPaddingHorizontal;
-        mDoubleLineButtonPaddingHorizontal = doubleLineButtonPaddingHorizontal;
-        mSingleToDoubleLineButtonWidthIncrease =
-                2 * (doubleLineButtonPaddingHorizontal - singleLineButtonPaddingHorizontal);
 
         mBreakIterator = BreakIterator.getLineInstance();
 
@@ -222,6 +204,12 @@ public class SmartReplyView extends ViewGroup {
         return new LayoutParams(params.width, params.height);
     }
 
+    private void clearLayoutLineCount(View view) {
+        if (view instanceof TextView) {
+            ((TextView) view).nullLayouts();
+        }
+    }
+
     @Override
     protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
         final int targetWidth = MeasureSpec.getMode(widthMeasureSpec) == MeasureSpec.UNSPECIFIED
@@ -237,8 +225,7 @@ public class SmartReplyView extends ViewGroup {
 
         SmartSuggestionMeasures accumulatedMeasures = new SmartSuggestionMeasures(
                 mPaddingLeft + mPaddingRight,
-                0 /* maxChildHeight */,
-                mSingleLineButtonPaddingHorizontal);
+                0 /* maxChildHeight */);
         int displayedChildCount = 0;
 
         // Set up a list of suggestions where actions come before replies. Note that the Buttons
@@ -268,8 +255,7 @@ public class SmartReplyView extends ViewGroup {
                 continue;
             }
 
-            child.setPadding(accumulatedMeasures.mButtonPaddingHorizontal, child.getPaddingTop(),
-                    accumulatedMeasures.mButtonPaddingHorizontal, child.getPaddingBottom());
+            clearLayoutLineCount(child);
             child.measure(MEASURE_SPEC_ANY_LENGTH, heightMeasureSpec);
 
             coveredSuggestions.add(child);
@@ -299,18 +285,6 @@ public class SmartReplyView extends ViewGroup {
             accumulatedMeasures.mMaxChildHeight =
                     Math.max(accumulatedMeasures.mMaxChildHeight, childHeight);
 
-            // Do we need to increase the number of lines in smart reply buttons to two?
-            final boolean increaseToTwoLines =
-                    (accumulatedMeasures.mButtonPaddingHorizontal
-                            == mSingleLineButtonPaddingHorizontal)
-                    && (lineCount == 2 || accumulatedMeasures.mMeasuredWidth > targetWidth);
-            if (increaseToTwoLines) {
-                accumulatedMeasures.mMeasuredWidth +=
-                        (displayedChildCount + 1) * mSingleToDoubleLineButtonWidthIncrease;
-                accumulatedMeasures.mButtonPaddingHorizontal =
-                        mDoubleLineButtonPaddingHorizontal;
-            }
-
             // If the last button doesn't fit into the remaining width, try squeezing preceding
             // smart reply buttons.
             if (accumulatedMeasures.mMeasuredWidth > targetWidth) {
@@ -372,8 +346,7 @@ public class SmartReplyView extends ViewGroup {
         mCandidateButtonQueueForSqueezing.clear();
 
         // Finally, we need to re-measure some buttons.
-        remeasureButtonsIfNecessary(accumulatedMeasures.mButtonPaddingHorizontal,
-                                    accumulatedMeasures.mMaxChildHeight);
+        remeasureButtonsIfNecessary(accumulatedMeasures.mMaxChildHeight);
 
         int buttonHeight = Math.max(getSuggestedMinimumHeight(), mPaddingTop
                 + accumulatedMeasures.mMaxChildHeight + mPaddingBottom);
@@ -405,18 +378,14 @@ public class SmartReplyView extends ViewGroup {
     private static class SmartSuggestionMeasures {
         int mMeasuredWidth = -1;
         int mMaxChildHeight = -1;
-        int mButtonPaddingHorizontal = -1;
 
-        SmartSuggestionMeasures(int measuredWidth, int maxChildHeight,
-                int buttonPaddingHorizontal) {
+        SmartSuggestionMeasures(int measuredWidth, int maxChildHeight) {
             this.mMeasuredWidth = measuredWidth;
             this.mMaxChildHeight = maxChildHeight;
-            this.mButtonPaddingHorizontal = buttonPaddingHorizontal;
         }
 
         public SmartSuggestionMeasures clone() {
-            return new SmartSuggestionMeasures(
-                    mMeasuredWidth, mMaxChildHeight, mButtonPaddingHorizontal);
+            return new SmartSuggestionMeasures(mMeasuredWidth, mMaxChildHeight);
         }
     }
 
@@ -547,17 +516,11 @@ public class SmartReplyView extends ViewGroup {
 
     private int squeezeButtonToTextWidth(Button button, int heightMeasureSpec, int textWidth) {
         int oldWidth = button.getMeasuredWidth();
-        if (button.getPaddingLeft() != mDoubleLineButtonPaddingHorizontal) {
-            // Correct for the fact that the button was laid out with single-line horizontal
-            // padding.
-            oldWidth += mSingleToDoubleLineButtonWidthIncrease;
-        }
 
         // Re-measure the squeezed smart reply button.
-        button.setPadding(mDoubleLineButtonPaddingHorizontal, button.getPaddingTop(),
-                mDoubleLineButtonPaddingHorizontal, button.getPaddingBottom());
+        clearLayoutLineCount(button);
         final int widthMeasureSpec = MeasureSpec.makeMeasureSpec(
-                2 * mDoubleLineButtonPaddingHorizontal + textWidth
+                button.getPaddingLeft() + button.getPaddingRight() + textWidth
                       + getLeftCompoundDrawableWidthWithPadding(button), MeasureSpec.AT_MOST);
         button.measure(widthMeasureSpec, heightMeasureSpec);
 
@@ -573,8 +536,7 @@ public class SmartReplyView extends ViewGroup {
         }
     }
 
-    private void remeasureButtonsIfNecessary(
-            int buttonPaddingHorizontal, int maxChildHeight) {
+    private void remeasureButtonsIfNecessary(int maxChildHeight) {
         final int maxChildHeightMeasure =
                 MeasureSpec.makeMeasureSpec(maxChildHeight, MeasureSpec.EXACTLY);
 
@@ -596,24 +558,7 @@ public class SmartReplyView extends ViewGroup {
                 newWidth = Integer.MAX_VALUE;
             }
 
-            // Re-measure reason 2: The button's horizontal padding is incorrect (because it was
-            // measured with the wrong number of lines).
-            if (child.getPaddingLeft() != buttonPaddingHorizontal) {
-                requiresNewMeasure = true;
-                if (newWidth != Integer.MAX_VALUE) {
-                    if (buttonPaddingHorizontal == mSingleLineButtonPaddingHorizontal) {
-                        // Change padding (2->1 line).
-                        newWidth -= mSingleToDoubleLineButtonWidthIncrease;
-                    } else {
-                        // Change padding (1->2 lines).
-                        newWidth += mSingleToDoubleLineButtonWidthIncrease;
-                    }
-                }
-                child.setPadding(buttonPaddingHorizontal, child.getPaddingTop(),
-                        buttonPaddingHorizontal, child.getPaddingBottom());
-            }
-
-            // Re-measure reason 3: The button's height is less than the max height of all buttons
+            // Re-measure reason 2: The button's height is less than the max height of all buttons
             // (all should have the same height).
             if (child.getMeasuredHeight() != maxChildHeight) {
                 requiresNewMeasure = true;
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/SmartReplyViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/SmartReplyViewTest.java
index 3d07eb16ba588..e5b7e3f71ac19 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/SmartReplyViewTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/SmartReplyViewTest.java
@@ -20,7 +20,6 @@ import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertNull;
 import static junit.framework.Assert.assertTrue;
-import static junit.framework.Assert.fail;
 
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.never;
@@ -100,8 +99,6 @@ public class SmartReplyViewTest extends SysuiTestCase {
 
     private Icon mActionIcon;
 
-    private int mSingleLinePaddingHorizontal;
-    private int mDoubleLinePaddingHorizontal;
     private int mSpacing;
 
     private NotificationEntry mEntry;
@@ -141,10 +138,6 @@ public class SmartReplyViewTest extends SysuiTestCase {
         mView = SmartReplyView.inflate(mContext, mConstants);
 
         final Resources res = mContext.getResources();
-        mSingleLinePaddingHorizontal = res.getDimensionPixelSize(
-                R.dimen.smart_reply_button_padding_horizontal_single_line);
-        mDoubleLinePaddingHorizontal = res.getDimensionPixelSize(
-                R.dimen.smart_reply_button_padding_horizontal_double_line);
         mSpacing = res.getDimensionPixelSize(R.dimen.smart_reply_button_spacing);
 
         mNotification = new Notification.Builder(mContext, "")
@@ -588,18 +581,6 @@ public class SmartReplyViewTest extends SysuiTestCase {
         layout.setBaselineAligned(false);
 
         final boolean isRtl = mView.getLayoutDirection() == View.LAYOUT_DIRECTION_RTL;
-        final int paddingHorizontal;
-        switch (lineCount) {
-            case 1:
-                paddingHorizontal = mSingleLinePaddingHorizontal;
-                break;
-            case 2:
-                paddingHorizontal = mDoubleLinePaddingHorizontal;
-                break;
-            default:
-                fail("Invalid line count " + lineCount);
-                return null;
-        }
 
         // Add smart replies
         Button previous = null;
@@ -617,8 +598,6 @@ public class SmartReplyViewTest extends SysuiTestCase {
                                 true /* delayOnClickListener */))
                         .iterator()));
         for (Button current : inflatedReplies) {
-            current.setPadding(paddingHorizontal, current.getPaddingTop(), paddingHorizontal,
-                    current.getPaddingBottom());
             if (previous != null) {
                 ViewGroup.MarginLayoutParams lp =
                         (ViewGroup.MarginLayoutParams) previous.getLayoutParams();
@@ -647,8 +626,6 @@ public class SmartReplyViewTest extends SysuiTestCase {
 
         // Add smart actions
         for (Button current : inflatedSmartActions) {
-            current.setPadding(paddingHorizontal, current.getPaddingTop(), paddingHorizontal,
-                    current.getPaddingBottom());
             if (previous != null) {
                 ViewGroup.MarginLayoutParams lp =
                         (ViewGroup.MarginLayoutParams) previous.getLayoutParams();