From 287842965a2c05a261bce332d975737df16aaab4 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Thu, 9 Feb 2023 21:53:28 +0000 Subject: [PATCH] [Media TTT] Don't use an animated-vector for the loading spinner. See bug for more context. tl;dr: AnimatedVectorDrawables will pause their animation when opening the shade (or in other scenarios), so we need to use a different kind of drawable as a workaround. This CL just uses a static drawable and animates its rotation using an `ObjectAnimator`. Bug: 243983980 Test: `adb shell cmd statusbar media-ttt-chip-sender MyTablet TRANSFER_TO_RECEIVER_TRIGGERED` -> see new loading spinner. Pull down the shade and verify the loading spinner keeps spinning. Plug the device in to charge to see the charging animation and verify the loading spinner keeps spinning. Test: _TRIGGERD -> _SUCCEEDED -> Undo ==> verify that the new loading spinner for the new TRIGGERED state is spinning Test: atest ChipbarCoordinatorTest Change-Id: Ic5701bbb241b919211463df6eb576f810ec2ca22 --- .../res/drawable/ic_progress_activity.xml | 26 +++++ packages/SystemUI/res/layout/chipbar.xml | 7 +- .../chipbar/ChipbarCoordinator.kt | 63 ++++++++++- .../chipbar/ChipbarCoordinatorTest.kt | 100 ++++++++++++++++++ 4 files changed, 187 insertions(+), 9 deletions(-) create mode 100644 packages/SystemUI/res/drawable/ic_progress_activity.xml diff --git a/packages/SystemUI/res/drawable/ic_progress_activity.xml b/packages/SystemUI/res/drawable/ic_progress_activity.xml new file mode 100644 index 0000000000000..abf0625d40d5a --- /dev/null +++ b/packages/SystemUI/res/drawable/ic_progress_activity.xml @@ -0,0 +1,26 @@ + + + + + diff --git a/packages/SystemUI/res/layout/chipbar.xml b/packages/SystemUI/res/layout/chipbar.xml index 8cf4f4de27dab..0ff944c2becf5 100644 --- a/packages/SystemUI/res/layout/chipbar.xml +++ b/packages/SystemUI/res/layout/chipbar.xml @@ -60,14 +60,13 @@ /> - diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt index 696134cde3c97..a20a5b2fdbbca 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinator.kt @@ -16,6 +16,8 @@ package com.android.systemui.temporarydisplay.chipbar +import android.animation.ObjectAnimator +import android.animation.ValueAnimator import android.content.Context import android.graphics.Rect import android.os.PowerManager @@ -27,11 +29,14 @@ import android.view.View.ACCESSIBILITY_LIVE_REGION_NONE import android.view.ViewGroup import android.view.WindowManager import android.view.accessibility.AccessibilityManager +import android.widget.ImageView import android.widget.TextView import androidx.annotation.IdRes +import androidx.annotation.VisibleForTesting import com.android.internal.widget.CachingIconView import com.android.systemui.Gefingerpoken import com.android.systemui.R +import com.android.systemui.animation.Interpolators import com.android.systemui.classifier.FalsingCollector import com.android.systemui.common.shared.model.ContentDescription.Companion.loadContentDescription import com.android.systemui.common.shared.model.Text.Companion.loadText @@ -101,6 +106,15 @@ constructor( private lateinit var parent: ChipbarRootView + /** The current loading information, or null we're not currently loading. */ + @VisibleForTesting + internal var loadingDetails: LoadingDetails? = null + private set(value) { + // Always cancel the old one before updating + field?.animator?.cancel() + field = value + } + override val windowLayoutParams = commonWindowLayoutParams.apply { gravity = Gravity.TOP.or(Gravity.CENTER_HORIZONTAL) } @@ -143,8 +157,22 @@ constructor( // ---- End item ---- // Loading - currentView.requireViewById(R.id.loading).visibility = - (newInfo.endItem == ChipbarEndItem.Loading).visibleIfTrue() + val isLoading = newInfo.endItem == ChipbarEndItem.Loading + val loadingView = currentView.requireViewById(R.id.loading) + loadingView.visibility = isLoading.visibleIfTrue() + + if (isLoading) { + val currentLoadingDetails = loadingDetails + // Since there can be multiple chipbars, we need to check if the loading view is the + // same and possibly re-start the loading animation on the new view. + if (currentLoadingDetails == null || currentLoadingDetails.loadingView != loadingView) { + val newDetails = createLoadingDetails(loadingView) + newDetails.animator.start() + loadingDetails = newDetails + } + } else { + loadingDetails = null + } // Error currentView.requireViewById(R.id.error).visibility = @@ -223,12 +251,17 @@ constructor( override fun animateViewOut(view: ViewGroup, removalReason: String?, onAnimationEnd: Runnable) { val innerView = view.getInnerView() innerView.accessibilityLiveRegion = ACCESSIBILITY_LIVE_REGION_NONE - val removed = chipbarAnimator.animateViewOut(innerView, onAnimationEnd) + + val fullEndRunnable = Runnable { + loadingDetails = null + onAnimationEnd.run() + } + val removed = chipbarAnimator.animateViewOut(innerView, fullEndRunnable) // If the view doesn't get animated, the [onAnimationEnd] runnable won't get run. So, just // run it immediately. if (!removed) { logger.logAnimateOutFailure() - onAnimationEnd.run() + fullEndRunnable.run() } updateGestureListening() @@ -269,7 +302,7 @@ constructor( } private fun ViewGroup.getInnerView(): ViewGroup { - return requireViewById(R.id.chipbar_inner) + return this.requireViewById(R.id.chipbar_inner) } override fun getTouchableRegion(view: View, outRect: Rect) { @@ -283,8 +316,28 @@ constructor( View.GONE } } + + private fun createLoadingDetails(loadingView: View): LoadingDetails { + // Ideally, we would use a view, which would automatically handle the loading + // spinner rotation for us. However, due to b/243983980, the ProgressBar animation + // unexpectedly pauses when SysUI starts another window. ObjectAnimator is a workaround that + // won't pause. + val animator = + ObjectAnimator.ofFloat(loadingView, View.ROTATION, 0f, 360f).apply { + duration = LOADING_ANIMATION_DURATION_MS + repeatCount = ValueAnimator.INFINITE + interpolator = Interpolators.LINEAR + } + return LoadingDetails(loadingView, animator) + } + + internal data class LoadingDetails( + val loadingView: View, + val animator: ObjectAnimator, + ) } @IdRes private val INFO_TAG = R.id.tag_chipbar_info private const val SWIPE_UP_GESTURE_REASON = "SWIPE_UP_GESTURE_DETECTED" private const val TAG = "ChipbarCoordinator" +private const val LOADING_ANIMATION_DURATION_MS = 1000L diff --git a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt index fc7436a6b2731..586bdc6c82153 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/temporarydisplay/chipbar/ChipbarCoordinatorTest.kt @@ -27,6 +27,7 @@ import android.view.WindowManager import android.view.accessibility.AccessibilityManager import android.widget.ImageView import android.widget.TextView +import androidx.core.animation.doOnCancel import androidx.test.filters.SmallTest import com.android.internal.logging.testing.UiEventLoggerFake import com.android.systemui.R @@ -360,6 +361,105 @@ class ChipbarCoordinatorTest : SysuiTestCase() { assertThat(isClicked).isTrue() } + @Test + fun displayView_loading_animationStarted() { + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.id.check_box, null), + Text.Loaded("text"), + endItem = ChipbarEndItem.Loading, + ) + ) + + assertThat(underTest.loadingDetails!!.animator.isStarted).isTrue() + } + + @Test + fun displayView_notLoading_noAnimation() { + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.id.check_box, null), + Text.Loaded("text"), + endItem = ChipbarEndItem.Error, + ) + ) + + assertThat(underTest.loadingDetails).isNull() + } + + @Test + fun displayView_loadingThenNotLoading_animationStopped() { + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.id.check_box, null), + Text.Loaded("text"), + endItem = ChipbarEndItem.Loading, + ) + ) + + val animator = underTest.loadingDetails!!.animator + var cancelled = false + animator.doOnCancel { cancelled = true } + + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.id.check_box, null), + Text.Loaded("text"), + endItem = ChipbarEndItem.Button(Text.Loaded("button")) {}, + ) + ) + + assertThat(cancelled).isTrue() + assertThat(underTest.loadingDetails).isNull() + } + + @Test + fun displayView_loadingThenHideView_animationStopped() { + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.id.check_box, null), + Text.Loaded("text"), + endItem = ChipbarEndItem.Loading, + ) + ) + + val animator = underTest.loadingDetails!!.animator + var cancelled = false + animator.doOnCancel { cancelled = true } + + underTest.removeView(DEVICE_ID, "TestReason") + + assertThat(cancelled).isTrue() + assertThat(underTest.loadingDetails).isNull() + } + + @Test + fun displayView_loadingThenNewLoading_animationStaysTheSame() { + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.id.check_box, null), + Text.Loaded("text"), + endItem = ChipbarEndItem.Loading, + ) + ) + + val animator = underTest.loadingDetails!!.animator + var cancelled = false + animator.doOnCancel { cancelled = true } + + underTest.displayView( + createChipbarInfo( + Icon.Resource(R.id.check_box, null), + Text.Loaded("new text"), + endItem = ChipbarEndItem.Loading, + ) + ) + + assertThat(underTest.loadingDetails!!.animator).isEqualTo(animator) + assertThat(underTest.loadingDetails!!.animator.isStarted).isTrue() + assertThat(cancelled).isFalse() + } + @Test fun displayView_vibrationEffect_doubleClickEffect() { underTest.displayView(