From 94d0be89f008ebe82d59839b8a3f7c7c43b333c1 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Mon, 6 Apr 2020 19:20:47 -0700 Subject: [PATCH] Fixed an issue where the unread count would disappear too fast Especially with images this was leading to really bad transitions since the image would pop over immediately. We now update the counter once the expansion has finished Fixes: 153392205 Test: add conversations with messages with unread count and image, click on it. Change-Id: I2ad2ec6a989a21154e3edbf09aabc6d17ea75bd7 --- .../internal/widget/ConversationLayout.java | 19 +++++----- .../notification/ConversationNotifications.kt | 15 ++++++-- .../collection/NotificationEntry.java | 7 ++++ .../row/ExpandableNotificationRow.java | 36 +++++++++++++++++++ .../row/NotificationContentView.java | 9 +++++ 5 files changed, 75 insertions(+), 11 deletions(-) diff --git a/core/java/com/android/internal/widget/ConversationLayout.java b/core/java/com/android/internal/widget/ConversationLayout.java index 5e72fa07242e3..26684201019c4 100644 --- a/core/java/com/android/internal/widget/ConversationLayout.java +++ b/core/java/com/android/internal/widget/ConversationLayout.java @@ -386,14 +386,17 @@ public class ConversationLayout extends FrameLayout /** @hide */ public void setUnreadCount(int unreadCount) { - mUnreadBadge.setVisibility(mIsCollapsed && unreadCount > 1 ? VISIBLE : GONE); - CharSequence text = unreadCount >= 100 - ? getResources().getString(R.string.unread_convo_overflow, 99) - : String.format(Locale.getDefault(), "%d", unreadCount); - mUnreadBadge.setText(text); - mUnreadBadge.setBackgroundTintList(ColorStateList.valueOf(mLayoutColor)); - boolean needDarkText = ColorUtils.calculateLuminance(mLayoutColor) > 0.5f; - mUnreadBadge.setTextColor(needDarkText ? Color.BLACK : Color.WHITE); + boolean visible = mIsCollapsed && unreadCount > 1; + mUnreadBadge.setVisibility(visible ? VISIBLE : GONE); + if (visible) { + CharSequence text = unreadCount >= 100 + ? getResources().getString(R.string.unread_convo_overflow, 99) + : String.format(Locale.getDefault(), "%d", unreadCount); + mUnreadBadge.setText(text); + mUnreadBadge.setBackgroundTintList(ColorStateList.valueOf(mLayoutColor)); + boolean needDarkText = ColorUtils.calculateLuminance(mLayoutColor) > 0.5f; + mUnreadBadge.setTextColor(needDarkText ? Color.BLACK : Color.WHITE); + } } private void addRemoteInputHistoryToMessages( diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/ConversationNotifications.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/ConversationNotifications.kt index 1696f07158654..53ec570903211 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/ConversationNotifications.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/ConversationNotifications.kt @@ -103,12 +103,20 @@ class ConversationNotificationManager @Inject constructor( override fun onEntryInflated(entry: NotificationEntry) { if (!entry.ranking.isConversation) return fun updateCount(isExpanded: Boolean) { - if (isExpanded && !notifPanelCollapsed) { + if (isExpanded && (!notifPanelCollapsed || entry.isPinnedAndExpanded())) { resetCount(entry.key) entry.row?.let(::resetBadgeUi) } } - entry.row?.setOnExpansionChangedListener(::updateCount) + entry.row?.setOnExpansionChangedListener { isExpanded -> + if (entry.row?.isShown == true && isExpanded) { + entry.row.performOnIntrinsicHeightReached { + updateCount(isExpanded) + } + } else { + updateCount(isExpanded) + } + } updateCount(entry.row?.isExpanded == true) } @@ -169,7 +177,8 @@ class ConversationNotificationManager @Inject constructor( private fun resetBadgeUi(row: ExpandableNotificationRow): Unit = (row.layouts?.asSequence() ?: emptySequence()) - .mapNotNull { layout -> layout.contractedChild as? ConversationLayout } + .flatMap { layout -> layout.allViews.asSequence()} + .mapNotNull { view -> view as? ConversationLayout } .forEach { convoLayout -> convoLayout.setUnreadCount(0) } private data class ConversationState(val unreadCount: Int, val notification: Notification) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java index 2618345a5608f..3b71e970c157a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/NotificationEntry.java @@ -610,6 +610,13 @@ public final class NotificationEntry extends ListEntry { return row != null && row.isPinned(); } + /** + * Is this entry pinned and was expanded while doing so + */ + public boolean isPinnedAndExpanded() { + return row != null && row.isPinnedAndExpanded(); + } + public void setRowPinned(boolean pinned) { if (row != null) row.setPinned(pinned); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java index 19b5f5c79ea29..c72b019313222 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java @@ -287,6 +287,12 @@ public class ExpandableNotificationRow extends ActivatableNotificationView if (isPinned()) { nowExpanded = !mExpandedWhenPinned; mExpandedWhenPinned = nowExpanded; + // Also notify any expansion changed listeners. This is necessary since the + // expansion doesn't actually change (it's already system expanded) but it + // changes visually + if (mExpansionChangedListener != null) { + mExpansionChangedListener.onExpansionChanged(nowExpanded); + } } else { nowExpanded = !isExpanded(); setUserExpanded(nowExpanded); @@ -329,6 +335,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView private NotificationInlineImageResolver mImageResolver; private NotificationMediaManager mMediaManager; @Nullable private OnExpansionChangedListener mExpansionChangedListener; + @Nullable private Runnable mOnIntrinsicHeightReachedRunnable; private SystemNotificationAsyncTask mSystemNotificationAsyncTask = new SystemNotificationAsyncTask(); @@ -361,6 +368,16 @@ public class ExpandableNotificationRow extends ActivatableNotificationView return Arrays.copyOf(mLayouts, mLayouts.length); } + /** + * Is this entry pinned and was expanded while doing so + */ + public boolean isPinnedAndExpanded() { + if (!isPinned()) { + return false; + } + return mExpandedWhenPinned; + } + @Override public boolean isGroupExpansionChanging() { if (isChildInGroup()) { @@ -2714,6 +2731,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView if (mMenuRow != null && mMenuRow.getMenuView() != null) { mMenuRow.onParentHeightUpdate(); } + handleIntrinsicHeightReached(); } @Override @@ -2931,6 +2949,24 @@ public class ExpandableNotificationRow extends ActivatableNotificationView mExpansionChangedListener = listener; } + /** + * Perform an action when the notification height has reached its intrinsic height. + * + * @param runnable the runnable to run + */ + public void performOnIntrinsicHeightReached(@Nullable Runnable runnable) { + mOnIntrinsicHeightReachedRunnable = runnable; + handleIntrinsicHeightReached(); + } + + private void handleIntrinsicHeightReached() { + if (mOnIntrinsicHeightReachedRunnable != null + && getActualHeight() == getIntrinsicHeight()) { + mOnIntrinsicHeightReachedRunnable.run(); + mOnIntrinsicHeightReachedRunnable = null; + } + } + @Override public void onInitializeAccessibilityNodeInfoInternal(AccessibilityNodeInfo info) { super.onInitializeAccessibilityNodeInfoInternal(info); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java index 8efdc1b56e8e3..eea4730ea40d0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentView.java @@ -16,6 +16,7 @@ package com.android.systemui.statusbar.notification.row; +import android.annotation.NonNull; import android.annotation.Nullable; import android.app.Notification; import android.app.PendingIntent; @@ -985,6 +986,14 @@ public class NotificationContentView extends FrameLayout { } } + public @NonNull View[] getAllViews() { + return new View[] { + mContractedChild, + mHeadsUpChild, + mExpandedChild, + mSingleLineView }; + } + public NotificationViewWrapper getVisibleWrapper(int visibleType) { switch (visibleType) { case VISIBLE_TYPE_EXPANDED: