From 39cb22695dfeb3043e34694958fb5a8300583a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateo=20Guzm=C3=A1n?= Date: Sun, 16 Mar 2025 18:33:17 +0100 Subject: [PATCH 1/7] Add AbstractLayoutAnimation tests --- .../AbstractLayoutAnimation.java | 10 +- .../AbstractLayoutAnimationTest.kt | 120 ++++++++++++++++++ 2 files changed, 127 insertions(+), 3 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.java index d71bb36e9743..bafa24f89aea 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.java @@ -23,6 +23,7 @@ import com.facebook.react.common.annotations.internal.LegacyArchitectureLogger; import com.facebook.react.uimanager.IllegalViewOperationException; import java.util.Map; +import com.facebook.react.common.annotations.VisibleForTesting; /** * Class responsible for parsing and converting layout animation data into native {@link Animation} @@ -55,8 +56,10 @@ InterpolatorType.EASE_IN, new AccelerateInterpolator(), InterpolatorType.EASE_OUT, new DecelerateInterpolator(), InterpolatorType.EASE_IN_EASE_OUT, new AccelerateDecelerateInterpolator()); - private @Nullable Interpolator mInterpolator; - private int mDelayMs; + @VisibleForTesting + public @Nullable Interpolator mInterpolator; + @VisibleForTesting + public int mDelayMs; protected @Nullable AnimatedPropertyType mAnimatedProperty; protected int mDurationMs; @@ -109,7 +112,8 @@ public void initializeFromConfig(ReadableMap data, int globalDuration) { return animation; } - private static Interpolator getInterpolator(InterpolatorType type, ReadableMap params) { + @VisibleForTesting + public static Interpolator getInterpolator(InterpolatorType type, ReadableMap params) { Interpolator interpolator; if (type.equals(InterpolatorType.SPRING)) { interpolator = diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt new file mode 100644 index 000000000000..f18bd16c20cc --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt @@ -0,0 +1,120 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.uimanager.layoutanimation + +import android.view.View +import android.view.animation.Animation +import com.facebook.react.bridge.ReadableMap +import com.facebook.react.uimanager.IllegalViewOperationException +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import org.junit.Assert.assertThrows + +class AbstractLayoutAnimationTest { + private lateinit var view: View + private lateinit var config: ReadableMap + private lateinit var animation: AbstractLayoutAnimation + + @Before + fun setUp() { + view = mock() + config = mock() + + animation = object : AbstractLayoutAnimation() { + override fun isValid(): Boolean = true + + override fun createAnimationImpl( + view: View, x: Int, y: Int, width: Int, height: Int + ): Animation? { + return mock() + } + } + } + + @Test + fun reset_clearsAnimationProperties() { + animation.reset() + assertThat(animation.mAnimatedProperty).isNull() + assertThat(animation.mDurationMs).isEqualTo(0) + assertThat(animation.mDelayMs).isEqualTo(0) + assertThat(animation.mInterpolator).isNull() + } + + @Test + fun createAnimation_returnsValidAnimation() { + val result = animation.createAnimation(view, 0, 0, 100, 100) + assertThat(result).isNotNull + } + + @Test + fun initializeFromConfig_throwsIfTypeMissing() { + whenever(config.hasKey("type")).thenReturn(false) + + val exception = assertThrows(IllegalArgumentException::class.java) { + animation.initializeFromConfig(config, 300) + } + assertThat(exception.message).isEqualTo("Missing interpolation type.") + } + + @Test + fun createAnimation_returnsNullWhenInvalid() { + val invalidAnimation = object : AbstractLayoutAnimation() { + override fun isValid(): Boolean = false + override fun createAnimationImpl( + view: View, x: Int, y: Int, width: Int, height: Int + ): Animation? = mock() + } + + val result = invalidAnimation.createAnimation(view, 0, 0, 100, 100) + assertThat(result).isNull() + } + + @Test + fun initializeFromConfig_throwsIfInvalidAnimation() { + whenever(config.hasKey("type")).thenReturn(true) + whenever(config.getString("type")).thenReturn("linear") + whenever(config.hasKey("duration")).thenReturn(true) + whenever(config.getInt("duration")).thenReturn(300) + + val invalidAnimation = object : AbstractLayoutAnimation() { + override fun isValid(): Boolean = false + override fun createAnimationImpl( + view: View, x: Int, y: Int, width: Int, height: Int + ): Animation? = mock() + } + + val exception = assertThrows(IllegalViewOperationException::class.java) { + invalidAnimation.initializeFromConfig(config, 300) + } + assertThat(exception.message).contains("Invalid layout animation") + } + + @Test + fun getInterpolator_returnsSpringInterpolator() { + val type = InterpolatorType.SPRING + val params = mock() + whenever(params.hasKey("damping")).thenReturn(true) + whenever(params.getDouble("damping")).thenReturn(0.5) + + val interpolator = AbstractLayoutAnimation.getInterpolator(type, params) + assertThat(interpolator).isInstanceOf(SimpleSpringInterpolator::class.java) + } + + @Test + fun getInterpolator_returnsDefaultInterpolator() { + val type = InterpolatorType.LINEAR + val params = mock() + + val interpolator = AbstractLayoutAnimation.getInterpolator(type, params) + assertThat(interpolator).isNotNull + } +} From d3f2fbd566a22e8db55df048092b9d79b61f8e3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateo=20Guzm=C3=A1n?= Date: Sun, 16 Mar 2025 18:58:10 +0100 Subject: [PATCH 2/7] Migrate AbstractLayoutAnimation to Kotlin --- .../AbstractLayoutAnimation.java | 129 ----------------- .../AbstractLayoutAnimation.kt | 130 ++++++++++++++++++ 2 files changed, 130 insertions(+), 129 deletions(-) delete mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.java create mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.java deleted file mode 100644 index bafa24f89aea..000000000000 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.java +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.uimanager.layoutanimation; - -import android.view.View; -import android.view.animation.AccelerateDecelerateInterpolator; -import android.view.animation.AccelerateInterpolator; -import android.view.animation.Animation; -import android.view.animation.BaseInterpolator; -import android.view.animation.DecelerateInterpolator; -import android.view.animation.Interpolator; -import android.view.animation.LinearInterpolator; -import androidx.annotation.Nullable; -import com.facebook.react.bridge.ReadableMap; -import com.facebook.react.common.MapBuilder; -import com.facebook.react.common.annotations.internal.LegacyArchitecture; -import com.facebook.react.common.annotations.internal.LegacyArchitectureLogLevel; -import com.facebook.react.common.annotations.internal.LegacyArchitectureLogger; -import com.facebook.react.uimanager.IllegalViewOperationException; -import java.util.Map; -import com.facebook.react.common.annotations.VisibleForTesting; - -/** - * Class responsible for parsing and converting layout animation data into native {@link Animation} - * in order to animate layout when a valid configuration has been supplied by the application. - */ -@LegacyArchitecture -/* package */ abstract class AbstractLayoutAnimation { - - static { - LegacyArchitectureLogger.assertWhenLegacyArchitectureMinifyingEnabled( - "AbstractLayoutAnimation", LegacyArchitectureLogLevel.WARNING); - } - - // Forces animation to be playing 10x slower, used for debug purposes. - private static final boolean SLOWDOWN_ANIMATION_MODE = false; - - abstract boolean isValid(); - - /** - * Create an animation object for the current animation type, based on the view and final screen - * coordinates. If the application-supplied configuration does not specify an animation definition - * for this types, or if the animation definition is invalid, returns null. - */ - abstract @Nullable Animation createAnimationImpl(View view, int x, int y, int width, int height); - - private static final Map INTERPOLATOR = - MapBuilder.of( - InterpolatorType.LINEAR, new LinearInterpolator(), - InterpolatorType.EASE_IN, new AccelerateInterpolator(), - InterpolatorType.EASE_OUT, new DecelerateInterpolator(), - InterpolatorType.EASE_IN_EASE_OUT, new AccelerateDecelerateInterpolator()); - - @VisibleForTesting - public @Nullable Interpolator mInterpolator; - @VisibleForTesting - public int mDelayMs; - - protected @Nullable AnimatedPropertyType mAnimatedProperty; - protected int mDurationMs; - - public void reset() { - mAnimatedProperty = null; - mDurationMs = 0; - mDelayMs = 0; - mInterpolator = null; - } - - public void initializeFromConfig(ReadableMap data, int globalDuration) { - mAnimatedProperty = - data.hasKey("property") - ? AnimatedPropertyType.fromString(data.getString("property")) - : null; - mDurationMs = data.hasKey("duration") ? data.getInt("duration") : globalDuration; - mDelayMs = data.hasKey("delay") ? data.getInt("delay") : 0; - if (!data.hasKey("type")) { - throw new IllegalArgumentException("Missing interpolation type."); - } - mInterpolator = getInterpolator(InterpolatorType.fromString(data.getString("type")), data); - - if (!isValid()) { - throw new IllegalViewOperationException("Invalid layout animation : " + data); - } - } - - /** - * Create an animation object to be used to animate the view, based on the animation config - * supplied at initialization time and the new view position and size. - * - * @param view the view to create the animation for - * @param x the new X position for the view - * @param y the new Y position for the view - * @param width the new width value for the view - * @param height the new height value for the view - */ - public final @Nullable Animation createAnimation(View view, int x, int y, int width, int height) { - if (!isValid()) { - return null; - } - Animation animation = createAnimationImpl(view, x, y, width, height); - if (animation != null) { - int slowdownFactor = SLOWDOWN_ANIMATION_MODE ? 10 : 1; - animation.setDuration(mDurationMs * slowdownFactor); - animation.setStartOffset(mDelayMs * slowdownFactor); - animation.setInterpolator(mInterpolator); - } - return animation; - } - - @VisibleForTesting - public static Interpolator getInterpolator(InterpolatorType type, ReadableMap params) { - Interpolator interpolator; - if (type.equals(InterpolatorType.SPRING)) { - interpolator = - new SimpleSpringInterpolator(SimpleSpringInterpolator.getSpringDamping(params)); - } else { - interpolator = INTERPOLATOR.get(type); - } - if (interpolator == null) { - throw new IllegalArgumentException("Missing interpolator for type : " + type); - } - return interpolator; - } -} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt new file mode 100644 index 000000000000..cfce2839e0b1 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt @@ -0,0 +1,130 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.uimanager.layoutanimation + +import android.view.View +import android.view.animation.AccelerateDecelerateInterpolator +import android.view.animation.AccelerateInterpolator +import android.view.animation.Animation +import android.view.animation.BaseInterpolator +import android.view.animation.DecelerateInterpolator +import android.view.animation.Interpolator +import android.view.animation.LinearInterpolator +import com.facebook.react.bridge.ReadableMap +import com.facebook.react.common.annotations.VisibleForTesting +import com.facebook.react.common.annotations.internal.LegacyArchitecture +import com.facebook.react.common.annotations.internal.LegacyArchitectureLogLevel +import com.facebook.react.common.annotations.internal.LegacyArchitectureLogger +import com.facebook.react.uimanager.IllegalViewOperationException + +/** + * Class responsible for parsing and converting layout animation data into native [Animation] + * in order to animate layout when a valid configuration has been supplied by the application. + */ +@LegacyArchitecture +/* package */ +internal abstract class AbstractLayoutAnimation { + internal abstract fun isValid(): Boolean + + /** + * Create an animation object for the current animation type, based on the view and final screen + * coordinates. If the application-supplied configuration does not specify an animation definition + * for this types, or if the animation definition is invalid, returns null. + */ + internal abstract fun createAnimationImpl( + view: View, + x: Int, + y: Int, + width: Int, + height: Int + ): Animation? + + @VisibleForTesting var mInterpolator: Interpolator? = null + @VisibleForTesting var mDelayMs: Int = 0 + @VisibleForTesting var mAnimatedProperty: AnimatedPropertyType? = null + @VisibleForTesting var mDurationMs: Int = 0 + + fun reset() { + mAnimatedProperty = null + mDurationMs = 0 + mDelayMs = 0 + mInterpolator = null + } + + fun initializeFromConfig(data: ReadableMap, globalDuration: Int) { + mAnimatedProperty = + if (data.hasKey("property")) + AnimatedPropertyType.fromString(data.getString("property")!!) + else null + mDurationMs = if (data.hasKey("duration")) data.getInt("duration") else globalDuration + mDelayMs = if (data.hasKey("delay")) data.getInt("delay") else 0 + require(data.hasKey("type")) { "Missing interpolation type." } + mInterpolator = getInterpolator( + InterpolatorType.fromString( + data.getString("type")!! + ), data + ) + + if (!isValid()) { + throw IllegalViewOperationException("Invalid layout animation : $data") + } + } + + /** + * Create an animation object to be used to animate the view, based on the animation config + * supplied at initialization time and the new view position and size. + * + * @param view the view to create the animation for + * @param x the new X position for the view + * @param y the new Y position for the view + * @param width the new width value for the view + * @param height the new height value for the view + */ + fun createAnimation(view: View, x: Int, y: Int, width: Int, height: Int): Animation? { + if (!isValid()) { + return null + } + val animation = createAnimationImpl(view, x, y, width, height) + if (animation != null) { + val slowdownFactor = if (SLOWDOWN_ANIMATION_MODE) 10 else 1 + animation.duration = (mDurationMs * slowdownFactor).toLong() + animation.startOffset = (mDelayMs * slowdownFactor).toLong() + animation.interpolator = mInterpolator + } + return animation + } + + companion object { + init { + LegacyArchitectureLogger.assertWhenLegacyArchitectureMinifyingEnabled( + "AbstractLayoutAnimation", LegacyArchitectureLogLevel.WARNING + ) + } + + // Forces animation to be playing 10x slower, used for debug purposes. + private const val SLOWDOWN_ANIMATION_MODE = false + + private val INTERPOLATOR: Map = mapOf( + InterpolatorType.LINEAR to LinearInterpolator(), + InterpolatorType.EASE_IN to AccelerateInterpolator(), + InterpolatorType.EASE_OUT to DecelerateInterpolator(), + InterpolatorType.EASE_IN_EASE_OUT to AccelerateDecelerateInterpolator() + ) + + @VisibleForTesting + fun getInterpolator(type: InterpolatorType, params: ReadableMap): Interpolator { + val interpolator = if (type == InterpolatorType.SPRING) { + SimpleSpringInterpolator(SimpleSpringInterpolator.getSpringDamping(params)) + } else { + INTERPOLATOR[type] + } + requireNotNull(interpolator) { "Missing interpolator for type : $type" } + return interpolator + } + } +} From 58bf3332161e2727ddf66ceca4f644aaaa566118 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateo=20Guzm=C3=A1n?= Date: Sun, 16 Mar 2025 19:23:55 +0100 Subject: [PATCH 3/7] Adjust hungarian annotation --- .../AbstractLayoutAnimation.kt | 31 +++++++++---------- .../layoutanimation/BaseLayoutAnimation.kt | 4 +-- .../layoutanimation/LayoutUpdateAnimation.kt | 2 +- .../AbstractLayoutAnimationTest.kt | 8 ++--- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt index cfce2839e0b1..553b26b0b07d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt @@ -27,7 +27,6 @@ import com.facebook.react.uimanager.IllegalViewOperationException * in order to animate layout when a valid configuration has been supplied by the application. */ @LegacyArchitecture -/* package */ internal abstract class AbstractLayoutAnimation { internal abstract fun isValid(): Boolean @@ -44,27 +43,27 @@ internal abstract class AbstractLayoutAnimation { height: Int ): Animation? - @VisibleForTesting var mInterpolator: Interpolator? = null - @VisibleForTesting var mDelayMs: Int = 0 - @VisibleForTesting var mAnimatedProperty: AnimatedPropertyType? = null - @VisibleForTesting var mDurationMs: Int = 0 + @VisibleForTesting var interpolator: Interpolator? = null + @VisibleForTesting var delayMs: Int = 0 + @VisibleForTesting var animatedProperty: AnimatedPropertyType? = null + @VisibleForTesting var durationMs: Int = 0 fun reset() { - mAnimatedProperty = null - mDurationMs = 0 - mDelayMs = 0 - mInterpolator = null + animatedProperty = null + durationMs = 0 + delayMs = 0 + interpolator = null } fun initializeFromConfig(data: ReadableMap, globalDuration: Int) { - mAnimatedProperty = + animatedProperty = if (data.hasKey("property")) AnimatedPropertyType.fromString(data.getString("property")!!) else null - mDurationMs = if (data.hasKey("duration")) data.getInt("duration") else globalDuration - mDelayMs = if (data.hasKey("delay")) data.getInt("delay") else 0 + durationMs = if (data.hasKey("duration")) data.getInt("duration") else globalDuration + delayMs = if (data.hasKey("delay")) data.getInt("delay") else 0 require(data.hasKey("type")) { "Missing interpolation type." } - mInterpolator = getInterpolator( + interpolator = getInterpolator( InterpolatorType.fromString( data.getString("type")!! ), data @@ -92,9 +91,9 @@ internal abstract class AbstractLayoutAnimation { val animation = createAnimationImpl(view, x, y, width, height) if (animation != null) { val slowdownFactor = if (SLOWDOWN_ANIMATION_MODE) 10 else 1 - animation.duration = (mDurationMs * slowdownFactor).toLong() - animation.startOffset = (mDelayMs * slowdownFactor).toLong() - animation.interpolator = mInterpolator + animation.duration = (durationMs * slowdownFactor).toLong() + animation.startOffset = (delayMs * slowdownFactor).toLong() + animation.interpolator = interpolator } return animation } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/BaseLayoutAnimation.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/BaseLayoutAnimation.kt index ff4a8107f388..2093712d1e77 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/BaseLayoutAnimation.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/BaseLayoutAnimation.kt @@ -20,10 +20,10 @@ import com.facebook.react.uimanager.IllegalViewOperationException internal abstract class BaseLayoutAnimation : AbstractLayoutAnimation() { abstract fun isReverse(): Boolean - override fun isValid(): Boolean = mDurationMs > 0 && mAnimatedProperty != null + override fun isValid(): Boolean = durationMs > 0 && animatedProperty != null override fun createAnimationImpl(view: View, x: Int, y: Int, width: Int, height: Int): Animation { - mAnimatedProperty?.let { + animatedProperty?.let { return when (it) { AnimatedPropertyType.OPACITY -> { val fromValue = if (isReverse()) view.alpha else 0.0f diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/LayoutUpdateAnimation.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/LayoutUpdateAnimation.kt index 1658a5d939ed..f1c6f6dc89ee 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/LayoutUpdateAnimation.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/LayoutUpdateAnimation.kt @@ -21,7 +21,7 @@ import com.facebook.react.common.annotations.internal.LegacyArchitectureLogger @LegacyArchitecture internal class LayoutUpdateAnimation : AbstractLayoutAnimation() { - internal override fun isValid(): Boolean = mDurationMs > 0 + internal override fun isValid(): Boolean = durationMs > 0 internal override fun createAnimationImpl( view: View, diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt index f18bd16c20cc..f890a9d55231 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt @@ -43,10 +43,10 @@ class AbstractLayoutAnimationTest { @Test fun reset_clearsAnimationProperties() { animation.reset() - assertThat(animation.mAnimatedProperty).isNull() - assertThat(animation.mDurationMs).isEqualTo(0) - assertThat(animation.mDelayMs).isEqualTo(0) - assertThat(animation.mInterpolator).isNull() + assertThat(animation.animatedProperty).isNull() + assertThat(animation.durationMs).isEqualTo(0) + assertThat(animation.delayMs).isEqualTo(0) + assertThat(animation.interpolator).isNull() } @Test From d3ab71c5ee453bc2bfc90c59aea4df007aae8274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateo=20Guzm=C3=A1n?= Date: Sun, 16 Mar 2025 19:34:24 +0100 Subject: [PATCH 4/7] Avoid force casting --- .../uimanager/layoutanimation/AbstractLayoutAnimation.kt | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt index 553b26b0b07d..e22dbd8ed980 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt @@ -58,16 +58,13 @@ internal abstract class AbstractLayoutAnimation { fun initializeFromConfig(data: ReadableMap, globalDuration: Int) { animatedProperty = if (data.hasKey("property")) - AnimatedPropertyType.fromString(data.getString("property")!!) + AnimatedPropertyType.fromString(data.getString("property") ?: "") else null durationMs = if (data.hasKey("duration")) data.getInt("duration") else globalDuration delayMs = if (data.hasKey("delay")) data.getInt("delay") else 0 require(data.hasKey("type")) { "Missing interpolation type." } - interpolator = getInterpolator( - InterpolatorType.fromString( - data.getString("type")!! - ), data - ) + + interpolator = getInterpolator(InterpolatorType.fromString(data.getString("type") ?: ""), data) if (!isValid()) { throw IllegalViewOperationException("Invalid layout animation : $data") From ea6970d11fe4138da747a0601e52486e84be5b62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateo=20Guzm=C3=A1n?= Date: Sun, 16 Mar 2025 20:27:13 +0100 Subject: [PATCH 5/7] Making createAnimation more Kotlin idiomatic --- .../layoutanimation/AbstractLayoutAnimation.kt | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt index e22dbd8ed980..4ac2380f5b4e 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt @@ -42,7 +42,7 @@ internal abstract class AbstractLayoutAnimation { width: Int, height: Int ): Animation? - + @VisibleForTesting var interpolator: Interpolator? = null @VisibleForTesting var delayMs: Int = 0 @VisibleForTesting var animatedProperty: AnimatedPropertyType? = null @@ -85,14 +85,13 @@ internal abstract class AbstractLayoutAnimation { if (!isValid()) { return null } - val animation = createAnimationImpl(view, x, y, width, height) - if (animation != null) { + + return createAnimationImpl(view, x, y, width, height)?.apply { val slowdownFactor = if (SLOWDOWN_ANIMATION_MODE) 10 else 1 - animation.duration = (durationMs * slowdownFactor).toLong() - animation.startOffset = (delayMs * slowdownFactor).toLong() - animation.interpolator = interpolator + duration = (durationMs * slowdownFactor).toLong() + startOffset = (delayMs * slowdownFactor).toLong() + interpolator = interpolator } - return animation } companion object { From a73ed53ff781fcbe0c965f2ad80cf3eaa9577bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateo=20Guzm=C3=A1n?= Date: Wed, 19 Mar 2025 22:13:17 +0100 Subject: [PATCH 6/7] Improve tests and var declarations --- .../layoutanimation/AbstractLayoutAnimation.kt | 10 +++++----- .../layoutanimation/AbstractLayoutAnimationTest.kt | 5 +++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt index 4ac2380f5b4e..7897ece2567a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt @@ -28,6 +28,11 @@ import com.facebook.react.uimanager.IllegalViewOperationException */ @LegacyArchitecture internal abstract class AbstractLayoutAnimation { + @VisibleForTesting var interpolator: Interpolator? = null + @VisibleForTesting var delayMs: Int = 0 + @VisibleForTesting var animatedProperty: AnimatedPropertyType? = null + @VisibleForTesting var durationMs: Int = 0 + internal abstract fun isValid(): Boolean /** @@ -43,11 +48,6 @@ internal abstract class AbstractLayoutAnimation { height: Int ): Animation? - @VisibleForTesting var interpolator: Interpolator? = null - @VisibleForTesting var delayMs: Int = 0 - @VisibleForTesting var animatedProperty: AnimatedPropertyType? = null - @VisibleForTesting var durationMs: Int = 0 - fun reset() { animatedProperty = null durationMs = 0 diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt index f890a9d55231..61e7e86c4edd 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimationTest.kt @@ -9,6 +9,7 @@ package com.facebook.react.uimanager.layoutanimation import android.view.View import android.view.animation.Animation +import android.view.animation.LinearInterpolator import com.facebook.react.bridge.ReadableMap import com.facebook.react.uimanager.IllegalViewOperationException import org.assertj.core.api.Assertions.assertThat @@ -99,7 +100,7 @@ class AbstractLayoutAnimationTest { } @Test - fun getInterpolator_returnsSpringInterpolator() { + fun getInterpolator_returnsSimpleSpringInterpolator() { val type = InterpolatorType.SPRING val params = mock() whenever(params.hasKey("damping")).thenReturn(true) @@ -115,6 +116,6 @@ class AbstractLayoutAnimationTest { val params = mock() val interpolator = AbstractLayoutAnimation.getInterpolator(type, params) - assertThat(interpolator).isNotNull + assertThat(interpolator).isInstanceOf(LinearInterpolator::class.java) } } From 79c69807b5da4913308cb62db97bf62c65eab84a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateo=20Guzm=C3=A1n?= Date: Thu, 20 Mar 2025 19:58:47 +0100 Subject: [PATCH 7/7] Remove unnecessary VisibleForTesting annotations --- .../layoutanimation/AbstractLayoutAnimation.kt | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt index 7897ece2567a..9f692f27f00c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/layoutanimation/AbstractLayoutAnimation.kt @@ -28,10 +28,10 @@ import com.facebook.react.uimanager.IllegalViewOperationException */ @LegacyArchitecture internal abstract class AbstractLayoutAnimation { - @VisibleForTesting var interpolator: Interpolator? = null - @VisibleForTesting var delayMs: Int = 0 - @VisibleForTesting var animatedProperty: AnimatedPropertyType? = null - @VisibleForTesting var durationMs: Int = 0 + var interpolator: Interpolator? = null + var delayMs: Int = 0 + var animatedProperty: AnimatedPropertyType? = null + var durationMs: Int = 0 internal abstract fun isValid(): Boolean @@ -56,10 +56,11 @@ internal abstract class AbstractLayoutAnimation { } fun initializeFromConfig(data: ReadableMap, globalDuration: Int) { - animatedProperty = - if (data.hasKey("property")) + animatedProperty = if (data.hasKey("property")) { AnimatedPropertyType.fromString(data.getString("property") ?: "") - else null + } else { + null + } durationMs = if (data.hasKey("duration")) data.getInt("duration") else globalDuration delayMs = if (data.hasKey("delay")) data.getInt("delay") else 0 require(data.hasKey("type")) { "Missing interpolation type." }