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
This commit is contained in:
Jeff DeCew
2021-05-10 17:43:28 -04:00
parent 0ffd1564d1
commit df7223b8e0
7 changed files with 22 additions and 101 deletions

View File

@@ -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"/>

View File

@@ -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"/>

View File

@@ -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>

View File

@@ -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>

View File

@@ -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>

View File

@@ -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;

View File

@@ -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();