From 35956b4551b8ef65cba7874767b043cd7b5d3103 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Tue, 25 Feb 2020 17:12:09 -0500 Subject: [PATCH] Update section header placement logic This new logic accounts for the current position of the header when determining the new placement in the NSSL. Fixes: 150166074 Test: atest, manual Change-Id: I699e5dd93ae1d13fc56aa5f411825a357199ff88 --- .../stack/NotificationSectionsManager.java | 197 +++++++++++------- .../NotificationSectionsManagerTest.java | 176 +++++++++++----- 2 files changed, 255 insertions(+), 118 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java index b3561c2deda73..8ee2f5002f0d2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManager.java @@ -16,6 +16,8 @@ package com.android.systemui.statusbar.notification.stack; +import static com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayout.ROWS_GENTLE; + import static java.lang.annotation.RetentionPolicy.SOURCE; import android.annotation.IntDef; @@ -100,14 +102,11 @@ public class NotificationSectionsManager implements StackScrollAlgorithm.Section private boolean mInitialized = false; private SectionHeaderView mGentleHeader; - private boolean mGentleHeaderVisible; @Nullable private View.OnClickListener mOnClearGentleNotifsClickListener; private SectionHeaderView mAlertingHeader; - private boolean mAlertingHeaderVisible; private PeopleHubView mPeopleHubView; - private boolean mPeopleHeaderVisible; private boolean mPeopleHubVisible = false; @Nullable private Subscription mPeopleHubSubscription; @@ -231,88 +230,135 @@ public class NotificationSectionsManager implements StackScrollAlgorithm.Section return; } + // The overall strategy here is to iterate over the current children of mParent, looking + // for where the sections headers are currently positioned, and where each section begins. + // Then, once we find the start of a new section, we track that position as the "target" for + // the section header, adjusted for the case where existing headers are in front of that + // target, but won't be once they are moved / removed after the pass has completed. + final boolean showHeaders = mStatusBarStateController.getState() != StatusBarState.KEYGUARD; final boolean usingPeopleFiltering = mSectionsFeatureManager.isFilteringEnabled(); boolean peopleNotifsPresent = false; + + int currentPeopleHeaderIdx = -1; int peopleHeaderTarget = -1; + int currentAlertingHeaderIdx = -1; int alertingHeaderTarget = -1; + int currentGentleHeaderIdx = -1; int gentleHeaderTarget = -1; - int viewCount = 0; + int lastNotifIndex = 0; - if (showHeaders) { - final int childCount = mParent.getChildCount(); - for (int i = 0; i < childCount; i++) { - View child = mParent.getChildAt(i); - if (child.getVisibility() == View.GONE - || !(child instanceof ExpandableNotificationRow)) { - continue; - } - ExpandableNotificationRow row = (ExpandableNotificationRow) child; - switch (row.getEntry().getBucket()) { - case BUCKET_PEOPLE: - if (peopleHeaderTarget == -1) { - peopleNotifsPresent = true; - peopleHeaderTarget = viewCount; - viewCount++; - } - break; - case BUCKET_ALERTING: - if (usingPeopleFiltering && alertingHeaderTarget == -1) { - alertingHeaderTarget = viewCount; - viewCount++; - } - break; - case BUCKET_SILENT: - if (gentleHeaderTarget == -1) { - gentleHeaderTarget = viewCount; - viewCount++; - } - break; - } - viewCount++; + final int childCount = mParent.getChildCount(); + for (int i = 0; i < childCount; i++) { + View child = mParent.getChildAt(i); + + // Track the existing positions of the headers + if (child == mPeopleHubView) { + currentPeopleHeaderIdx = i; + continue; } - if (usingPeopleFiltering && mPeopleHubVisible && peopleHeaderTarget == -1) { - // Insert the people header even if there are no people visible, in order to show - // the hub. Put it directly above the next header. - if (alertingHeaderTarget != -1) { - peopleHeaderTarget = alertingHeaderTarget; - alertingHeaderTarget++; - gentleHeaderTarget++; - } else if (gentleHeaderTarget != -1) { - peopleHeaderTarget = gentleHeaderTarget; - gentleHeaderTarget++; - } else { - // Put it at the end of the list. - peopleHeaderTarget = viewCount; - } + if (child == mAlertingHeader) { + currentAlertingHeaderIdx = i; + continue; + } + if (child == mGentleHeader) { + currentGentleHeaderIdx = i; + continue; + } + + if (!(child instanceof ExpandableNotificationRow)) { + continue; + } + lastNotifIndex = i; + ExpandableNotificationRow row = (ExpandableNotificationRow) child; + // Once we enter a new section, calculate the target position for the header. + switch (row.getEntry().getBucket()) { + case BUCKET_HEADS_UP: + break; + case BUCKET_PEOPLE: + peopleNotifsPresent = true; + if (showHeaders && peopleHeaderTarget == -1) { + peopleHeaderTarget = i; + // Offset the target if there are other headers before this that will be + // moved. + if (currentPeopleHeaderIdx != -1) { + peopleHeaderTarget--; + } + if (currentAlertingHeaderIdx != -1) { + peopleHeaderTarget--; + } + if (currentGentleHeaderIdx != -1) { + peopleHeaderTarget--; + } + } + break; + case BUCKET_ALERTING: + if (showHeaders && usingPeopleFiltering && alertingHeaderTarget == -1) { + alertingHeaderTarget = i; + // Offset the target if there are other headers before this that will be + // moved. + if (currentAlertingHeaderIdx != -1) { + alertingHeaderTarget--; + } + if (currentGentleHeaderIdx != -1) { + alertingHeaderTarget--; + } + } + break; + case BUCKET_SILENT: + if (showHeaders && gentleHeaderTarget == -1) { + gentleHeaderTarget = i; + // Offset the target if there are other headers before this that will be + // moved. + if (currentGentleHeaderIdx != -1) { + gentleHeaderTarget--; + } + } + break; + default: + throw new IllegalStateException("Cannot find section bucket for view"); + } + } + if (showHeaders && usingPeopleFiltering && mPeopleHubVisible && peopleHeaderTarget == -1) { + // Insert the people header even if there are no people visible, in order to show + // the hub. Put it directly above the next header. + if (alertingHeaderTarget != -1) { + peopleHeaderTarget = alertingHeaderTarget; + } else if (gentleHeaderTarget != -1) { + peopleHeaderTarget = gentleHeaderTarget; + } else { + // Put it at the end of the list. + peopleHeaderTarget = lastNotifIndex; } } - // Allow swiping the people header if the section is empty - mPeopleHubView.setCanSwipe(mPeopleHubVisible && !peopleNotifsPresent); + // Add headers in reverse order to preserve indices + adjustHeaderVisibilityAndPosition( + gentleHeaderTarget, mGentleHeader, currentGentleHeaderIdx); + adjustHeaderVisibilityAndPosition( + alertingHeaderTarget, mAlertingHeader, currentAlertingHeaderIdx); + adjustHeaderVisibilityAndPosition( + peopleHeaderTarget, mPeopleHubView, currentPeopleHeaderIdx); - mPeopleHeaderVisible = adjustHeaderVisibilityAndPosition( - peopleHeaderTarget, mPeopleHubView, mPeopleHeaderVisible); - mAlertingHeaderVisible = adjustHeaderVisibilityAndPosition( - alertingHeaderTarget, mAlertingHeader, mAlertingHeaderVisible); - mGentleHeaderVisible = adjustHeaderVisibilityAndPosition( - gentleHeaderTarget, mGentleHeader, mGentleHeaderVisible); + // Update headers to reflect state of section contents + mGentleHeader.setAreThereDismissableGentleNotifs( + mParent.hasActiveClearableNotifications(ROWS_GENTLE)); + mPeopleHubView.setCanSwipe(showHeaders && mPeopleHubVisible && !peopleNotifsPresent); + if (peopleHeaderTarget != currentPeopleHeaderIdx) { + mPeopleHubView.resetTranslation(); + } } - private boolean adjustHeaderVisibilityAndPosition( - int targetIndex, StackScrollerDecorView header, boolean isCurrentlyVisible) { - if (targetIndex == -1) { - if (isCurrentlyVisible) { + private void adjustHeaderVisibilityAndPosition( + int targetPosition, StackScrollerDecorView header, int currentPosition) { + if (targetPosition == -1) { + if (currentPosition != -1) { mParent.removeView(header); } - return false; } else { - if (header instanceof SwipeableView) { - ((SwipeableView) header).resetTranslation(); - } - if (!isCurrentlyVisible) { + if (currentPosition == -1) { // If the header is animating away, it will still have a parent, so detach it first // TODO: We should really cancel the active animations here. This will happen // automatically when the view's intro animation starts, but it's a fragile link. @@ -321,11 +367,10 @@ public class NotificationSectionsManager implements StackScrollAlgorithm.Section header.setTransientContainer(null); } header.setContentVisible(true); - mParent.addView(header, targetIndex); - } else if (mParent.indexOfChild(header) != targetIndex) { - mParent.changeViewPosition(header, targetIndex); + mParent.addView(header, targetPosition); + } else { + mParent.changeViewPosition(header, targetPosition); } - return true; } } @@ -400,10 +445,20 @@ public class NotificationSectionsManager implements StackScrollAlgorithm.Section @VisibleForTesting - SectionHeaderView getGentleHeaderView() { + ExpandableView getGentleHeaderView() { return mGentleHeader; } + @VisibleForTesting + ExpandableView getAlertingHeaderView() { + return mAlertingHeader; + } + + @VisibleForTesting + ExpandableView getPeopleHeaderView() { + return mPeopleHubView; + } + private final ConfigurationListener mConfigurationListener = new ConfigurationListener() { @Override public void onLocaleListChanged() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManagerTest.java index abfbcd99167b5..c64dd093a4c22 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationSectionsManagerTest.java @@ -19,6 +19,8 @@ package com.android.systemui.statusbar.notification.stack; import static android.view.ViewGroup.LayoutParams.WRAP_CONTENT; import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_ALERTING; +import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_HEADS_UP; +import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_PEOPLE; import static com.android.systemui.statusbar.notification.stack.NotificationSectionsManager.BUCKET_SILENT; import static org.mockito.ArgumentMatchers.any; @@ -107,7 +109,7 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { @Test public void testInsertHeader() { // GIVEN a stack with HI and LO rows but no section headers - setStackState(ChildType.HIPRI, ChildType.HIPRI, ChildType.HIPRI, ChildType.LOPRI); + setStackState(ChildType.ALERTING, ChildType.ALERTING, ChildType.ALERTING, ChildType.GENTLE); // WHEN we update the section headers mSectionsManager.updateSectionBoundaries(); @@ -119,11 +121,15 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { @Test public void testRemoveHeader() { // GIVEN a stack that originally had a header between the HI and LO sections - setStackState(ChildType.HIPRI, ChildType.HIPRI, ChildType.LOPRI); + setStackState(ChildType.ALERTING, ChildType.ALERTING, ChildType.GENTLE); mSectionsManager.updateSectionBoundaries(); // WHEN the last LO row is replaced with a HI row - setStackState(ChildType.HIPRI, ChildType.HIPRI, ChildType.HEADER, ChildType.HIPRI); + setStackState( + ChildType.ALERTING, + ChildType.ALERTING, + ChildType.GENTLE_HEADER, + ChildType.ALERTING); clearInvocations(mNssl); mSectionsManager.updateSectionBoundaries(); @@ -134,7 +140,7 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { @Test public void testDoNothingIfHeaderAlreadyRemoved() { // GIVEN a stack with only HI rows - setStackState(ChildType.HIPRI, ChildType.HIPRI, ChildType.HIPRI); + setStackState(ChildType.ALERTING, ChildType.ALERTING, ChildType.ALERTING); // WHEN we update the sections headers mSectionsManager.updateSectionBoundaries(); @@ -147,19 +153,19 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { public void testMoveHeaderForward() { // GIVEN a stack that originally had a header between the HI and LO sections setStackState( - ChildType.HIPRI, - ChildType.HIPRI, - ChildType.HIPRI, - ChildType.LOPRI); + ChildType.ALERTING, + ChildType.ALERTING, + ChildType.ALERTING, + ChildType.GENTLE); mSectionsManager.updateSectionBoundaries(); // WHEN the LO section moves forward setStackState( - ChildType.HIPRI, - ChildType.HIPRI, - ChildType.LOPRI, - ChildType.HEADER, - ChildType.LOPRI); + ChildType.ALERTING, + ChildType.ALERTING, + ChildType.GENTLE, + ChildType.GENTLE_HEADER, + ChildType.GENTLE); mSectionsManager.updateSectionBoundaries(); // THEN the LO section header is also moved forward @@ -170,19 +176,19 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { public void testMoveHeaderBackward() { // GIVEN a stack that originally had a header between the HI and LO sections setStackState( - ChildType.HIPRI, - ChildType.LOPRI, - ChildType.LOPRI, - ChildType.LOPRI); + ChildType.ALERTING, + ChildType.GENTLE, + ChildType.GENTLE, + ChildType.GENTLE); mSectionsManager.updateSectionBoundaries(); // WHEN the LO section moves backward setStackState( - ChildType.HIPRI, - ChildType.HEADER, - ChildType.HIPRI, - ChildType.HIPRI, - ChildType.LOPRI); + ChildType.ALERTING, + ChildType.GENTLE_HEADER, + ChildType.ALERTING, + ChildType.ALERTING, + ChildType.GENTLE); mSectionsManager.updateSectionBoundaries(); // THEN the LO section header is also moved backward (with appropriate index shifting) @@ -193,14 +199,14 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { public void testHeaderRemovedFromTransientParent() { // GIVEN a stack where the header is animating away setStackState( - ChildType.HIPRI, - ChildType.LOPRI, - ChildType.LOPRI, - ChildType.LOPRI); + ChildType.ALERTING, + ChildType.GENTLE, + ChildType.GENTLE, + ChildType.GENTLE); mSectionsManager.updateSectionBoundaries(); setStackState( - ChildType.HIPRI, - ChildType.HEADER); + ChildType.ALERTING, + ChildType.GENTLE_HEADER); mSectionsManager.updateSectionBoundaries(); clearInvocations(mNssl); @@ -209,8 +215,8 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { // WHEN the LO section reappears setStackState( - ChildType.HIPRI, - ChildType.LOPRI); + ChildType.ALERTING, + ChildType.GENTLE); mSectionsManager.updateSectionBoundaries(); // THEN the header is first removed from the transient parent before being added to the @@ -223,7 +229,7 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { public void testHeaderNotShownOnLockscreen() { // GIVEN a stack of HI and LO notifs on the lockscreen when(mStatusBarStateController.getState()).thenReturn(StatusBarState.KEYGUARD); - setStackState(ChildType.HIPRI, ChildType.HIPRI, ChildType.HIPRI, ChildType.LOPRI); + setStackState(ChildType.ALERTING, ChildType.ALERTING, ChildType.ALERTING, ChildType.GENTLE); // WHEN we update the section headers mSectionsManager.updateSectionBoundaries(); @@ -236,7 +242,7 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { public void testHeaderShownWhenEnterLockscreen() { // GIVEN a stack of HI and LO notifs on the lockscreen when(mStatusBarStateController.getState()).thenReturn(StatusBarState.KEYGUARD); - setStackState(ChildType.HIPRI, ChildType.HIPRI, ChildType.HIPRI, ChildType.LOPRI); + setStackState(ChildType.ALERTING, ChildType.ALERTING, ChildType.ALERTING, ChildType.GENTLE); mSectionsManager.updateSectionBoundaries(); // WHEN we unlock @@ -250,37 +256,104 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { @Test public void testHeaderHiddenWhenEnterLockscreen() { // GIVEN a stack of HI and LO notifs on the shade - when(mStatusBarStateController.getState()).thenReturn(StatusBarState.SHADE_LOCKED); - setStackState(ChildType.HIPRI, ChildType.HIPRI, ChildType.HIPRI, ChildType.LOPRI); - mSectionsManager.updateSectionBoundaries(); + setStackState(ChildType.ALERTING, ChildType.GENTLE_HEADER, ChildType.GENTLE); // WHEN we go back to the keyguard when(mStatusBarStateController.getState()).thenReturn(StatusBarState.KEYGUARD); mSectionsManager.updateSectionBoundaries(); // Then the section header is removed - verify(mNssl).removeView(eq(mSectionsManager.getGentleHeaderView())); + verify(mNssl).removeView(mSectionsManager.getGentleHeaderView()); } - private enum ChildType { HEADER, HIPRI, LOPRI } + @Test + public void testPeopleFiltering_addHeadersFromShowingOnlyGentle() { + enablePeopleFiltering(); + + setStackState( + ChildType.GENTLE_HEADER, + ChildType.PERSON, + ChildType.ALERTING, + ChildType.GENTLE); + mSectionsManager.updateSectionBoundaries(); + + verify(mNssl).changeViewPosition(mSectionsManager.getGentleHeaderView(), 2); + verify(mNssl).addView(mSectionsManager.getAlertingHeaderView(), 1); + verify(mNssl).addView(mSectionsManager.getPeopleHeaderView(), 0); + } + + @Test + public void testPeopleFiltering_addAllHeaders() { + enablePeopleFiltering(); + + setStackState( + ChildType.PERSON, + ChildType.ALERTING, + ChildType.GENTLE); + mSectionsManager.updateSectionBoundaries(); + + verify(mNssl).addView(mSectionsManager.getGentleHeaderView(), 2); + verify(mNssl).addView(mSectionsManager.getAlertingHeaderView(), 1); + verify(mNssl).addView(mSectionsManager.getPeopleHeaderView(), 0); + } + + @Test + public void testPeopleFiltering_moveAllHeaders() { + enablePeopleFiltering(); + + setStackState( + ChildType.PEOPLE_HEADER, + ChildType.ALERTING_HEADER, + ChildType.GENTLE_HEADER, + ChildType.PERSON, + ChildType.ALERTING, + ChildType.GENTLE); + mSectionsManager.updateSectionBoundaries(); + + verify(mNssl).changeViewPosition(mSectionsManager.getGentleHeaderView(), 4); + verify(mNssl).changeViewPosition(mSectionsManager.getAlertingHeaderView(), 2); + verify(mNssl).changeViewPosition(mSectionsManager.getPeopleHeaderView(), 0); + } + + private void enablePeopleFiltering() { + when(mSectionsFeatureManager.isFilteringEnabled()).thenReturn(true); + when(mSectionsFeatureManager.getNumberOfBuckets()).thenReturn(4); + } + + private enum ChildType { + PEOPLE_HEADER, ALERTING_HEADER, GENTLE_HEADER, HEADS_UP, PERSON, ALERTING, GENTLE, OTHER + } private void setStackState(ChildType... children) { when(mNssl.getChildCount()).thenReturn(children.length); for (int i = 0; i < children.length; i++) { View child; switch (children[i]) { - case HEADER: + case PEOPLE_HEADER: + child = mSectionsManager.getPeopleHeaderView(); + break; + case ALERTING_HEADER: + child = mSectionsManager.getAlertingHeaderView(); + break; + case GENTLE_HEADER: child = mSectionsManager.getGentleHeaderView(); break; - case HIPRI: - case LOPRI: - ExpandableNotificationRow notifRow = mock(ExpandableNotificationRow.class, - RETURNS_DEEP_STUBS); - when(notifRow.getVisibility()).thenReturn(View.VISIBLE); - when(notifRow.getEntry().getBucket()).thenReturn( - children[i] == ChildType.HIPRI ? BUCKET_ALERTING : BUCKET_SILENT); - when(notifRow.getParent()).thenReturn(mNssl); - child = notifRow; + case HEADS_UP: + child = mockNotification(BUCKET_HEADS_UP); + break; + case PERSON: + child = mockNotification(BUCKET_PEOPLE); + break; + case ALERTING: + child = mockNotification(BUCKET_ALERTING); + break; + case GENTLE: + child = mockNotification(BUCKET_SILENT); + break; + case OTHER: + child = mock(View.class); + when(child.getVisibility()).thenReturn(View.VISIBLE); + when(child.getParent()).thenReturn(mNssl); break; default: throw new RuntimeException("Unknown ChildType: " + children[i]); @@ -289,4 +362,13 @@ public class NotificationSectionsManagerTest extends SysuiTestCase { when(mNssl.indexOfChild(child)).thenReturn(i); } } + + private View mockNotification(int bucket) { + ExpandableNotificationRow notifRow = mock(ExpandableNotificationRow.class, + RETURNS_DEEP_STUBS); + when(notifRow.getVisibility()).thenReturn(View.VISIBLE); + when(notifRow.getEntry().getBucket()).thenReturn(bucket); + when(notifRow.getParent()).thenReturn(mNssl); + return notifRow; + } }