From 8875de1d0837ebaa5aabe9230e07ac3e6c23ba53 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Thu, 22 Mar 2018 10:14:32 -0700 Subject: [PATCH] Fixed an issue where the roundness of a notification could be wrong When animating away a heads-up, there was no listener in place and if the view in question wasn't the first notification, it's roundness could be wrong. Change-Id: I0676ee18c9fb0c52b190b82b8b9adfa63d584262 Fixes: 76147670 Test: runtest systemUI --- .../statusbar/ExpandableNotificationRow.java | 10 ++++++++ .../statusbar/NotificationEntryManager.java | 1 + .../statusbar/NotificationListContainer.java | 7 ++++++ .../stack/NotificationRoundnessManager.java | 5 ++++ .../stack/NotificationStackScrollLayout.java | 7 ++++++ .../ExpandableNotificationRowTest.java | 13 ++++++++++ .../NotificationRoundnessManagerTest.java | 25 +++++++++++++++++++ 7 files changed, 68 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java b/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java index 9bd8080525a76..6b9567dd9d8a9 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java @@ -88,6 +88,7 @@ import com.android.systemui.statusbar.stack.StackScrollState; import java.util.ArrayList; import java.util.List; import java.util.function.BooleanSupplier; +import java.util.function.Consumer; public class ExpandableNotificationRow extends ActivatableNotificationView implements PluginListener { @@ -179,6 +180,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView private boolean mExpandAnimationRunning; private AboveShelfChangedListener mAboveShelfChangedListener; private HeadsUpManager mHeadsUpManager; + private Consumer mHeadsUpAnimatingAwayListener; private View mHelperButton; private boolean mChildIsExpanding; @@ -1115,13 +1117,21 @@ public class ExpandableNotificationRow extends ActivatableNotificationView public void setHeadsUpAnimatingAway(boolean headsUpAnimatingAway) { boolean wasAboveShelf = isAboveShelf(); + boolean changed = headsUpAnimatingAway != mHeadsupDisappearRunning; mHeadsupDisappearRunning = headsUpAnimatingAway; mPrivateLayout.setHeadsUpAnimatingAway(headsUpAnimatingAway); + if (changed && mHeadsUpAnimatingAwayListener != null) { + mHeadsUpAnimatingAwayListener.accept(headsUpAnimatingAway); + } if (isAboveShelf() != wasAboveShelf) { mAboveShelfChangedListener.onAboveShelfStateChanged(!wasAboveShelf); } } + public void setHeadsUpAnimatingAwayListener(Consumer listener) { + mHeadsUpAnimatingAwayListener = listener; + } + /** * @return if the view was just heads upped and is now animating away. During such a time the * layout needs to be kept consistent diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationEntryManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationEntryManager.java index 48828ab9d032e..7a7cc99eb44d0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationEntryManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationEntryManager.java @@ -332,6 +332,7 @@ public class NotificationEntryManager implements Dumpable, NotificationInflater. row.setOnExpandClickListener(mPresenter); row.setInflationCallback(this); row.setLongPressListener(getNotificationLongClicker()); + mListContainer.bindRow(row); mRemoteInputManager.bindRow(row); // Get the app name. diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListContainer.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListContainer.java index 0c19ec091ca62..af9a3a3775bf7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListContainer.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationListContainer.java @@ -188,4 +188,11 @@ public interface NotificationListContainer { default void applyExpandAnimationParams(ExpandAnimationParameters params) {} default void setExpandingNotification(ExpandableNotificationRow row) {} + + /** + * Bind a newly created row. + * + * @param row The notification to bind. + */ + default void bindRow(ExpandableNotificationRow row) {} } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationRoundnessManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationRoundnessManager.java index a36c966a33104..f98b3d92a1133 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationRoundnessManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationRoundnessManager.java @@ -47,6 +47,11 @@ class NotificationRoundnessManager implements OnHeadsUpChangedListener { updateRounding(headsUp, true /* animate */); } + public void onHeadsupAnimatingAwayChanged(ExpandableNotificationRow row, + boolean isAnimatingAway) { + updateRounding(row, false /* animate */); + } + private void updateRounding(ActivatableNotificationView view, boolean animate) { float topRoundness = getRoundness(view, true /* top */); float bottomRoundness = getRoundness(view, false /* top */); 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 14e801f0b2acb..8a3cb0d67080b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/stack/NotificationStackScrollLayout.java @@ -109,6 +109,7 @@ import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.function.BiConsumer; +import java.util.function.Consumer; /** * A layout which handles a dynamic amount of notifications and presents them in a scrollable stack. @@ -3013,6 +3014,12 @@ public class NotificationStackScrollLayout extends ViewGroup requestChildrenUpdate(); } + @Override + public void bindRow(ExpandableNotificationRow row) { + row.setHeadsUpAnimatingAwayListener(animatingAway + -> mRoundnessManager.onHeadsupAnimatingAwayChanged(row, animatingAway)); + } + @Override public void applyExpandAnimationParams(ExpandAnimationParameters params) { mAmbientState.setExpandAnimationTopChange(params == null ? 0 : params.getTopChange()); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/ExpandableNotificationRowTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/ExpandableNotificationRowTest.java index 34e444e9ad18a..231a186686a0c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/ExpandableNotificationRowTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/ExpandableNotificationRowTest.java @@ -40,17 +40,22 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.function.Consumer; + @SmallTest @RunWith(AndroidJUnit4.class) public class ExpandableNotificationRowTest extends SysuiTestCase { private ExpandableNotificationRow mGroup; private NotificationTestHelper mNotificationTestHelper; + boolean mHeadsUpAnimatingAway = false; @Before public void setUp() throws Exception { mNotificationTestHelper = new NotificationTestHelper(mContext); mGroup = mNotificationTestHelper.createGroup(); + mGroup.setHeadsUpAnimatingAwayListener( + animatingAway -> mHeadsUpAnimatingAway = animatingAway); } @Test @@ -195,4 +200,12 @@ public class ExpandableNotificationRowTest extends SysuiTestCase { mGroup.getAppOpsOnClickListener().onClick(view); verify(l, times(1)).onClick(any(), anyInt(), anyInt(), any()); } + + @Test + public void testHeadsUpAnimatingAwayListener() { + mGroup.setHeadsUpAnimatingAway(true); + Assert.assertEquals(true, mHeadsUpAnimatingAway); + mGroup.setHeadsUpAnimatingAway(false); + Assert.assertEquals(false, mHeadsUpAnimatingAway); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/stack/NotificationRoundnessManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/stack/NotificationRoundnessManagerTest.java index 1d2c01dbd5640..28d1aff7f9838 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/stack/NotificationRoundnessManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/stack/NotificationRoundnessManagerTest.java @@ -46,6 +46,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import java.util.HashSet; +import java.util.function.Consumer; @SmallTest @RunWith(AndroidJUnit4.class) @@ -61,7 +62,11 @@ public class NotificationRoundnessManagerTest extends SysuiTestCase { public void setUp() throws Exception { NotificationTestHelper testHelper = new NotificationTestHelper(getContext()); mFirst = testHelper.createRow(); + mFirst.setHeadsUpAnimatingAwayListener(animatingAway + -> mRoundnessManager.onHeadsupAnimatingAwayChanged(mFirst, animatingAway)); mSecond = testHelper.createRow(); + mSecond.setHeadsUpAnimatingAwayListener(animatingAway + -> mRoundnessManager.onHeadsupAnimatingAwayChanged(mSecond, animatingAway)); mRoundnessManager.setOnRoundingChangedCallback(mRoundnessCallback); mRoundnessManager.setAnimatedChildren(mAnimatedChildren); mRoundnessManager.setFirstAndLastBackgroundChild(mFirst, mFirst); @@ -153,4 +158,24 @@ public class NotificationRoundnessManagerTest extends SysuiTestCase { Assert.assertEquals(0.0f, mFirst.getCurrentBottomRoundness(), 0.0f); Assert.assertEquals(0.0f, mFirst.getCurrentTopRoundness(), 0.0f); } + + @Test + public void testRoundingUpdatedWhenAnimatingAwayTrue() { + mRoundnessManager.setExpanded(0.0f, 0.0f); + mRoundnessManager.setFirstAndLastBackgroundChild(mSecond, mSecond); + mFirst.setHeadsUpAnimatingAway(true); + Assert.assertEquals(1.0f, mFirst.getCurrentBottomRoundness(), 0.0f); + Assert.assertEquals(1.0f, mFirst.getCurrentTopRoundness(), 0.0f); + } + + + @Test + public void testRoundingUpdatedWhenAnimatingAwayFalse() { + mRoundnessManager.setExpanded(0.0f, 0.0f); + mRoundnessManager.setFirstAndLastBackgroundChild(mSecond, mSecond); + mFirst.setHeadsUpAnimatingAway(true); + mFirst.setHeadsUpAnimatingAway(false); + Assert.assertEquals(0.0f, mFirst.getCurrentBottomRoundness(), 0.0f); + Assert.assertEquals(0.0f, mFirst.getCurrentTopRoundness(), 0.0f); + } }