Merge "[Media TTT] Don't use an animated-vector for the loading spinner." into tm-qpr-dev am: 9f085a9111

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/21357980 Change-Id: If3db252a475969fc341e6800822d3c96a8dc550e Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
2023-02-13 18:41:25 +00:00
parent 468f6a4922 9f085a9111
commit 7f74e1014e
4 changed files with 187 additions and 9 deletions
--- a/packages/SystemUI/res/drawable/ic_progress_activity.xml
+++ b/packages/SystemUI/res/drawable/ic_progress_activity.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+     Copyright (C) 2023 The Android Open Source Project
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+          http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+-->
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+    android:width="48dp"
+    android:height="48dp"
+    android:viewportWidth="48"
+    android:viewportHeight="48"
+    android:tint="?attr/colorControlNormal">
+    <path
+        android:fillColor="@android:color/white"
+        android:pathData="M24,44Q19.8,44 16.15,42.45Q12.5,40.9 9.8,38.2Q7.1,35.5 5.55,31.85Q4,28.2 4,24Q4,19.8 5.55,16.15Q7.1,12.5 9.8,9.8Q12.5,7.1 16.15,5.55Q19.8,4 24,4Q24.6,4 25.05,4.45Q25.5,4.9 25.5,5.5Q25.5,6.1 25.05,6.55Q24.6,7 24,7Q16.95,7 11.975,11.975Q7,16.95 7,24Q7,31.05 11.975,36.025Q16.95,41 24,41Q31.05,41 36.025,36.025Q41,31.05 41,24Q41,23.4 41.45,22.95Q41.9,22.5 42.5,22.5Q43.1,22.5 43.55,22.95Q44,23.4 44,24Q44,28.2 42.45,31.85Q40.9,35.5 38.2,38.2Q35.5,40.9 31.85,42.45Q28.2,44 24,44Z"/>
+</vector>
--- a/packages/SystemUI/res/layout/chipbar.xml
+++ b/packages/SystemUI/res/layout/chipbar.xml
@@ -60,14 +60,13 @@
            />

        <!-- At most one of [loading, failure_icon, undo] will be visible at a time. -->
-        <ProgressBar
+        <ImageView
            android:id="@+id/loading"
-            android:indeterminate="true"
            android:layout_width="@dimen/media_ttt_status_icon_size"
            android:layout_height="@dimen/media_ttt_status_icon_size"
            android:layout_marginStart="@dimen/media_ttt_last_item_start_margin"
-            android:indeterminateTint="?androidprv:attr/colorAccentPrimaryVariant"
-            style="?android:attr/progressBarStyleSmall"
+            android:src="@drawable/ic_progress_activity"
+            android:tint="?androidprv:attr/colorAccentPrimaryVariant"
            android:alpha="0.0"
            />

--- 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<View>(R.id.loading).visibility =
-            (newInfo.endItem == ChipbarEndItem.Loading).visibleIfTrue()
+        val isLoading = newInfo.endItem == ChipbarEndItem.Loading
+        val loadingView = currentView.requireViewById<ImageView>(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<View>(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 <ProgressBar> 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
--- 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(