From d73fe9b5862446187811e5ff2365d58b6a9e2885 Mon Sep 17 00:00:00 2001 From: Lyn Han Date: Tue, 5 Apr 2022 13:40:22 -0400 Subject: [PATCH 1/2] Rename notification padding to divider height Bug: 227107113 Test: NotificationStackSizeCalculatorTest Change-Id: I6246da9af9b9d565f07a48655f68add011d1f488 --- .../stack/NotificationStackSizeCalculator.kt | 6 +++--- .../stack/NotificationStackSizeCalculatorTest.kt | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt index d68f371035109..297eb8b09d06d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt @@ -54,7 +54,7 @@ constructor( /** * Minimum space between two notifications. There might be more space, see [calculateGapHeight]. */ - private var notificationPadding by notNull() + private var dividerHeight by notNull() init { updateResources() @@ -155,7 +155,7 @@ constructor( maxKeyguardNotifications = infiniteIfNegative(resources.getInteger(R.integer.keyguard_max_notification_count)) - notificationPadding = + dividerHeight = max(1, resources.getDimensionPixelSize(R.dimen.notification_divider_height)) } @@ -178,7 +178,7 @@ constructor( intrinsicHeight.toFloat() } if (visibleIndex != 0) { - size += notificationPadding + size += dividerHeight } val gapHeight = calculateGapHeight(stack, previousView, visibleIndex) log { "\ti=$visibleIndex gapHeight=$gapHeight"} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculatorTest.kt index 497a857d1deb2..dfd70a2e810b0 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculatorTest.kt @@ -57,7 +57,7 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { .thenReturn(GAP_HEIGHT) with(testableResources) { addOverride(R.integer.keyguard_max_notification_count, -1) - addOverride(R.dimen.notification_divider_height, NOTIFICATION_PADDING.toInt()) + addOverride(R.dimen.notification_divider_height, DIVIDER_HEIGHT.toInt()) } sizeCalculator = @@ -109,7 +109,7 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { fun computeMaxKeyguardNotifications_spaceForOne_shelfUsableForLastNotification_returnsTwo() { val rowHeight = ROW_HEIGHT val totalSpaceForEachRow = GAP_HEIGHT + rowHeight - val shelfHeight = totalSpaceForEachRow + NOTIFICATION_PADDING + val shelfHeight = totalSpaceForEachRow + DIVIDER_HEIGHT val spaceForOne = totalSpaceForEachRow val rows = listOf( @@ -127,7 +127,7 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { fun computeMaxKeyguardNotifications_spaceForTwo_returnsTwo() { val rowHeight = ROW_HEIGHT val totalSpaceForEachRow = GAP_HEIGHT + rowHeight - val spaceForTwo = totalSpaceForEachRow * 2 + NOTIFICATION_PADDING + val spaceForTwo = totalSpaceForEachRow * 2 + DIVIDER_HEIGHT val rows = listOf( createMockRow(rowHeight), @@ -143,7 +143,7 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { fun computeHeight_returnsAtMostSpaceAvailable_withGapBeforeShelf() { val rowHeight = ROW_HEIGHT val shelfHeight = SHELF_HEIGHT - val totalSpaceForEachRow = GAP_HEIGHT + rowHeight + NOTIFICATION_PADDING + val totalSpaceForEachRow = GAP_HEIGHT + rowHeight + DIVIDER_HEIGHT val availableSpace = totalSpaceForEachRow * 2 // All rows in separate sections (default setup). @@ -164,7 +164,7 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { fun computeHeight_returnsAtMostSpaceAvailable_noGapBeforeShelf() { val rowHeight = ROW_HEIGHT val shelfHeight = SHELF_HEIGHT - val totalSpaceForEachRow = GAP_HEIGHT + rowHeight + NOTIFICATION_PADDING + val totalSpaceForEachRow = GAP_HEIGHT + rowHeight + DIVIDER_HEIGHT val availableSpace = totalSpaceForEachRow * 1 // Both rows are in the same section. @@ -223,7 +223,7 @@ class NotificationStackSizeCalculatorTest : SysuiTestCase() { /** Default dimensions for tests that don't overwrite them. */ companion object { const val GAP_HEIGHT = 12f - const val NOTIFICATION_PADDING = 3f + const val DIVIDER_HEIGHT = 3f const val SHELF_HEIGHT = 14f const val ROW_HEIGHT = SHELF_HEIGHT * 3 } From 4778c487d1a140884974e00aec3c6b18436ae3ae Mon Sep 17 00:00:00 2001 From: Lyn Han Date: Thu, 7 Apr 2022 18:11:38 -0400 Subject: [PATCH 2/2] Add missing divider height before notification shelf The mismatch between computed height and actual height required caused the notification right before the shelf to be slightly clipped, pushing its icon slightly into the shelf and messing up icon overflow calculations. Bug: 227361193 Test: manually inspect shelf icon placement => see that notification before shelf is no longer incorrectly clipped Change-Id: Icb787f5b141b7df3b3ae801a2d89925d99924034 --- .../stack/NotificationStackSizeCalculator.kt | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt index 297eb8b09d06d..7fb115d21fa5e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackSizeCalculator.kt @@ -52,7 +52,7 @@ constructor( private var maxKeyguardNotifications by notNull() /** - * Minimum space between two notifications. There might be more space, see [calculateGapHeight]. + * Minimum space between two notifications, see [calculateGapAndDividerHeight]. */ private var dividerHeight by notNull() @@ -84,16 +84,20 @@ constructor( val onLockscreen = true val showableRows = children.filter { it.isShowable(onLockscreen) } val showableRowsCount = showableRows.count() + log { "\tshowableRowsCount=$showableRowsCount "} + showableRows.forEachIndexed { i, current -> val spaceNeeded = current.spaceNeeded(count, previous, stack, onLockscreen) + val spaceAfter = remainingSpace - spaceNeeded previous = current - log { "\ti=$i spaceNeeded=$spaceNeeded remainingSpace=$remainingSpace" } + log { "\ti=$i spaceNeeded=$spaceNeeded remainingSpace=$remainingSpace " + + "spaceAfter=$spaceAfter" } if (remainingSpace - spaceNeeded >= 0 && count < maxKeyguardNotifications) { count += 1 remainingSpace -= spaceNeeded } else if (remainingSpace - spaceNeeded > -shelfHeight && i == showableRowsCount - 1) { - log { "Showing all notifications. Shelf is not be needed." } + log { "Show all notifications. Shelf not needed." } // If this is the last one, and it fits using the space shelf would use, then we can // display it, as the shelf will not be needed (as all notifications are shown). return count + 1 @@ -139,8 +143,7 @@ constructor( height += spaceNeeded count += 1 } else { - val gapBeforeFirstViewInShelf = current.calculateGapHeight(stack, previous, count) - height += gapBeforeFirstViewInShelf + height += current.calculateGapAndDividerHeight(stack, previous, count) height += shelfHeight log { "returning height with shelf -> $height" } return height @@ -177,12 +180,7 @@ constructor( } else { intrinsicHeight.toFloat() } - if (visibleIndex != 0) { - size += dividerHeight - } - val gapHeight = calculateGapHeight(stack, previousView, visibleIndex) - log { "\ti=$visibleIndex gapHeight=$gapHeight"} - size += gapHeight + size += calculateGapAndDividerHeight(stack, previousView, visibleIndex) return size } @@ -202,11 +200,17 @@ constructor( return true } - private fun ExpandableView.calculateGapHeight( + private fun ExpandableView.calculateGapAndDividerHeight( stack: NotificationStackScrollLayout, previous: ExpandableView?, visibleIndex: Int - ) = stack.calculateGapHeight(previous, /* current= */ this, visibleIndex) + ) : Float { + var height = stack.calculateGapHeight(previous, /* current= */ this, visibleIndex) + if (visibleIndex != 0) { + height += dividerHeight + } + return height + } /** * Can a view be shown on the lockscreen when calculating the number of allowed notifications to