From 68c51fe56605b05aebf528454e164e3f64a8feef Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Fri, 19 Nov 2021 15:26:48 +0000 Subject: [PATCH] Convert from Pair to FeedbackIcon This also pre-constructs the icons to avoid per-notification allocations. Test: atest AssistantFeedbackControllerTest NotificationContentViewTest ExpandableNotificationRowTest Change-Id: I9872461dc0797784ab6352edfeaa5a9a0d458e2d --- .../NotificationViewHierarchyManager.java | 3 +- .../AssistantFeedbackController.java | 47 +++++++------------ .../statusbar/notification/FeedbackIcon.kt | 31 ++++++++++++ .../coordinator/RowAppearanceCoordinator.kt | 5 +- .../collection/render/NotifRowController.kt | 9 ++-- .../row/ExpandableNotificationRow.java | 11 +++-- .../ExpandableNotificationRowController.java | 7 +-- .../row/NotificationContentView.java | 11 +++-- .../row/NotificationGutsManager.java | 7 +-- .../NotificationHeaderViewWrapper.java | 17 ++++--- .../row/wrapper/NotificationViewWrapper.java | 8 ++-- .../stack/NotificationChildrenContainer.java | 14 +++--- .../AssistantFeedbackControllerTest.java | 42 ++++++++++++----- .../RowAppearanceCoordinatorTest.kt | 9 ++-- .../row/ExpandableNotificationRowTest.java | 16 +++---- .../row/NotificationContentViewTest.java | 6 +-- 16 files changed, 138 insertions(+), 105 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/FeedbackIcon.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index 83ef41ea0d1db..8eb27a93dece2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -572,8 +572,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle stack.push(notificationChildren.get(i)); } } - row.showFeedbackIcon(mAssistantFeedbackController.showFeedbackIndicator(entry), - mAssistantFeedbackController.getFeedbackResources(entry)); + row.setFeedbackIcon(mAssistantFeedbackController.getFeedbackIcon(entry)); row.setLastAudiblyAlertedMs(entry.getLastAudiblyAlertedMs()); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/AssistantFeedbackController.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/AssistantFeedbackController.java index 4b4e51383b7e6..420dd3f0e464a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/AssistantFeedbackController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/AssistantFeedbackController.java @@ -24,7 +24,9 @@ import android.app.NotificationManager; import android.content.Context; import android.os.Handler; import android.provider.DeviceConfig; -import android.util.Pair; +import android.util.SparseArray; + +import androidx.annotation.Nullable; import com.android.internal.R; import com.android.systemui.dagger.SysUISingleton; @@ -52,6 +54,8 @@ public class AssistantFeedbackController { public static final int STATUS_PROMOTED = 3; public static final int STATUS_DEMOTED = 4; + private final SparseArray mIcons; + private volatile boolean mFeedbackEnabled; private final DeviceConfig.OnPropertiesChangedListener mPropertiesChangedListener = @@ -76,6 +80,16 @@ public class AssistantFeedbackController { ENABLE_NAS_FEEDBACK, false); mDeviceConfigProxy.addOnPropertiesChangedListener(DeviceConfig.NAMESPACE_SYSTEMUI, this::postToHandler, mPropertiesChangedListener); + // Populate the array of statuses. + mIcons = new SparseArray<>(4); + mIcons.set(STATUS_ALERTED, new FeedbackIcon(R.drawable.ic_feedback_alerted, + R.string.notification_feedback_indicator_alerted)); + mIcons.set(STATUS_SILENCED, new FeedbackIcon(R.drawable.ic_feedback_silenced, + R.string.notification_feedback_indicator_silenced)); + mIcons.set(STATUS_PROMOTED, new FeedbackIcon(R.drawable.ic_feedback_uprank, + R.string.notification_feedback_indicator_promoted)); + mIcons.set(STATUS_DEMOTED, new FeedbackIcon(R.drawable.ic_feedback_downrank, + R.string.notification_feedback_indicator_demoted)); } private void postToHandler(Runnable r) { @@ -119,41 +133,16 @@ public class AssistantFeedbackController { } } - /** - * Determines whether to show feedback indicator. The feedback indicator will be shown - * if {@link #isFeedbackEnabled()} is enabled and assistant has changed this notification's rank - * or importance. - * - * @param entry Notification Entry to show feedback for - */ - public boolean showFeedbackIndicator(NotificationEntry entry) { - return getFeedbackStatus(entry) != STATUS_UNCHANGED; - } - /** * Get the feedback indicator image and content description resources according to assistant's * changes on this notification's rank or importance. * * @param entry Notification Entry to show feedback for */ - public Pair getFeedbackResources(NotificationEntry entry) { + @Nullable + public FeedbackIcon getFeedbackIcon(NotificationEntry entry) { int feedbackStatus = getFeedbackStatus(entry); - switch (feedbackStatus) { - case STATUS_ALERTED: - return new Pair(R.drawable.ic_feedback_alerted, - R.string.notification_feedback_indicator_alerted); - case STATUS_SILENCED: - return new Pair(R.drawable.ic_feedback_silenced, - R.string.notification_feedback_indicator_silenced); - case STATUS_PROMOTED: - return new Pair(R.drawable.ic_feedback_uprank, - R.string.notification_feedback_indicator_promoted); - case STATUS_DEMOTED: - return new Pair(R.drawable.ic_feedback_downrank, - R.string.notification_feedback_indicator_demoted); - default: - return new Pair(0, 0); - } + return mIcons.get(feedbackStatus); } /** diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/FeedbackIcon.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/FeedbackIcon.kt new file mode 100644 index 0000000000000..36b987b6edaf1 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/FeedbackIcon.kt @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.notification + +import android.annotation.DrawableRes +import android.annotation.StringRes + +/** + * The feedback icon to show in the header of a notification. + * The icon consists of a drawable and a content description to set on the ImageView. + */ +data class FeedbackIcon( + /** The drawable resource */ + @DrawableRes val iconRes: Int, + /** The content description string resource */ + @StringRes val contentDescRes: Int +) \ No newline at end of file diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RowAppearanceCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RowAppearanceCoordinator.kt index c8f736027e500..4e9d3ac07a966 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RowAppearanceCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/coordinator/RowAppearanceCoordinator.kt @@ -64,10 +64,7 @@ class RowAppearanceCoordinator @Inject internal constructor( // very first notification and if it's not a child of grouped notifications. controller.setSystemExpanded(mAlwaysExpandNonGroupedNotification || entry == entryToExpand) // Show/hide the feedback icon - controller.showFeedbackIcon( - mAssistantFeedbackController.showFeedbackIndicator(entry), - mAssistantFeedbackController.getFeedbackResources(entry) - ) + controller.setFeedbackIcon(mAssistantFeedbackController.getFeedbackIcon(entry)) // Show the "alerted" bell icon controller.setLastAudiblyAlertedMs(entry.lastAudiblyAlertedMs) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NotifRowController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NotifRowController.kt index c10e4018b5dbf..5ee94ba1624c7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NotifRowController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/NotifRowController.kt @@ -16,7 +16,7 @@ package com.android.systemui.statusbar.notification.collection.render -import android.util.Pair +import com.android.systemui.statusbar.notification.FeedbackIcon /** A view controller for a notification row */ interface NotifRowController { @@ -34,9 +34,6 @@ interface NotifRowController { */ fun setLastAudiblyAlertedMs(lastAudiblyAlertedMs: Long) - /** - * Sets both whether to show a feedback indicator and which resources to use for the drawable - * and content description. - */ - fun showFeedbackIcon(showFeedbackIndicator: Boolean, feedbackResources: Pair?) + /** Shows the given feedback icon, or hides the icon if null. */ + fun setFeedbackIcon(icon: FeedbackIcon?) } 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 1c2b938a0cc59..fb8e189d47895 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 @@ -56,7 +56,6 @@ import android.util.AttributeSet; import android.util.FloatProperty; import android.util.Log; import android.util.MathUtils; -import android.util.Pair; import android.util.Property; import android.view.KeyEvent; import android.view.LayoutInflater; @@ -92,6 +91,7 @@ import com.android.systemui.statusbar.RemoteInputController; import com.android.systemui.statusbar.StatusBarIconView; import com.android.systemui.statusbar.notification.AboveShelfChangedListener; import com.android.systemui.statusbar.notification.ExpandAnimationParameters; +import com.android.systemui.statusbar.notification.FeedbackIcon; import com.android.systemui.statusbar.notification.NotificationFadeAware; import com.android.systemui.statusbar.notification.NotificationLaunchAnimatorController; import com.android.systemui.statusbar.notification.NotificationUtils; @@ -1683,12 +1683,13 @@ public class ExpandableNotificationRow extends ActivatableNotificationView setTargetPoint(null); } - public void showFeedbackIcon(boolean show, Pair resIds) { + /** Shows the given feedback icon, or hides the icon if null. */ + public void setFeedbackIcon(@Nullable FeedbackIcon icon) { if (mIsSummaryWithChildren) { - mChildrenContainer.showFeedbackIcon(show, resIds); + mChildrenContainer.setFeedbackIcon(icon); } - mPrivateLayout.showFeedbackIcon(show, resIds); - mPublicLayout.showFeedbackIcon(show, resIds); + mPrivateLayout.setFeedbackIcon(icon); + mPublicLayout.setFeedbackIcon(icon); } /** Sets the last time the notification being displayed audibly alerted the user. */ diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java index 10e8b140b3e12..e458315292eb7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowController.java @@ -21,11 +21,11 @@ import static com.android.systemui.statusbar.NotificationRemoteInputManager.ENAB import static com.android.systemui.statusbar.StatusBarState.KEYGUARD; import android.util.Log; -import android.util.Pair; import android.view.View; import android.view.ViewGroup; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import com.android.systemui.R; import com.android.systemui.classifier.FalsingCollector; @@ -34,6 +34,7 @@ import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.shared.plugins.PluginManager; import com.android.systemui.statusbar.NotificationMediaManager; +import com.android.systemui.statusbar.notification.FeedbackIcon; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.render.GroupExpansionManager; import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager; @@ -292,7 +293,7 @@ public class ExpandableNotificationRowController implements NotifViewController } @Override - public void showFeedbackIcon(boolean show, Pair feedbackResources) { - mView.showFeedbackIcon(show, feedbackResources); + public void setFeedbackIcon(@Nullable FeedbackIcon icon) { + mView.setFeedbackIcon(icon); } } 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 438992e225779..4dec1f1f975c9 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 @@ -29,7 +29,6 @@ import android.util.ArrayMap; import android.util.AttributeSet; import android.util.IndentingPrintWriter; import android.util.Log; -import android.util.Pair; import android.view.LayoutInflater; import android.view.MotionEvent; import android.view.View; @@ -46,6 +45,7 @@ import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin; import com.android.systemui.statusbar.RemoteInputController; import com.android.systemui.statusbar.SmartReplyController; import com.android.systemui.statusbar.TransformableView; +import com.android.systemui.statusbar.notification.FeedbackIcon; import com.android.systemui.statusbar.notification.NotificationFadeAware; import com.android.systemui.statusbar.notification.NotificationUtils; import com.android.systemui.statusbar.notification.collection.NotificationEntry; @@ -1656,15 +1656,16 @@ public class NotificationContentView extends FrameLayout implements Notification return null; } - public void showFeedbackIcon(boolean show, Pair resIds) { + /** Shows the given feedback icon, or hides the icon if null. */ + public void setFeedbackIcon(@Nullable FeedbackIcon icon) { if (mContractedChild != null) { - mContractedWrapper.showFeedbackIcon(show, resIds); + mContractedWrapper.setFeedbackIcon(icon); } if (mExpandedChild != null) { - mExpandedWrapper.showFeedbackIcon(show, resIds); + mExpandedWrapper.setFeedbackIcon(icon); } if (mHeadsUpChild != null) { - mHeadsUpWrapper.showFeedbackIcon(show, resIds); + mHeadsUpWrapper.setFeedbackIcon(icon); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java index 8e02d9f635d33..6d13024e5489e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java @@ -337,14 +337,15 @@ public class NotificationGutsManager implements Dumpable, NotificationLifetimeEx private void initializeFeedbackInfo( final ExpandableNotificationRow row, FeedbackInfo feedbackInfo) { + if (mAssistantFeedbackController.getFeedbackIcon(row.getEntry()) == null) { + return; + } StatusBarNotification sbn = row.getEntry().getSbn(); UserHandle userHandle = sbn.getUser(); PackageManager pmUser = StatusBar.getPackageManagerForUser(mContext, userHandle.getIdentifier()); - if (mAssistantFeedbackController.showFeedbackIndicator(row.getEntry())) { - feedbackInfo.bindGuts(pmUser, sbn, row.getEntry(), row, mAssistantFeedbackController); - } + feedbackInfo.bindGuts(pmUser, sbn, row.getEntry(), row, mAssistantFeedbackController); } /** diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java index 8ee91342daed0..7a654365e0ae4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationHeaderViewWrapper.java @@ -21,7 +21,6 @@ import static com.android.systemui.statusbar.notification.TransformState.TRANSFO import android.app.Notification; import android.content.Context; import android.util.ArraySet; -import android.util.Pair; import android.view.NotificationHeaderView; import android.view.NotificationTopLineView; import android.view.View; @@ -32,12 +31,15 @@ import android.widget.ImageButton; import android.widget.ImageView; import android.widget.TextView; +import androidx.annotation.Nullable; + import com.android.internal.widget.CachingIconView; import com.android.internal.widget.NotificationExpandButton; import com.android.systemui.animation.Interpolators; import com.android.systemui.statusbar.TransformableView; import com.android.systemui.statusbar.ViewTransformationHelper; import com.android.systemui.statusbar.notification.CustomInterpolatorTransformation; +import com.android.systemui.statusbar.notification.FeedbackIcon; import com.android.systemui.statusbar.notification.ImageTransformState; import com.android.systemui.statusbar.notification.TransformState; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; @@ -126,16 +128,17 @@ public class NotificationHeaderViewWrapper extends NotificationViewWrapper { } } - /** Shows or hides feedback indicator */ + /** Shows the given feedback icon, or hides the icon if null. */ @Override - public void showFeedbackIcon(boolean show, Pair resIds) { + public void setFeedbackIcon(@Nullable FeedbackIcon icon) { if (mFeedbackIcon != null) { - mFeedbackIcon.setVisibility(show ? View.VISIBLE : View.GONE); - if (show) { + mFeedbackIcon.setVisibility(icon != null ? View.VISIBLE : View.GONE); + if (icon != null) { if (mFeedbackIcon instanceof ImageButton) { - ((ImageButton) mFeedbackIcon).setImageResource(resIds.first); + ((ImageButton) mFeedbackIcon).setImageResource(icon.getIconRes()); } - mFeedbackIcon.setContentDescription(mView.getContext().getString(resIds.second)); + mFeedbackIcon.setContentDescription( + mView.getContext().getString(icon.getContentDescRes())); } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationViewWrapper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationViewWrapper.java index 6c3e0d2f798b9..1c22f09332364 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationViewWrapper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationViewWrapper.java @@ -29,7 +29,6 @@ import android.graphics.Rect; import android.graphics.drawable.ColorDrawable; import android.graphics.drawable.Drawable; import android.os.Build; -import android.util.Pair; import android.view.NotificationHeaderView; import android.view.View; import android.view.ViewGroup; @@ -42,6 +41,7 @@ import com.android.internal.widget.CachingIconView; import com.android.settingslib.Utils; import com.android.systemui.statusbar.CrossFadeHelper; import com.android.systemui.statusbar.TransformableView; +import com.android.systemui.statusbar.notification.FeedbackIcon; import com.android.systemui.statusbar.notification.NotificationFadeAware; import com.android.systemui.statusbar.notification.TransformState; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; @@ -101,10 +101,8 @@ public abstract class NotificationViewWrapper implements TransformableView { public void onContentUpdated(ExpandableNotificationRow row) { } - /** - * Shows or hides feedback icon. - */ - public void showFeedbackIcon(boolean show, Pair resIds) { + /** Shows the given feedback icon, or hides the icon if null. */ + public void setFeedbackIcon(@Nullable FeedbackIcon icon) { } public void onReinflated() { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java index 18751242ec6fb..046a133741ae7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java @@ -24,7 +24,6 @@ import android.content.res.TypedArray; import android.graphics.drawable.ColorDrawable; import android.service.notification.StatusBarNotification; import android.util.AttributeSet; -import android.util.Pair; import android.view.ContextThemeWrapper; import android.view.LayoutInflater; import android.view.NotificationHeaderView; @@ -33,11 +32,14 @@ import android.view.ViewGroup; import android.widget.RemoteViews; import android.widget.TextView; +import androidx.annotation.Nullable; + import com.android.internal.annotations.VisibleForTesting; import com.android.internal.widget.NotificationExpandButton; import com.android.systemui.R; import com.android.systemui.statusbar.CrossFadeHelper; import com.android.systemui.statusbar.NotificationGroupingUtil; +import com.android.systemui.statusbar.notification.FeedbackIcon; import com.android.systemui.statusbar.notification.NotificationFadeAware; import com.android.systemui.statusbar.notification.NotificationUtils; import com.android.systemui.statusbar.notification.collection.legacy.VisualStabilityManager; @@ -1294,15 +1296,13 @@ public class NotificationChildrenContainer extends ViewGroup mCurrentHeaderTranslation = (int) ((1.0f - headerVisibleAmount) * mTranslationForHeader); } - /** - * Shows or hides feedback icon. - */ - public void showFeedbackIcon(boolean show, Pair resIds) { + /** Shows the given feedback icon, or hides the icon if null. */ + public void setFeedbackIcon(@Nullable FeedbackIcon icon) { if (mNotificationHeaderWrapper != null) { - mNotificationHeaderWrapper.showFeedbackIcon(show, resIds); + mNotificationHeaderWrapper.setFeedbackIcon(icon); } if (mNotificationHeaderWrapperLowPriority != null) { - mNotificationHeaderWrapperLowPriority.showFeedbackIcon(show, resIds); + mNotificationHeaderWrapperLowPriority.setFeedbackIcon(icon); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/AssistantFeedbackControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/AssistantFeedbackControllerTest.java index e16d4d71efe7d..fda8f519758c2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/AssistantFeedbackControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/AssistantFeedbackControllerTest.java @@ -32,6 +32,8 @@ import static com.android.systemui.statusbar.notification.AssistantFeedbackContr import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import android.app.Notification; @@ -43,7 +45,6 @@ import android.service.notification.StatusBarNotification; import android.test.suitebuilder.annotation.SmallTest; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; -import android.util.Pair; import com.android.internal.config.sysui.SystemUiDeviceConfigFlags; import com.android.systemui.SysuiTestCase; @@ -51,8 +52,6 @@ import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; import com.android.systemui.util.DeviceConfigProxyFake; -import junit.framework.Assert; - import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -97,9 +96,24 @@ public class AssistantFeedbackControllerTest extends SysuiTestCase { @Test public void testFeedback_flagDisabled() { switchFlag("false"); + // test flag disables logic with default values assertEquals(STATUS_UNCHANGED, mAssistantFeedbackController.getFeedbackStatus( getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_UNCHANGED))); - assertFalse(mAssistantFeedbackController.showFeedbackIndicator( + assertNull(mAssistantFeedbackController.getFeedbackIcon( + getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_UNCHANGED))); + // test that the flag disables logic with values that otherwise would return a value + assertEquals(STATUS_UNCHANGED, mAssistantFeedbackController.getFeedbackStatus( + getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_HIGH, RANKING_PROMOTED))); + assertNull(mAssistantFeedbackController.getFeedbackIcon( + getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_HIGH, RANKING_PROMOTED))); + } + + @Test + public void testFeedback_noChange() { + switchFlag("true"); + assertEquals(STATUS_UNCHANGED, mAssistantFeedbackController.getFeedbackStatus( + getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_UNCHANGED))); + assertNull(mAssistantFeedbackController.getFeedbackIcon( getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_UNCHANGED))); } @@ -108,15 +122,15 @@ public class AssistantFeedbackControllerTest extends SysuiTestCase { switchFlag("true"); NotificationEntry entry = getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_HIGH, RANKING_UNCHANGED); assertEquals(STATUS_PROMOTED, mAssistantFeedbackController.getFeedbackStatus(entry)); - assertTrue(mAssistantFeedbackController.showFeedbackIndicator(entry)); + assertNotNull(mAssistantFeedbackController.getFeedbackIcon(entry)); entry = getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_LOW, RANKING_UNCHANGED); assertEquals(STATUS_SILENCED, mAssistantFeedbackController.getFeedbackStatus(entry)); - assertTrue(mAssistantFeedbackController.showFeedbackIndicator(entry)); + assertNotNull(mAssistantFeedbackController.getFeedbackIcon(entry)); entry = getEntry(IMPORTANCE_LOW, IMPORTANCE_MIN, RANKING_UNCHANGED); assertEquals(STATUS_DEMOTED, mAssistantFeedbackController.getFeedbackStatus(entry)); - assertTrue(mAssistantFeedbackController.showFeedbackIndicator(entry)); + assertNotNull(mAssistantFeedbackController.getFeedbackIcon(entry)); } @Test @@ -125,18 +139,20 @@ public class AssistantFeedbackControllerTest extends SysuiTestCase { NotificationEntry entry = getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_PROMOTED); assertEquals(STATUS_PROMOTED, mAssistantFeedbackController.getFeedbackStatus(entry)); - assertTrue(mAssistantFeedbackController.showFeedbackIndicator(entry)); + assertNotNull(mAssistantFeedbackController.getFeedbackIcon(entry)); entry = getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_DEMOTED); assertEquals(STATUS_DEMOTED, mAssistantFeedbackController.getFeedbackStatus(entry)); - assertTrue(mAssistantFeedbackController.showFeedbackIndicator(entry)); + assertNotNull(mAssistantFeedbackController.getFeedbackIcon(entry)); } @Test - public void testGetFeedbackResources_flagDisabled() { - switchFlag("false"); - Assert.assertEquals(new Pair(0, 0), mAssistantFeedbackController.getFeedbackResources( - getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_DEFAULT, RANKING_UNCHANGED))); + public void testGetFeedbackIcon_whenPromoted() { + switchFlag("true"); + FeedbackIcon expected = new FeedbackIcon(com.android.internal.R.drawable.ic_feedback_uprank, + com.android.internal.R.string.notification_feedback_indicator_promoted); + assertEquals(expected, mAssistantFeedbackController.getFeedbackIcon( + getEntry(IMPORTANCE_DEFAULT, IMPORTANCE_HIGH, RANKING_PROMOTED))); } private NotificationEntry getEntry(int oldImportance, int newImportance, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/RowAppearanceCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/RowAppearanceCoordinatorTest.kt index 52fce130fe697..447ba1510e139 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/RowAppearanceCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/coordinator/RowAppearanceCoordinatorTest.kt @@ -17,10 +17,10 @@ package com.android.systemui.statusbar.notification.collection.coordinator import android.testing.AndroidTestingRunner import android.testing.TestableLooper.RunWithLooper -import android.util.Pair import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.statusbar.notification.AssistantFeedbackController +import com.android.systemui.statusbar.notification.FeedbackIcon import com.android.systemui.statusbar.notification.SectionClassifier import com.android.systemui.statusbar.notification.collection.NotifPipeline import com.android.systemui.statusbar.notification.collection.NotificationEntry @@ -75,8 +75,7 @@ class RowAppearanceCoordinatorTest : SysuiTestCase() { afterRenderEntryListener = withArgCaptor { verify(pipeline).addOnAfterRenderEntryListener(capture()) } - whenever(assistantFeedbackController.showFeedbackIndicator(any())).thenReturn(true) - whenever(assistantFeedbackController.getFeedbackResources(any())).thenReturn(Pair(1, 2)) + whenever(assistantFeedbackController.getFeedbackIcon(any())).thenReturn(FeedbackIcon(1, 2)) entry1 = NotificationEntryBuilder().setSection(section1).setLastAudiblyAlertedMs(17).build() entry2 = NotificationEntryBuilder().setSection(section2).build() } @@ -110,8 +109,8 @@ class RowAppearanceCoordinatorTest : SysuiTestCase() { } @Test - fun testShowFeedbackIcon() { + fun testSetFeedbackIcon() { afterRenderEntryListener.onAfterRenderEntry(entry1, controller1) - verify(controller1).showFeedbackIcon(eq(true), eq(Pair(1, 2))) + verify(controller1).setFeedbackIcon(eq(FeedbackIcon(1, 2))) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java index fa25c3f1e0053..e9e191107e5c7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java @@ -39,7 +39,6 @@ import android.app.NotificationChannel; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; -import android.util.Pair; import android.view.View; import androidx.test.filters.SmallTest; @@ -49,6 +48,7 @@ import com.android.systemui.SysuiTestCase; import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.notification.AboveShelfChangedListener; +import com.android.systemui.statusbar.notification.FeedbackIcon; import com.android.systemui.statusbar.notification.stack.NotificationChildrenContainer; import org.junit.Assert; @@ -212,7 +212,7 @@ public class ExpandableNotificationRowTest extends SysuiTestCase { // public notification is custom layout - no header mGroupRow.setSensitive(true, true); mGroupRow.setOnFeedbackClickListener(null); - mGroupRow.showFeedbackIcon(false, null); + mGroupRow.setFeedbackIcon(null); } @Test @@ -226,13 +226,13 @@ public class ExpandableNotificationRowTest extends SysuiTestCase { mGroupRow.setChildrenContainer(mockContainer); final boolean show = true; - final Pair resIds = new Pair(R.drawable.ic_feedback_alerted, - R.string.notification_feedback_indicator_alerted); - mGroupRow.showFeedbackIcon(show, resIds); + final FeedbackIcon icon = new FeedbackIcon( + R.drawable.ic_feedback_alerted, R.string.notification_feedback_indicator_alerted); + mGroupRow.setFeedbackIcon(icon); - verify(mockContainer, times(1)).showFeedbackIcon(show, resIds); - verify(privateLayout, times(1)).showFeedbackIcon(show, resIds); - verify(publicLayout, times(1)).showFeedbackIcon(show, resIds); + verify(mockContainer, times(1)).setFeedbackIcon(icon); + verify(privateLayout, times(1)).setFeedbackIcon(icon); + verify(publicLayout, times(1)).setFeedbackIcon(icon); } @Test diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentViewTest.java index 94e273b1965a3..682ff1fc8c523 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentViewTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationContentViewTest.java @@ -23,7 +23,6 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import android.util.Pair; import android.view.NotificationHeaderView; import android.view.View; import android.view.ViewPropertyAnimator; @@ -36,6 +35,7 @@ import com.android.internal.R; import com.android.internal.widget.NotificationExpandButton; import com.android.systemui.SysuiTestCase; import com.android.systemui.media.dialog.MediaOutputDialogFactory; +import com.android.systemui.statusbar.notification.FeedbackIcon; import org.junit.Before; import org.junit.Test; @@ -76,7 +76,7 @@ public class NotificationContentViewTest extends SysuiTestCase { @Test @UiThreadTest - public void testShowFeedbackIcon() { + public void testSetFeedbackIcon() { View mockContracted = mock(NotificationHeaderView.class); when(mockContracted.findViewById(com.android.internal.R.id.feedback)) .thenReturn(mockContracted); @@ -94,7 +94,7 @@ public class NotificationContentViewTest extends SysuiTestCase { mView.setExpandedChild(mockExpanded); mView.setHeadsUpChild(mockHeadsUp); - mView.showFeedbackIcon(true, new Pair(R.drawable.ic_feedback_alerted, + mView.setFeedbackIcon(new FeedbackIcon(R.drawable.ic_feedback_alerted, R.string.notification_feedback_indicator_alerted)); verify(mockContracted, times(1)).setVisibility(View.VISIBLE);