From 426998c943245e5362a37c8cd8942b085f3e76db Mon Sep 17 00:00:00 2001 From: Anna Zappone Date: Tue, 13 Apr 2021 21:07:25 +0000 Subject: [PATCH] Fix some UX polish bugs Remove last chatted to for 1 day and under Try to adaptively add more outer padding to 2x1 with a max of 16dp Let small layout match parent height Test: PeopleSpaceUtilsTest Bug: 184606325, 182471248, 185138047 Change-Id: I08a305dacf2b249475ffe0231f0412e694c2922a --- .../res/layout/people_tile_medium_empty.xml | 90 +++++++++--------- .../people_tile_medium_with_content.xml | 2 +- .../SystemUI/res/layout/people_tile_small.xml | 8 +- packages/SystemUI/res/values/dimens.xml | 1 + .../systemui/people/PeopleSpaceUtils.java | 38 -------- .../systemui/people/PeopleTileViewHelper.java | 91 +++++++++++++++---- .../people/PeopleTileViewHelperTest.java | 51 ++++++++--- 7 files changed, 156 insertions(+), 125 deletions(-) diff --git a/packages/SystemUI/res/layout/people_tile_medium_empty.xml b/packages/SystemUI/res/layout/people_tile_medium_empty.xml index 7d9cbb9075067..4236493dce91c 100644 --- a/packages/SystemUI/res/layout/people_tile_medium_empty.xml +++ b/packages/SystemUI/res/layout/people_tile_medium_empty.xml @@ -14,7 +14,7 @@ ~ See the License for the specific language governing permissions and ~ limitations under the License. --> - - + + - - - - - - - - + android:layout_height="wrap_content"> + + - \ No newline at end of file + \ No newline at end of file diff --git a/packages/SystemUI/res/layout/people_tile_medium_with_content.xml b/packages/SystemUI/res/layout/people_tile_medium_with_content.xml index c9e4945a7be2e..70706600e6dab 100644 --- a/packages/SystemUI/res/layout/people_tile_medium_with_content.xml +++ b/packages/SystemUI/res/layout/people_tile_medium_with_content.xml @@ -97,7 +97,7 @@ android:gravity="bottom" android:layout_gravity="center_vertical" android:orientation="horizontal" - android:paddingTop="4dp" + android:paddingTop="2dp" android:layout_width="match_parent" android:layout_height="wrap_content" android:clipToOutline="true"> diff --git a/packages/SystemUI/res/layout/people_tile_small.xml b/packages/SystemUI/res/layout/people_tile_small.xml index 34aa8e4982314..7c28fc10c1849 100644 --- a/packages/SystemUI/res/layout/people_tile_small.xml +++ b/packages/SystemUI/res/layout/people_tile_small.xml @@ -21,7 +21,7 @@ + android:layout_height="22dp" /> 64dp 108dp 14sp + 14sp 24sp 12sp 14sp diff --git a/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceUtils.java b/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceUtils.java index 0fbe1f71a7d72..fbf2f4ded325b 100644 --- a/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceUtils.java +++ b/packages/SystemUI/src/com/android/systemui/people/PeopleSpaceUtils.java @@ -36,9 +36,6 @@ import android.graphics.Bitmap; import android.graphics.Canvas; import android.graphics.drawable.BitmapDrawable; import android.graphics.drawable.Drawable; -import android.icu.text.MeasureFormat; -import android.icu.util.Measure; -import android.icu.util.MeasureUnit; import android.net.Uri; import android.os.Bundle; import android.os.Parcelable; @@ -65,7 +62,6 @@ import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import java.text.SimpleDateFormat; -import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -73,7 +69,6 @@ import java.util.Comparator; import java.util.Date; import java.util.HashSet; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -85,9 +80,6 @@ public class PeopleSpaceUtils { /** Turns on debugging information about People Space. */ public static final boolean DEBUG = true; private static final String TAG = "PeopleSpaceUtils"; - private static final int DAYS_IN_A_WEEK = 7; - private static final int MIN_HOUR = 1; - private static final int ONE_DAY = 1; public static final String PACKAGE_NAME = "package_name"; public static final String USER_ID = "user_id"; public static final String SHORTCUT_ID = "shortcut_id"; @@ -426,36 +418,6 @@ public class PeopleSpaceUtils { return bitmap; } - /** Returns a readable status describing the {@code lastInteraction}. */ - public static String getLastInteractionString(Context context, long lastInteraction) { - if (lastInteraction == 0L) { - Log.e(TAG, "Could not get valid last interaction"); - return context.getString(R.string.basic_status); - } - long now = System.currentTimeMillis(); - Duration durationSinceLastInteraction = Duration.ofMillis(now - lastInteraction); - MeasureFormat formatter = MeasureFormat.getInstance(Locale.getDefault(), - MeasureFormat.FormatWidth.WIDE); - if (durationSinceLastInteraction.toHours() < MIN_HOUR) { - return context.getString(R.string.timestamp, formatter.formatMeasures( - new Measure(durationSinceLastInteraction.toMinutes(), MeasureUnit.MINUTE))); - } else if (durationSinceLastInteraction.toDays() < ONE_DAY) { - return context.getString(R.string.timestamp, formatter.formatMeasures( - new Measure(durationSinceLastInteraction.toHours(), - MeasureUnit.HOUR))); - } else if (durationSinceLastInteraction.toDays() < DAYS_IN_A_WEEK) { - return context.getString(R.string.timestamp, formatter.formatMeasures( - new Measure(durationSinceLastInteraction.toHours(), - MeasureUnit.DAY))); - } else { - return context.getString(durationSinceLastInteraction.toDays() == DAYS_IN_A_WEEK - ? R.string.timestamp : R.string.over_timestamp, - formatter.formatMeasures( - new Measure(durationSinceLastInteraction.toDays() / DAYS_IN_A_WEEK, - MeasureUnit.WEEK))); - } - } - /** * Returns whether the {@code conversation} should be kept for display in the People Space. * diff --git a/packages/SystemUI/src/com/android/systemui/people/PeopleTileViewHelper.java b/packages/SystemUI/src/com/android/systemui/people/PeopleTileViewHelper.java index 51af47d92d7b4..6b917c576fd82 100644 --- a/packages/SystemUI/src/com/android/systemui/people/PeopleTileViewHelper.java +++ b/packages/SystemUI/src/com/android/systemui/people/PeopleTileViewHelper.java @@ -33,7 +33,6 @@ import static android.appwidget.AppWidgetManager.OPTION_APPWIDGET_MIN_WIDTH; import static com.android.systemui.people.PeopleSpaceUtils.convertDrawableToBitmap; import static com.android.systemui.people.PeopleSpaceUtils.getUserId; -import android.annotation.ColorInt; import android.annotation.Nullable; import android.app.PendingIntent; import android.app.people.ConversationStatus; @@ -41,27 +40,28 @@ import android.app.people.PeopleSpaceTile; import android.content.Context; import android.content.Intent; import android.content.res.Configuration; -import android.content.res.TypedArray; import android.graphics.Bitmap; import android.graphics.drawable.Drawable; import android.graphics.drawable.Icon; +import android.icu.text.MeasureFormat; +import android.icu.util.Measure; +import android.icu.util.MeasureUnit; import android.net.Uri; import android.os.Bundle; import android.text.TextUtils; import android.util.IconDrawableFactory; import android.util.Log; -import android.view.ContextThemeWrapper; import android.view.View; import android.widget.RemoteViews; import android.widget.TextView; import com.android.internal.annotations.VisibleForTesting; -import com.android.settingslib.Utils; import com.android.systemui.R; import com.android.systemui.people.widget.LaunchConversationActivity; import com.android.systemui.people.widget.PeopleSpaceWidgetProvider; import java.text.NumberFormat; +import java.time.Duration; import java.util.Arrays; import java.util.List; import java.util.Locale; @@ -76,6 +76,10 @@ class PeopleTileViewHelper { public static final boolean DEBUG = true; private static final String TAG = "PeopleTileView"; + private static final int DAYS_IN_A_WEEK = 7; + private static final int ONE_DAY = 1; + private static final int MAX_WEEKS = 2; + public static final int LAYOUT_SMALL = 0; public static final int LAYOUT_MEDIUM = 1; public static final int LAYOUT_LARGE = 2; @@ -83,7 +87,9 @@ class PeopleTileViewHelper { private static final int MIN_CONTENT_MAX_LINES = 2; private static final int FIXED_HEIGHT_DIMENS_FOR_LARGE_CONTENT = 14 + 12 + 4 + 16; - private static final int FIXED_HEIGHT_DIMENS_FOR_MEDIUM_CONTENT = 8 + 4 + 4 + 8; + private static final int MIN_MEDIUM_VERTICAL_PADDING = 4; + private static final int MAX_MEDIUM_PADDING = 16; + private static final int FIXED_HEIGHT_DIMENS_FOR_MEDIUM_CONTENT_BEFORE_PADDING = 4 + 4; private static final int FIXED_HEIGHT_DIMENS_FOR_SMALL = 6 + 4 + 8; private static final int FIXED_WIDTH_DIMENS_FOR_SMALL = 4 + 4; @@ -96,6 +102,8 @@ class PeopleTileViewHelper { public static final String EMPTY_STRING = ""; + private int mMediumVerticalPadding; + private Context mContext; private PeopleSpaceTile mTile; private float mDensity; @@ -205,7 +213,8 @@ class PeopleTileViewHelper { private int getContentHeightForLayout(int lineHeight) { switch (mLayoutSize) { case LAYOUT_MEDIUM: - return mHeight - (lineHeight + FIXED_HEIGHT_DIMENS_FOR_MEDIUM_CONTENT); + return mHeight - (lineHeight + FIXED_HEIGHT_DIMENS_FOR_MEDIUM_CONTENT_BEFORE_PADDING + + mMediumVerticalPadding * 2); case LAYOUT_LARGE: return mHeight - (getSizeInDp( R.dimen.max_people_avatar_size_for_large_content) + lineHeight @@ -224,7 +233,16 @@ class PeopleTileViewHelper { } // Small layout used below a certain minimum mWidth with any mHeight. if (mWidth >= getSizeInDp(R.dimen.required_width_for_medium)) { - if (DEBUG) Log.d(TAG, "Medium view for mWidth: " + mWidth + " mHeight: " + mHeight); + int spaceAvailableForPadding = + mHeight - (getSizeInDp(R.dimen.avatar_size_for_medium) + 4 + getLineHeight( + getSizeInDp(R.dimen.name_text_size_for_medium))); + if (DEBUG) { + Log.d(TAG, "Medium view for mWidth: " + mWidth + " mHeight: " + mHeight + + " with padding space: " + spaceAvailableForPadding); + } + int maxVerticalPadding = Math.min(Math.floorDiv(spaceAvailableForPadding, 2), + MAX_MEDIUM_PADDING); + mMediumVerticalPadding = Math.max(MIN_MEDIUM_VERTICAL_PADDING, maxVerticalPadding); return LAYOUT_MEDIUM; } // Small layout can always handle our minimum mWidth and mHeight for our widget. @@ -347,11 +365,7 @@ class PeopleTileViewHelper { setMaxLines(views); CharSequence content = mTile.getNotificationContent(); views = setPunctuationRemoteViewsFields(views, content); - // TODO(b/184931139): Update to RemoteViews wrapper to set via attribute once available - @ColorInt int color = Utils.getColorAttr(mContext, - android.R.attr.textColorPrimary).getDefaultColor(); - views.setInt(R.id.text_content, "setTextColor", color); - + views.setColorAttr(R.id.text_content, "setTextColor", android.R.attr.textColorPrimary); views.setTextViewText(R.id.text_content, mTile.getNotificationContent()); views.setViewVisibility(R.id.image, View.GONE); views.setImageViewResource(R.id.predefined_icon, R.drawable.ic_message); @@ -398,9 +412,7 @@ class PeopleTileViewHelper { views.setViewVisibility(R.id.messages_count, View.GONE); setMaxLines(views); // Secondary text color for statuses. - @ColorInt int secondaryColor = Utils.getColorAttr(mContext, - android.R.attr.textColorSecondary).getDefaultColor(); - views.setInt(R.id.text_content, "setTextColor", secondaryColor); + views.setColorAttr(R.id.text_content, "setTextColor", android.R.attr.textColorSecondary); views.setTextViewText(R.id.text_content, statusText); Icon statusIcon = status.getIcon(); @@ -541,6 +553,14 @@ class PeopleTileViewHelper { views.setViewVisibility(R.id.text_content, View.VISIBLE); views.setViewVisibility(R.id.subtext, View.GONE); } + + if (mLayoutSize == LAYOUT_MEDIUM) { + if (DEBUG) Log.d(TAG, "Set vertical padding: " + mMediumVerticalPadding); + int horizontalPadding = (int) Math.floor(MAX_MEDIUM_PADDING * mDensity); + int verticalPadding = (int) Math.floor(mMediumVerticalPadding * mDensity); + views.setViewPadding(R.id.item, horizontalPadding, verticalPadding, horizontalPadding, + verticalPadding); + } return views; } @@ -551,9 +571,16 @@ class PeopleTileViewHelper { views.setViewVisibility(R.id.predefined_icon, View.GONE); views.setViewVisibility(R.id.messages_count, View.GONE); } - String status = PeopleSpaceUtils.getLastInteractionString(mContext, + String status = getLastInteractionString(mContext, mTile.getLastInteractionTimestamp()); - views.setTextViewText(R.id.last_interaction, status); + if (status != null) { + if (DEBUG) Log.d(TAG, "Show last interaction"); + views.setViewVisibility(R.id.last_interaction, View.VISIBLE); + views.setTextViewText(R.id.last_interaction, status); + } else { + if (DEBUG) Log.d(TAG, "Hide last interaction"); + views.setViewVisibility(R.id.last_interaction, View.GONE); + } return views; } @@ -599,4 +626,34 @@ class PeopleTileViewHelper { hasNewStory); return convertDrawableToBitmap(personDrawable); } + + /** Returns a readable status describing the {@code lastInteraction}. */ + @Nullable + public static String getLastInteractionString(Context context, long lastInteraction) { + if (lastInteraction == 0L) { + Log.e(TAG, "Could not get valid last interaction"); + return null; + } + long now = System.currentTimeMillis(); + Duration durationSinceLastInteraction = Duration.ofMillis(now - lastInteraction); + MeasureFormat formatter = MeasureFormat.getInstance(Locale.getDefault(), + MeasureFormat.FormatWidth.WIDE); + if (durationSinceLastInteraction.toDays() <= ONE_DAY) { + return null; + } else if (durationSinceLastInteraction.toDays() < DAYS_IN_A_WEEK) { + return context.getString(R.string.timestamp, formatter.formatMeasures( + new Measure(durationSinceLastInteraction.toHours(), + MeasureUnit.DAY))); + } else if (durationSinceLastInteraction.toDays() <= DAYS_IN_A_WEEK * 2) { + return context.getString(durationSinceLastInteraction.toDays() == DAYS_IN_A_WEEK + ? R.string.timestamp : R.string.over_timestamp, + formatter.formatMeasures( + new Measure(durationSinceLastInteraction.toDays() / DAYS_IN_A_WEEK, + MeasureUnit.WEEK))); + } else { + // Over 2 weeks ago + return context.getString(R.string.over_timestamp, + formatter.formatMeasures(new Measure(MAX_WEEKS, MeasureUnit.WEEK))); + } + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/people/PeopleTileViewHelperTest.java b/packages/SystemUI/tests/src/com/android/systemui/people/PeopleTileViewHelperTest.java index 107ac831811fa..3cc55f2f9070f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/people/PeopleTileViewHelperTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/people/PeopleTileViewHelperTest.java @@ -114,8 +114,6 @@ public class PeopleTileViewHelperTest extends SysuiTestCase { when(mMockContext.getString(R.string.birthday_status)).thenReturn( mContext.getString(R.string.birthday_status)); - when(mMockContext.getString(R.string.basic_status)).thenReturn( - mContext.getString(R.string.basic_status)); when(mMockContext.getPackageManager()).thenReturn(mPackageManager); when(mMockContext.getString(R.string.over_timestamp)).thenReturn( mContext.getString(R.string.over_timestamp)); @@ -126,7 +124,6 @@ public class PeopleTileViewHelperTest extends SysuiTestCase { when(resources.getConfiguration()).thenReturn(configuration); when(resources.getDisplayMetrics()).thenReturn(displayMetrics); TextView textView = mock(TextView.class); - // when(new TextView(mMockContext)).thenReturn(textView); when(textView.getLineHeight()).thenReturn(16); when(mPackageManager.getApplicationIcon(anyString())).thenReturn(null); mPeopleTileViewHelper = new PeopleTileViewHelper(mContext, @@ -134,16 +131,41 @@ public class PeopleTileViewHelperTest extends SysuiTestCase { } @Test - public void testCreateRemoteViewsWithLastInteractionTime() { + public void testCreateRemoteViewsWithLastInteractionTimeUnderOneDayHidden() { RemoteViews views = new PeopleTileViewHelper(mContext, PERSON_TILE_WITHOUT_NOTIFICATION, 0, mOptions).getViews(); View result = views.apply(mContext, null); + // Not showing last interaction. + assertEquals(View.GONE, result.findViewById(R.id.last_interaction).getVisibility()); + + mOptions.putInt(OPTION_APPWIDGET_MIN_WIDTH, + getSizeInDp(R.dimen.required_width_for_large)); + mOptions.putInt(OPTION_APPWIDGET_MIN_WIDTH, + getSizeInDp(R.dimen.required_height_for_large)); + RemoteViews largeView = new PeopleTileViewHelper(mContext, + PERSON_TILE_WITHOUT_NOTIFICATION, 0, mOptions).getViews(); + View largeResult = largeView.apply(mContext, null); + + // Not showing last interaction. + assertEquals(View.GONE, largeResult.findViewById(R.id.last_interaction).getVisibility()); + } + + @Test + public void testCreateRemoteViewsWithLastInteractionTime() { + PeopleSpaceTile tileWithLastInteraction = + PERSON_TILE_WITHOUT_NOTIFICATION.toBuilder().setLastInteractionTimestamp( + 123445L).build(); + RemoteViews views = new PeopleTileViewHelper(mContext, + tileWithLastInteraction, 0, mOptions).getViews(); + View result = views.apply(mContext, null); + TextView name = (TextView) result.findViewById(R.id.name); assertEquals(name.getText(), NAME); // Has last interaction. + assertEquals(View.VISIBLE, result.findViewById(R.id.last_interaction).getVisibility()); TextView lastInteraction = (TextView) result.findViewById(R.id.last_interaction); - assertEquals(lastInteraction.getText(), mContext.getString(R.string.basic_status)); + assertEquals(lastInteraction.getText(), "Over 2 weeks ago"); // No availability. assertEquals(View.GONE, result.findViewById(R.id.availability).getVisibility()); // Shows person icon. @@ -154,7 +176,7 @@ public class PeopleTileViewHelperTest extends SysuiTestCase { mOptions.putInt(OPTION_APPWIDGET_MIN_WIDTH, getSizeInDp(R.dimen.required_width_for_medium) - 1); RemoteViews smallView = new PeopleTileViewHelper(mContext, - PERSON_TILE_WITHOUT_NOTIFICATION, 0, mOptions).getViews(); + tileWithLastInteraction, 0, mOptions).getViews(); View smallResult = smallView.apply(mContext, null); // Show name over predefined icon. @@ -171,14 +193,15 @@ public class PeopleTileViewHelperTest extends SysuiTestCase { mOptions.putInt(OPTION_APPWIDGET_MIN_WIDTH, getSizeInDp(R.dimen.required_height_for_large)); RemoteViews largeView = new PeopleTileViewHelper(mContext, - PERSON_TILE_WITHOUT_NOTIFICATION, 0, mOptions).getViews(); + tileWithLastInteraction, 0, mOptions).getViews(); View largeResult = largeView.apply(mContext, null); name = (TextView) largeResult.findViewById(R.id.name); assertEquals(name.getText(), NAME); // Has last interaction. + assertEquals(View.VISIBLE, largeResult.findViewById(R.id.last_interaction).getVisibility()); lastInteraction = (TextView) result.findViewById(R.id.last_interaction); - assertEquals(lastInteraction.getText(), mContext.getString(R.string.basic_status)); + assertEquals(lastInteraction.getText(), "Over 2 weeks ago"); // No availability. assertEquals(View.GONE, result.findViewById(R.id.availability).getVisibility()); // Shows person icon. @@ -202,8 +225,7 @@ public class PeopleTileViewHelperTest extends SysuiTestCase { TextView name = (TextView) result.findViewById(R.id.name); assertEquals(name.getText(), NAME); // Has last interaction over status. - TextView lastInteraction = (TextView) result.findViewById(R.id.last_interaction); - assertEquals(lastInteraction.getText(), mContext.getString(R.string.basic_status)); + assertEquals(View.GONE, result.findViewById(R.id.last_interaction).getVisibility()); // Has availability. assertEquals(View.VISIBLE, result.findViewById(R.id.availability).getVisibility()); // Has person icon. @@ -237,14 +259,13 @@ public class PeopleTileViewHelperTest extends SysuiTestCase { name = (TextView) largeResult.findViewById(R.id.name); assertEquals(name.getText(), NAME); // Has last interaction. - lastInteraction = (TextView) result.findViewById(R.id.last_interaction); - assertEquals(lastInteraction.getText(), mContext.getString(R.string.basic_status)); + assertEquals(View.GONE, largeResult.findViewById(R.id.last_interaction).getVisibility()); // Has availability. - assertEquals(View.VISIBLE, result.findViewById(R.id.availability).getVisibility()); + assertEquals(View.VISIBLE, largeResult.findViewById(R.id.availability).getVisibility()); // Shows person icon. - assertEquals(View.VISIBLE, result.findViewById(R.id.person_icon).getVisibility()); + assertEquals(View.VISIBLE, largeResult.findViewById(R.id.person_icon).getVisibility()); // No status. - assertThat((View) result.findViewById(R.id.text_content)).isNull(); + assertThat((View) largeResult.findViewById(R.id.text_content)).isNull(); } @Test