From 3f19f60d654421eee5b35a92129081f08c977629 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Mon, 2 May 2016 18:01:56 -0700 Subject: [PATCH] Fixed an issue where children would animate strangely when removed Previously we tried to fix a notification flicker when the summary of a group was removed. This lead to a few issues that group children would now also generate an animation and the whole notification would just look weird. We are now cancelling notification children as soon as their parent is cancelled already in systemUI to ensure that the animations are properly rendered. Change-Id: Ie639b4ad28bdb55d922308e04c14a4e5b32b90bb Fixes: 28190616 --- .../systemui/statusbar/BaseStatusBar.java | 3 +- .../statusbar/ExpandableNotificationRow.java | 30 ++++++++++++++ .../phone/NotificationGroupManager.java | 6 +++ .../statusbar/phone/PhoneStatusBar.java | 40 ++++++++++++++++++- .../stack/NotificationStackScrollLayout.java | 1 + .../NotificationManagerService.java | 2 +- 6 files changed, 79 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/BaseStatusBar.java b/packages/SystemUI/src/com/android/systemui/statusbar/BaseStatusBar.java index be683444fb43c..05c9fd43c83c2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/BaseStatusBar.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/BaseStatusBar.java @@ -2161,7 +2161,8 @@ public abstract class BaseStatusBar extends SystemUI implements entry.row.setOnKeyguard(false); entry.row.setSystemExpanded(visibleNotifications == 0 && !childNotification); } - boolean suppressedSummary = mGroupManager.isSummaryOfSuppressedGroup(entry.notification); + boolean suppressedSummary = mGroupManager.isSummaryOfSuppressedGroup( + entry.notification) && !entry.row.isRemoved(); boolean childWithVisibleSummary = childNotification && mGroupManager.getGroupSummary(entry.notification).getVisibility() == View.VISIBLE; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java b/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java index 5b16fce925d18..af48c9f901d92 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java @@ -156,6 +156,9 @@ public class ExpandableNotificationRow extends ActivatableNotificationView { } } }; + private boolean mDismissed; + private boolean mKeepInParent; + private boolean mRemoved; public NotificationContentView getPrivateLayout() { return mPrivateLayout; @@ -632,6 +635,9 @@ public class ExpandableNotificationRow extends ActivatableNotificationView { ArrayList clonedList = new ArrayList<>(notificationChildren); for (int i = 0; i < clonedList.size(); i++) { ExpandableNotificationRow row = clonedList.get(i); + if (row.keepInParent()) { + continue; + } mChildrenContainer.removeNotification(row); mHeaderUtil.restoreNotificationHeader(row); row.setIsChildInGroup(false, null); @@ -639,6 +645,30 @@ public class ExpandableNotificationRow extends ActivatableNotificationView { onChildrenCountChanged(); } + public void setDismissed(boolean dismissed) { + mDismissed = dismissed; + } + + public boolean isDismissed() { + return mDismissed; + } + + public boolean keepInParent() { + return mKeepInParent; + } + + public void setKeepInParent(boolean keepInParent) { + mKeepInParent = keepInParent; + } + + public boolean isRemoved() { + return mRemoved; + } + + public void setRemoved(boolean removed) { + mRemoved = removed; + } + public interface ExpansionLogger { public void logNotificationExpansion(String key, boolean userAction, boolean expanded); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupManager.java index fffb20a7425c7..a51ee4df7ec38 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationGroupManager.java @@ -241,6 +241,12 @@ public class NotificationGroupManager implements HeadsUpManager.OnHeadsUpChanged if (group == null || group.summary == null || group.suppressed) { return false; } + if (group.children.isEmpty()) { + // If the suppression of a group changes because the last child was removed, this can + // still be called temporarily because the child hasn't been fully removed yet. Let's + // make sure we still return false in that case. + return false; + } return true; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBar.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBar.java index 23e3561710a38..c660cd8c634e4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBar.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/PhoneStatusBar.java @@ -1484,6 +1484,8 @@ public class PhoneStatusBar extends BaseStatusBar implements DemoMode, mHeadsUpEntriesToRemoveOnSwitch.add(mHeadsUpManager.getEntry(key)); return; } + // Let's remove the children if this was a summary + handleGroupSummaryRemoved(key, ranking); StatusBarNotification old = removeNotificationViews(key, ranking); if (SPEW) Log.d(TAG, "removeNotification key=" + key + " old=" + old); @@ -1500,6 +1502,40 @@ public class PhoneStatusBar extends BaseStatusBar implements DemoMode, setAreThereNotifications(); } + /** + * Ensures that the group children are cancelled immediately when the group summary is cancelled + * instead of waiting for the notification manager to send all cancels. Otherwise this could + * lead to flickers. + * + * This also ensures that the animation looks nice and only consists of a single disappear + * animation instead of multiple. + * + * @param key the key of the notification was removed + * @param ranking the current ranking + */ + private void handleGroupSummaryRemoved(String key, + RankingMap ranking) { + Entry entry = mNotificationData.get(key); + if (entry != null && entry.row != null + && entry.row.isSummaryWithChildren()) { + if (entry.notification.getOverrideGroupKey() != null && !entry.row.isDismissed()) { + // We don't want to remove children for autobundled notifications as they are not + // always cancelled. We only remove them if they were dismissed by the user. + return; + } + entry.row.setRemoved(true); + List notificationChildren = + entry.row.getNotificationChildren(); + ArrayList toRemove = new ArrayList<>(notificationChildren); + for (int i = 0; i < toRemove.size(); i++) { + toRemove.get(i).setKeepInParent(true); + } + for (int i = 0; i < toRemove.size(); i++) { + removeNotification(toRemove.get(i).getStatusBarNotification().getKey(), ranking); + } + } + } + @Override protected void refreshLayout(int layoutDirection) { if (mNavigationBarView != null) { @@ -1689,7 +1725,9 @@ public class PhoneStatusBar extends BaseStatusBar implements DemoMode, if (children != null) { toRemove.clear(); for (ExpandableNotificationRow childRow : children) { - if (orderedChildren == null || !orderedChildren.contains(childRow)) { + if ((orderedChildren == null + || !orderedChildren.contains(childRow)) + && !childRow.keepInParent()) { toRemove.add(childRow); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationStackScrollLayout.java index 9603e81c1e5db..13d5a669a3ef3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationStackScrollLayout.java @@ -723,6 +723,7 @@ public class NotificationStackScrollLayout extends ViewGroup performDismiss(groupSummary); } } + row.setDismissed(true); } final View veto = v.findViewById(R.id.veto); if (veto != null && veto.getVisibility() != View.GONE) { diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 9bdb14920fc89..e411579dbf84b 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -3316,9 +3316,9 @@ public class NotificationManagerService extends SystemService { mNotificationList.remove(index); + cancelNotificationLocked(r, sendDelete, reason); cancelGroupChildrenLocked(r, callingUid, callingPid, listenerName, REASON_GROUP_SUMMARY_CANCELED); - cancelNotificationLocked(r, sendDelete, reason); updateLightsLocked(); } }