From 30649549dd72c9fb0b94239d8fe379a856e8bcef Mon Sep 17 00:00:00 2001 From: Lyn Han Date: Thu, 14 Jul 2022 12:54:23 -0400 Subject: Fix dot/icon overlap in notification shelf This bug was caused by an off-by-one error where isOverflowing = iconX > overflowX should have been isOverflowing = iconX >= overflowX As a result, the icon that should have been hidden by the dot was considered "not overflowing" (iconX=overflowX) so it remained visible. This change corrects the definition of isOverflowing; refactors isOverflowing and forceOverflow out of calculateIconXTranslations; adds tests. Bug: 232056690 Test: NotificationIconContainer Test: add notifs to short shelf until overflow swipe notifs away until short shelf empty (no dot/icon overlap) Test: swipe down on ls notif then let go swipe down on ls notif to go to full shade add notifs/swipe away notifs in full shade scroll in full shade with long shelf (no regressions) Change-Id: Ie270c55f6c9d27b5d0fee73c22fedb5335760179 --- .../statusbar/phone/NotificationIconContainer.java | 35 +++++--- .../phone/NotificationIconContainerTest.kt | 100 +++++++++++++++++++++ 2 files changed, 125 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java index a2140c6ab6b7..7b8c5fc998fa 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java @@ -423,6 +423,21 @@ public class NotificationIconContainer extends ViewGroup { + getActualPaddingEnd(); } + @VisibleForTesting + boolean shouldForceOverflow(int i, int speedBumpIndex, float iconAppearAmount, + int maxVisibleIcons) { + return speedBumpIndex != -1 && i >= speedBumpIndex + && iconAppearAmount > 0.0f || i >= maxVisibleIcons; + } + + @VisibleForTesting + boolean isOverflowing(boolean isLastChild, float translationX, float layoutEnd, + float iconSize) { + // Layout end, as used here, does not include padding end. + final float overflowX = isLastChild ? layoutEnd : layoutEnd - iconSize; + return translationX >= overflowX; + } + /** * Calculate the horizontal translations for each notification based on how much the icons * are inserted into the notification container. @@ -448,26 +463,26 @@ public class NotificationIconContainer extends ViewGroup { if (mFirstVisibleIconState == null) { mFirstVisibleIconState = iconState; } - boolean forceOverflow = mSpeedBumpIndex != -1 && i >= mSpeedBumpIndex - && iconState.iconAppearAmount > 0.0f || i >= maxVisibleIcons; - boolean isLastChild = i == childCount - 1; - float drawingScale = mOnLockScreen && view instanceof StatusBarIconView - ? ((StatusBarIconView) view).getIconScaleIncreased() - : 1f; iconState.visibleState = iconState.hidden ? StatusBarIconView.STATE_HIDDEN : StatusBarIconView.STATE_ICON; - final float overflowDotX = layoutEnd - mIconSize; - boolean isOverflowing = translationX > overflowDotX; + final boolean forceOverflow = shouldForceOverflow(i, mSpeedBumpIndex, + iconState.iconAppearAmount, maxVisibleIcons); + final boolean isOverflowing = forceOverflow || isOverflowing( + /* isLastChild= */ i == childCount - 1, translationX, layoutEnd, mIconSize); - if (firstOverflowIndex == -1 && (forceOverflow || isOverflowing)) { - firstOverflowIndex = isLastChild && !forceOverflow ? i - 1 : i; + // First icon to overflow. + if (firstOverflowIndex == -1 && isOverflowing) { + firstOverflowIndex = i; mVisualOverflowStart = layoutEnd - mIconSize; if (forceOverflow || mIsStaticLayout) { mVisualOverflowStart = Math.min(translationX, mVisualOverflowStart); } } + final float drawingScale = mOnLockScreen && view instanceof StatusBarIconView + ? ((StatusBarIconView) view).getIconScaleIncreased() + : 1f; translationX += iconState.iconAppearAmount * view.getWidth() * drawingScale; } mNumDots = 0; diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationIconContainerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationIconContainerTest.kt index 2ff6dd43e84e..086e5df50f3f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationIconContainerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationIconContainerTest.kt @@ -153,6 +153,106 @@ class NotificationIconContainerTest : SysuiTestCase() { assertTrue(iconContainer.hasOverflow()) } + @Test + fun shouldForceOverflow_appearingAboveSpeedBump_true() { + val forceOverflow = iconContainer.shouldForceOverflow( + /* i= */ 1, + /* speedBumpIndex= */ 0, + /* iconAppearAmount= */ 1f, + /* maxVisibleIcons= */ 5 + ) + assertTrue(forceOverflow); + } + + @Test + fun shouldForceOverflow_moreThanMaxVisible_true() { + val forceOverflow = iconContainer.shouldForceOverflow( + /* i= */ 10, + /* speedBumpIndex= */ 11, + /* iconAppearAmount= */ 0f, + /* maxVisibleIcons= */ 5 + ) + assertTrue(forceOverflow); + } + + @Test + fun shouldForceOverflow_belowSpeedBumpAndLessThanMaxVisible_false() { + val forceOverflow = iconContainer.shouldForceOverflow( + /* i= */ 0, + /* speedBumpIndex= */ 11, + /* iconAppearAmount= */ 0f, + /* maxVisibleIcons= */ 5 + ) + assertFalse(forceOverflow); + } + + @Test + fun isOverflowing_lastChildXLessThanLayoutEnd_false() { + val isOverflowing = iconContainer.isOverflowing( + /* isLastChild= */ true, + /* translationX= */ 0f, + /* layoutEnd= */ 10f, + /* iconSize= */ 2f, + ) + assertFalse(isOverflowing) + } + + + @Test + fun isOverflowing_lastChildXEqualToLayoutEnd_true() { + val isOverflowing = iconContainer.isOverflowing( + /* isLastChild= */ true, + /* translationX= */ 10f, + /* layoutEnd= */ 10f, + /* iconSize= */ 2f, + ) + assertTrue(isOverflowing) + } + + @Test + fun isOverflowing_lastChildXGreaterThanLayoutEnd_true() { + val isOverflowing = iconContainer.isOverflowing( + /* isLastChild= */ true, + /* translationX= */ 20f, + /* layoutEnd= */ 10f, + /* iconSize= */ 2f, + ) + assertTrue(isOverflowing) + } + + @Test + fun isOverflowing_notLastChildXLessThanDotX_false() { + val isOverflowing = iconContainer.isOverflowing( + /* isLastChild= */ false, + /* translationX= */ 0f, + /* layoutEnd= */ 10f, + /* iconSize= */ 2f, + ) + assertFalse(isOverflowing) + } + + @Test + fun isOverflowing_notLastChildXGreaterThanDotX_true() { + val isOverflowing = iconContainer.isOverflowing( + /* isLastChild= */ false, + /* translationX= */ 20f, + /* layoutEnd= */ 10f, + /* iconSize= */ 2f, + ) + assertTrue(isOverflowing) + } + + @Test + fun isOverflowing_notLastChildXEqualToDotX_true() { + val isOverflowing = iconContainer.isOverflowing( + /* isLastChild= */ false, + /* translationX= */ 8f, + /* layoutEnd= */ 10f, + /* iconSize= */ 2f, + ) + assertTrue(isOverflowing) + } + private fun mockStatusBarIcon() : StatusBarIconView { val iconView = mock(StatusBarIconView::class.java) whenever(iconView.width).thenReturn(10) -- cgit v1.2.3-59-g8ed1b