Use immutable shouldBeAnimated during Unfold Animation
Context:
We decide to run animations by using shouldBeAnimated lambda function. It represents mutable state which is combination of the
StatusBar and some external business logic.
It shows that shouldBeAnimated value may change for some reasons after the animation is fired.
This may cause some visual issues like wrong translationX values in the view(animation started but not completed properly).
Solution:
We make shouldBeAnimated immutable variable instead of lambda func in ViewToTranslate and use it during the animation.
We already map `ViewIdToTranslate` to `ViewToTranslate` on each animation start. It also verifies that we invalidate the preserved value on each animation start.
Test: atest UnfoldConstantTranslateAnimatorTest
Bug: 264454435
Change-Id: I75f84a668ab3395193901cff569ca56e55456316
diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimator.kt b/packages/SystemUI/shared/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimator.kt
index 12e0b9a..c5979cc 100644
--- a/packages/SystemUI/shared/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimator.kt
+++ b/packages/SystemUI/shared/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimator.kt
@@ -65,35 +65,40 @@
} else {
1
}
- viewsToTranslate.forEach { (view, direction, shouldBeAnimated) ->
- if (shouldBeAnimated()) {
- view.get()?.translationX = xTrans * direction.multiplier * rtlMultiplier
- }
+ viewsToTranslate.forEach { (view, direction) ->
+ view.get()?.translationX = xTrans * direction.multiplier * rtlMultiplier
}
}
/** Finds in [parent] all views specified by [ids] and register them for the animation. */
private fun registerViewsForAnimation(parent: ViewGroup, ids: Set<ViewIdToTranslate>) {
viewsToTranslate =
- ids.mapNotNull { (id, dir, pred) ->
- parent.findViewById<View>(id)?.let { view ->
- ViewToTranslate(WeakReference(view), dir, pred)
+ ids.asSequence()
+ .filter { it.shouldBeAnimated() }
+ .mapNotNull {
+ parent.findViewById<View>(it.viewId)?.let { view ->
+ ViewToTranslate(WeakReference(view), it.direction)
+ }
}
- }
+ .toList()
}
- /** Represents a view to animate. [rootView] should contain a view with [viewId] inside. */
+ /**
+ * Represents a view to animate. [rootView] should contain a view with [viewId] inside.
+ * [shouldBeAnimated] is only evaluated when the viewsToTranslate is registered in
+ * [registerViewsForAnimation].
+ */
data class ViewIdToTranslate(
val viewId: Int,
val direction: Direction,
val shouldBeAnimated: () -> Boolean = { true }
)
- private data class ViewToTranslate(
- val view: WeakReference<View>,
- val direction: Direction,
- val shouldBeAnimated: () -> Boolean
- )
+ /**
+ * Represents a view whose animation process is in-progress. It should be immutable because the
+ * started animation should be completed.
+ */
+ private data class ViewToTranslate(val view: WeakReference<View>, val direction: Direction)
/** Direction of the animation. */
enum class Direction(val multiplier: Float) {
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java
index 6b4a4d1..be791f9 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java
@@ -2211,10 +2211,10 @@
pw.println("Current Status Bar state:");
pw.println(" mExpandedVisible=" + mShadeController.isExpandedVisible());
pw.println(" mDisplayMetrics=" + mDisplayMetrics);
- pw.println(" mStackScroller: " + CentralSurfaces.viewInfo(mStackScroller));
- pw.println(" mStackScroller: " + CentralSurfaces.viewInfo(mStackScroller)
- + " scroll " + mStackScroller.getScrollX()
+ pw.print(" mStackScroller: " + CentralSurfaces.viewInfo(mStackScroller));
+ pw.print(" scroll " + mStackScroller.getScrollX()
+ "," + mStackScroller.getScrollY());
+ pw.println(" translationX " + mStackScroller.getTranslationX());
}
pw.print(" mInteractingWindows="); pw.println(mInteractingWindows);
diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimatorTest.kt
index 7a74b12..56fc0c7 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimatorTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shared/animation/UnfoldConstantTranslateAnimatorTest.kt
@@ -36,21 +36,26 @@
private val progressProvider = TestUnfoldTransitionProvider()
- @Mock private lateinit var parent: ViewGroup
+ @Mock
+ private lateinit var parent: ViewGroup
+
+ @Mock
+ private lateinit var shouldBeAnimated: () -> Boolean
private lateinit var animator: UnfoldConstantTranslateAnimator
- private val viewsIdToRegister =
- setOf(
- ViewIdToTranslate(START_VIEW_ID, Direction.START),
- ViewIdToTranslate(END_VIEW_ID, Direction.END))
+ private val viewsIdToRegister
+ get() =
+ setOf(
+ ViewIdToTranslate(START_VIEW_ID, Direction.START, shouldBeAnimated),
+ ViewIdToTranslate(END_VIEW_ID, Direction.END, shouldBeAnimated)
+ )
@Before
fun setup() {
MockitoAnnotations.initMocks(this)
-
- animator =
- UnfoldConstantTranslateAnimator(viewsIdToRegister, progressProvider)
+ whenever(shouldBeAnimated.invoke()).thenReturn(true)
+ animator = UnfoldConstantTranslateAnimator(viewsIdToRegister, progressProvider)
animator.init(parent, MAX_TRANSLATION)
}
@@ -96,6 +101,20 @@
moveAndValidate(listOf(leftView to START, rightView to END), View.LAYOUT_DIRECTION_LTR)
}
+ @Test
+ fun onTransition_completeStartedTranslation() {
+ // GIVEN
+ val leftView = View(context)
+ whenever(parent.findViewById<View>(START_VIEW_ID)).thenReturn(leftView)
+ // To start animation, shouldBeAnimated should return true.
+ // There is a possibility for shouldBeAnimated to return false during the animation.
+ whenever(shouldBeAnimated.invoke()).thenReturn(true).thenReturn(false)
+
+ // shouldBeAnimated state may change during the animation.
+ // However, started animation should be completed.
+ moveAndValidate(listOf(leftView to START), View.LAYOUT_DIRECTION_LTR)
+ }
+
private fun moveAndValidate(list: List<Pair<View, Int>>, layoutDirection: Int) {
// Compare values as ints because -0f != 0f
