From f68c0a15c1c120f401c824b93952bc8d1c3cbf13 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Thu, 14 Dec 2023 13:43:15 -0500 Subject: [PATCH] Fix NotifKeyguardViewStateRepo#isPulseExpanding We were treating the NotificationWakeUpCoordinator#onPulseExpansionChanged callback as a notification that expansion has started or stopped, when in reality it was a notification that the actual expansion progress has changed; the boolean parameter does not reflect that the expansion is occurring, but whether or not the _fact_ that an expansion is occuring has changed. The fix is as follows: override fun onPulseExpansionChanged(expandingChanged: Boolean) { // Before trySend(expandingChanged) // After if (expandingChanged) trySend(wakeUpCoordinator.isPulseExpanding) } Rather than just fix this in the NotificationsKeyguardViewStateRepository, the API of `onPulseExpansionChanged(expandingChanged: Boolean)` is replaced with the more-idiomatic and (ideally) less error-prone `onPulseExpandedChanged(isPulseExpanded: Boolean)`, whose boolean argument specifies the new value of NotificationWakeUpCoordinator.isPulseExpanding() Flag: ACONFIG com.android.systemui.notifications_icon_container_refactor DEVELOPMENT Test: atest SystemUITests Fixes: 316116909 Change-Id: I8ef78ffd69de3589d1b948df5034d44f51485f7f --- .../ui/binder/KeyguardRootViewBinder.kt | 21 ++++++++--------- .../NotificationPanelViewController.java | 2 +- .../NotificationWakeUpCoordinator.kt | 23 +++++++++++++++---- ...otificationsKeyguardViewStateRepository.kt | 4 ++-- ...acyNotificationIconAreaControllerImpl.java | 2 +- .../phone/NotificationIconContainer.java | 21 +++++++++++++---- ...icationsKeyguardViewStateRepositoryTest.kt | 2 +- 7 files changed, 49 insertions(+), 26 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardRootViewBinder.kt b/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardRootViewBinder.kt index 01a1ca3eeb93..08e2a8fb6d77 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardRootViewBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/ui/binder/KeyguardRootViewBinder.kt @@ -393,7 +393,6 @@ object KeyguardRootViewBinder { iconsAppearTranslationPx: Int, screenOffAnimationController: ScreenOffAnimationController, ) { - val statusViewMigrated = KeyguardShadeMigrationNssl.isEnabled animate().cancel() val animatorListener = object : AnimatorListenerAdapter() { @@ -404,13 +403,13 @@ object KeyguardRootViewBinder { when { !isVisible.isAnimating -> { alpha = 1f - if (!statusViewMigrated) { + if (!KeyguardShadeMigrationNssl.isEnabled) { translationY = 0f } visibility = if (isVisible.value) View.VISIBLE else View.INVISIBLE } newAodTransition() -> { - animateInIconTranslation(statusViewMigrated) + animateInIconTranslation() if (isVisible.value) { CrossFadeHelper.fadeIn(this, animatorListener) } else { @@ -419,7 +418,7 @@ object KeyguardRootViewBinder { } !isVisible.value -> { // Let's make sure the icon are translated to 0, since we cancelled it above - animateInIconTranslation(statusViewMigrated) + animateInIconTranslation() CrossFadeHelper.fadeOut(this, animatorListener) } visibility != View.VISIBLE -> { @@ -429,13 +428,12 @@ object KeyguardRootViewBinder { appearIcons( animate = screenOffAnimationController.shouldAnimateAodIcons(), iconsAppearTranslationPx, - statusViewMigrated, animatorListener, ) } else -> { // Let's make sure the icons are translated to 0, since we cancelled it above - animateInIconTranslation(statusViewMigrated) + animateInIconTranslation() // We were fading out, let's fade in instead CrossFadeHelper.fadeIn(this, animatorListener) } @@ -445,11 +443,10 @@ object KeyguardRootViewBinder { private fun View.appearIcons( animate: Boolean, iconAppearTranslation: Int, - statusViewMigrated: Boolean, animatorListener: Animator.AnimatorListener, ) { if (animate) { - if (!statusViewMigrated) { + if (!KeyguardShadeMigrationNssl.isEnabled) { translationY = -iconAppearTranslation.toFloat() } alpha = 0f @@ -457,19 +454,19 @@ object KeyguardRootViewBinder { .alpha(1f) .setInterpolator(Interpolators.LINEAR) .setDuration(AOD_ICONS_APPEAR_DURATION) - .apply { if (statusViewMigrated) animateInIconTranslation() } + .apply { if (KeyguardShadeMigrationNssl.isEnabled) animateInIconTranslation() } .setListener(animatorListener) .start() } else { alpha = 1.0f - if (!statusViewMigrated) { + if (!KeyguardShadeMigrationNssl.isEnabled) { translationY = 0f } } } - private fun View.animateInIconTranslation(statusViewMigrated: Boolean) { - if (!statusViewMigrated) { + private fun View.animateInIconTranslation() { + if (!KeyguardShadeMigrationNssl.isEnabled) { animate().animateInIconTranslation().setDuration(AOD_ICONS_APPEAR_DURATION).start() } } diff --git a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java index 95f7c94a235f..878e6faf32e7 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java @@ -1096,7 +1096,7 @@ public final class NotificationPanelViewController implements ShadeSurface, Dump } @Override - public void onPulseExpansionChanged(boolean expandingChanged) { + public void onPulseExpansionAmountChanged(boolean expandingChanged) { if (mKeyguardBypassController.getBypassEnabled()) { // Position the notifications while dragging down while pulsing requestScrollerTopPaddingUpdate(false /* animate */); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationWakeUpCoordinator.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationWakeUpCoordinator.kt index 8d1e8d0ab524..0c67279c1660 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationWakeUpCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationWakeUpCoordinator.kt @@ -20,9 +20,9 @@ import android.util.FloatProperty import android.view.animation.Interpolator import androidx.annotation.VisibleForTesting import androidx.core.animation.ObjectAnimator -import com.android.systemui.Dumpable import com.android.app.animation.Interpolators import com.android.app.animation.InterpolatorsAndroidX +import com.android.systemui.Dumpable import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dump.DumpManager import com.android.systemui.plugins.statusbar.StatusBarStateController @@ -31,6 +31,7 @@ import com.android.systemui.shade.ShadeExpansionListener import com.android.systemui.shade.ShadeViewController import com.android.systemui.statusbar.StatusBarState import com.android.systemui.statusbar.notification.collection.NotificationEntry +import com.android.systemui.statusbar.notification.shared.NotificationIconContainerRefactor import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutController import com.android.systemui.statusbar.notification.stack.StackStateAnimator import com.android.systemui.statusbar.phone.DozeParameters @@ -206,8 +207,15 @@ constructor( val nowExpanding = isPulseExpanding() val changed = nowExpanding != pulseExpanding pulseExpanding = nowExpanding - for (listener in wakeUpListeners) { - listener.onPulseExpansionChanged(changed) + if (!NotificationIconContainerRefactor.isEnabled) { + for (listener in wakeUpListeners) { + listener.onPulseExpansionAmountChanged(changed) + } + } + if (changed) { + for (listener in wakeUpListeners) { + listener.onPulseExpandingChanged(pulseExpanding) + } } } } @@ -620,13 +628,20 @@ constructor( * * @param expandingChanged if the user has started or stopped expanding */ - fun onPulseExpansionChanged(expandingChanged: Boolean) {} + @Deprecated( + message = "Use onPulseExpandedChanged instead.", + replaceWith = ReplaceWith("onPulseExpandedChanged"), + ) + fun onPulseExpansionAmountChanged(expandingChanged: Boolean) {} /** * Called when the animator started by [scheduleDelayedDozeAmountAnimation] begins running * after the start delay, or after it ends/is cancelled. */ fun onDelayedDozeAmountAnimationRunning(running: Boolean) {} + + /** Called whenever a pulse has started or stopped expanding. */ + fun onPulseExpandingChanged(isPulseExpanding: Boolean) {} } companion object { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/data/repository/NotificationsKeyguardViewStateRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/data/repository/NotificationsKeyguardViewStateRepository.kt index cf03d1c5addc..2cc1403a80a5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/data/repository/NotificationsKeyguardViewStateRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/data/repository/NotificationsKeyguardViewStateRepository.kt @@ -62,8 +62,8 @@ constructor( override val isPulseExpanding: Flow = conflatedCallbackFlow { val listener = object : NotificationWakeUpCoordinator.WakeUpListener { - override fun onPulseExpansionChanged(expandingChanged: Boolean) { - trySend(expandingChanged) + override fun onPulseExpandingChanged(isPulseExpanding: Boolean) { + trySend(isPulseExpanding) } } trySend(wakeUpCoordinator.isPulseExpanding()) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LegacyNotificationIconAreaControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LegacyNotificationIconAreaControllerImpl.java index a62a1ed9f0c0..e79f3ff19031 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/LegacyNotificationIconAreaControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/LegacyNotificationIconAreaControllerImpl.java @@ -608,7 +608,7 @@ public class LegacyNotificationIconAreaControllerImpl implements } @Override - public void onPulseExpansionChanged(boolean expandingChanged) { + public void onPulseExpansionAmountChanged(boolean expandingChanged) { if (expandingChanged) { updateAodIconsVisibility(true /* animate */, false /* force */); } 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 0dabafbdecb0..f34a44a5c4b0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationIconContainer.java @@ -284,11 +284,22 @@ public class NotificationIconContainer extends ViewGroup { @Override public String toString() { - return "NotificationIconContainer(" - + "dozing=" + mDozing + " onLockScreen=" + mOnLockScreen - + " overrideIconColor=" + mOverrideIconColor - + " speedBumpIndex=" + mSpeedBumpIndex - + " themedTextColorPrimary=#" + Integer.toHexString(mThemedTextColorPrimary) + ')'; + if (NotificationIconContainerRefactor.isEnabled()) { + return super.toString() + + " {" + + " overrideIconColor=" + mOverrideIconColor + + ", maxIcons=" + mMaxIcons + + ", isStaticLayout=" + mIsStaticLayout + + ", themedTextColorPrimary=#" + Integer.toHexString(mThemedTextColorPrimary) + + " }"; + } else { + return "NotificationIconContainer(" + + "dozing=" + mDozing + " onLockScreen=" + mOnLockScreen + + " overrideIconColor=" + mOverrideIconColor + + " speedBumpIndex=" + mSpeedBumpIndex + + " themedTextColorPrimary=#" + Integer.toHexString(mThemedTextColorPrimary) + + ')'; + } } @VisibleForTesting diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/data/repository/NotificationsKeyguardViewStateRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/data/repository/NotificationsKeyguardViewStateRepositoryTest.kt index f3094cdd4faf..170f651aed91 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/data/repository/NotificationsKeyguardViewStateRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/data/repository/NotificationsKeyguardViewStateRepositoryTest.kt @@ -80,7 +80,7 @@ class NotificationsKeyguardViewStateRepositoryTest : SysuiTestCase() { assertThat(isPulseExpanding).isFalse() withArgCaptor { verify(mockWakeUpCoordinator).addListener(capture()) } - .onPulseExpansionChanged(true) + .onPulseExpandingChanged(true) runCurrent() assertThat(isPulseExpanding).isTrue()