From a3b5e2295b663389dcf97696da279532367d4ce0 Mon Sep 17 00:00:00 2001 From: Michal Brzezinski Date: Tue, 14 Dec 2021 16:07:57 +0000 Subject: Fixing split shade going full black when changing density The source of issue was status bar height defined in px while shelf height is defined in dp. When changing density, shelf height value in px decreases but status bar height remains the same. When density is low enough it means status bar height ends up bigger than shelf height and that messes up the calculation of NSSL's min expansion height. Fixes: 210476328 Test: When having some notifications, change Display size to "Small" in settings -> expand split shade and check it's visible Change-Id: I1e2506f9cf8b21f776a3698f58863ed868da07ba --- .../stack/NotificationStackScrollLayout.java | 8 +++- .../stack/NotificationStackScrollLayoutTest.java | 52 ++++++++++++++++------ 2 files changed, 44 insertions(+), 16 deletions(-) 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 a337cba45f2c..5a8c99e774dc 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 @@ -411,7 +411,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable private NotificationShelf mShelf; private int mMaxDisplayedNotifications = -1; private float mKeyguardBottomPadding = -1; - private int mStatusBarHeight; + @VisibleForTesting int mStatusBarHeight; private int mMinInteractionHeight; private final Rect mClipRect = new Rect(); private boolean mIsClipped; @@ -4849,8 +4849,12 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable @ShadeViewRefactor(RefactorComponent.COORDINATOR) public int getMinExpansionHeight() { + // shelf height is defined in dp but status bar height can be defined in px, that makes + // relation between them variable - sometimes one might be bigger than the other when + // changing density. That’s why we need to ensure we’re not subtracting negative value below return mShelf.getIntrinsicHeight() - - (mShelf.getIntrinsicHeight() - mStatusBarHeight + mWaterfallTopInset) / 2 + - Math.max(0, + (mShelf.getIntrinsicHeight() - mStatusBarHeight + mWaterfallTopInset) / 2) + mWaterfallTopInset; } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java index 185d9cd8733e..326369b8e815 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java @@ -22,6 +22,7 @@ import static android.view.View.GONE; import static com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayout.ROWS_ALL; import static com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayout.ROWS_GENTLE; +import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static junit.framework.Assert.assertEquals; @@ -103,6 +104,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { @Mock private NotificationSwipeHelper mNotificationSwipeHelper; @Mock private NotificationStackScrollLayoutController mStackScrollLayoutController; @Mock private UnlockedScreenOffAnimationController mUnlockedScreenOffAnimationController; + @Mock private NotificationShelf mNotificationShelf; @Before @UiThreadTest @@ -124,13 +126,13 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { mDependency.injectTestDependency(GroupMembershipManager.class, mGroupMembershipManger); mDependency.injectTestDependency(GroupExpansionManager.class, mGroupExpansionManager); mDependency.injectTestDependency(AmbientState.class, mAmbientState); + mDependency.injectTestDependency(NotificationShelf.class, mNotificationShelf); mDependency.injectTestDependency( UnlockedScreenOffAnimationController.class, mUnlockedScreenOffAnimationController); NotificationShelfController notificationShelfController = mock(NotificationShelfController.class); - NotificationShelf notificationShelf = mock(NotificationShelf.class); - when(notificationShelfController.getView()).thenReturn(notificationShelf); + when(notificationShelfController.getView()).thenReturn(mNotificationShelf); when(mNotificationSectionsManager.createSectionsForBuckets()).thenReturn( new NotificationSection[]{ mNotificationSection @@ -160,7 +162,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { anyBoolean()); doNothing().when(mGroupExpansionManager).collapseGroups(); doNothing().when(mExpandHelper).cancelImmediately(); - doNothing().when(notificationShelf).setAnimationsEnabled(anyBoolean()); + doNothing().when(mNotificationShelf).setAnimationsEnabled(anyBoolean()); } @Test @@ -203,21 +205,43 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { @Test @UiThreadTest - public void testSetExpandedHeight_blockingHelperManagerReceivedCallbacks() { - final float[] expectedHeight = {0f}; - final float[] expectedAppear = {0f}; + public void testSetExpandedHeight_listenerReceivedCallbacks() { + final float expectedHeight = 0f; mStackScroller.addOnExpandedHeightChangedListener((height, appear) -> { - Assert.assertEquals(expectedHeight[0], height, 0); - Assert.assertEquals(expectedAppear[0], appear, .1); + Assert.assertEquals(expectedHeight, height, 0); }); - expectedHeight[0] = 1f; - expectedAppear[0] = 1f; - mStackScroller.setExpandedHeight(expectedHeight[0]); + mStackScroller.setExpandedHeight(expectedHeight); + } - expectedHeight[0] = 100f; - expectedAppear[0] = 0f; - mStackScroller.setExpandedHeight(expectedHeight[0]); + @Test + public void testAppearFractionCalculation() { + // appear start position + when(mNotificationShelf.getIntrinsicHeight()).thenReturn(100); + // because it's the same as shelf height, appear start position equals shelf height + mStackScroller.mStatusBarHeight = 100; + // appear end position + when(mEmptyShadeView.getHeight()).thenReturn(200); + + assertEquals(0f, mStackScroller.calculateAppearFraction(100)); + assertEquals(1f, mStackScroller.calculateAppearFraction(200)); + assertEquals(0.5f, mStackScroller.calculateAppearFraction(150)); + } + + @Test + public void testAppearFractionCalculationIsNotNegativeWhenShelfBecomesSmaller() { + // this situation might occur if status bar height is defined in pixels while shelf height + // in dp and screen density changes - appear start position + // (calculated in NSSL#getMinExpansionHeight) that is adjusting for status bar might + // increase and become bigger that end position, which should be prevented + + // appear start position + when(mNotificationShelf.getIntrinsicHeight()).thenReturn(80); + mStackScroller.mStatusBarHeight = 100; + // appear end position + when(mEmptyShadeView.getHeight()).thenReturn(90); + + assertThat(mStackScroller.calculateAppearFraction(100)).isAtLeast(0); } @Test -- cgit v1.2.3-59-g8ed1b