From 59d8322d5e175f354e2b0c6187e7d1a6c1f840ed Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Thu, 24 Jun 2021 14:02:27 +0200 Subject: [PATCH] Fixed an issue where notification dismissals wouldn't animate For notification groups with a single child, where the parent is suppressed, no notifications were generated for the dismissal because the stackscroller thought that a child transfer was in progress. This also fixes a case where we accidentally triggered a resize animation because remoteInputs were removed even though they haven't changed their visibility. This was masking the above behavior from manifesting always and only showed for notifications that had no remote input. Fixes: 187291379 Test: add group with a single child and no remote input, dismiss, observe animation happening Change-Id: Iab4f6697feb393c4e1cc8b5f546c9daa110f3230 --- .../NotificationViewHierarchyManager.java | 32 +++++++++++++------ .../statusbar/RemoteInputController.java | 2 ++ .../stack/NotificationStackScrollLayout.java | 9 +++++- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index 3c549f94ad0f5..467f27f640f9e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -158,14 +158,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle final int N = activeNotifications.size(); for (int i = 0; i < N; i++) { NotificationEntry ent = activeNotifications.get(i); - final boolean isBubbleNotificationSuppressedFromShade = mBubblesOptional.isPresent() - && mBubblesOptional.get().isBubbleNotificationSuppressedFromShade( - ent.getKey(), ent.getSbn().getGroupKey()); - if (ent.isRowDismissed() || ent.isRowRemoved() - || isBubbleNotificationSuppressedFromShade - || mFgsSectionController.hasEntry(ent)) { - // we don't want to update removed notifications because they could - // temporarily become children if they were isolated before. + if (shouldSuppressActiveNotification(ent)) { continue; } @@ -254,9 +247,11 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle } for (ExpandableNotificationRow viewToRemove : viewsToRemove) { - if (mEntryManager.getPendingOrActiveNotif(viewToRemove.getEntry().getKey()) != null) { + NotificationEntry entry = viewToRemove.getEntry(); + if (mEntryManager.getPendingOrActiveNotif(entry.getKey()) != null + && !shouldSuppressActiveNotification(entry)) { // we are only transferring this notification to its parent, don't generate an - // animation + // animation. If the notification is suppressed, this isn't a transfer. mListContainer.setChildTransferInProgress(true); } if (viewToRemove.isSummaryWithChildren()) { @@ -325,6 +320,23 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle endUpdate(); } + /** + * Should a notification entry from the active list be suppressed and not show? + */ + private boolean shouldSuppressActiveNotification(NotificationEntry ent) { + final boolean isBubbleNotificationSuppressedFromShade = mBubblesOptional.isPresent() + && mBubblesOptional.get().isBubbleNotificationSuppressedFromShade( + ent.getKey(), ent.getSbn().getGroupKey()); + if (ent.isRowDismissed() || ent.isRowRemoved() + || isBubbleNotificationSuppressedFromShade + || mFgsSectionController.hasEntry(ent)) { + // we want to suppress removed notifications because they could + // temporarily become children if they were isolated before. + return true; + } + return false; + } + private void addNotificationChildrenAndSort() { // Let's now add all notification children which are missing boolean orderChanged = false; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/RemoteInputController.java b/packages/SystemUI/src/com/android/systemui/statusbar/RemoteInputController.java index 924eb263de50d..b6aed23e64eec 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/RemoteInputController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/RemoteInputController.java @@ -132,6 +132,8 @@ public class RemoteInputController { public void removeRemoteInput(NotificationEntry entry, Object token) { Objects.requireNonNull(entry); if (entry.mRemoteEditImeVisible) return; + // If the view is being removed, this may be called even though we're not active + if (!isRemoteInputActive(entry)) return; pruneWeakThenRemoveAndContains(null /* contains */, entry /* remove */, token); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java index 4e6d376919e9b..5ccb064334189 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java @@ -143,7 +143,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable false /* default */); // TODO(b/187291379) disable again before release private static final boolean DEBUG_REMOVE_ANIMATION = SystemProperties.getBoolean( - "persist.debug.nssl.dismiss", true /* default */); + "persist.debug.nssl.dismiss", false /* default */); private static final float RUBBER_BAND_FACTOR_NORMAL = 0.35f; private static final float RUBBER_BAND_FACTOR_AFTER_EXPAND = 0.15f; @@ -3205,6 +3205,13 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable ignoreChildren); mAnimationEvents.add(event); mSwipedOutViews.remove(child); + if (DEBUG_REMOVE_ANIMATION) { + String key = ""; + if (child instanceof ExpandableNotificationRow) { + key = ((ExpandableNotificationRow) child).getEntry().getKey(); + } + Log.d(TAG, "created Remove Event - SwipedOut: " + childWasSwipedOut + " " + key); + } } mChildrenToRemoveAnimated.clear(); }