Skip to content

Commit 8fc40a7

Browse files
jingjing2222jingjing2222
committed
Fix Android Fabric Native Animated prop reset
1 parent eeb17ba commit 8fc40a7

5 files changed

Lines changed: 150 additions & 14 deletions

File tree

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/PropsAnimatedNode.kt‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,21 @@ internal class PropsAnimatedNode(
6565
}
6666

6767
fun restoreDefaultValues() {
68-
// In Fabric, we don't restore default values since the FabricUIManager doesn't have access
69-
// to the ShadowNode layer. This method was only relevant for the legacy Paper renderer.
68+
if (connectedViewTag == -1) {
69+
return
70+
}
71+
72+
val defaultPropsMap = JavaOnlyMap()
73+
for ((key, value) in propNodeMapping) {
74+
val node = nativeAnimatedNodesManager.getNodeById(value)
75+
requireNotNull(node) { "Mapped property node does not exist" }
76+
if (node is StyleAnimatedNode) {
77+
node.collectViewDefaultValues(defaultPropsMap)
78+
} else {
79+
defaultPropsMap.putNull(key)
80+
}
81+
}
82+
connectedViewUIManager?.synchronouslyUpdateViewOnUIThread(connectedViewTag, defaultPropsMap)
7083
}
7184

7285
fun updateView() {

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/StyleAnimatedNode.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,11 @@ internal class StyleAnimatedNode(
5858
}
5959
}
6060

61+
fun collectViewDefaultValues(propsMap: JavaOnlyMap) {
62+
for (key in propMapping.keys) {
63+
propsMap.putNull(key)
64+
}
65+
}
66+
6167
override fun prettyPrint(): String = "StyleAnimatedNode[$tag] mPropMapping: $propMapping"
6268
}

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -626,13 +626,14 @@ internal constructor(
626626
public fun storeSynchronousMountPropsOverride(reactTag: Int, props: ReadableMap): Unit {
627627
if (ReactNativeFeatureFlags.overrideBySynchronousMountPropsAtMountingAndroid()) {
628628
val propsMap = getMapFromPropsReadableMap(props)
629-
var synchronousMountProps = tagToSynchronousMountProps[reactTag]
630-
if (synchronousMountProps != null) {
631-
synchronousMountProps.putAll(propsMap)
629+
val synchronousMountProps = tagToSynchronousMountProps[reactTag] ?: mutableMapOf()
630+
removeNullPropsFromPropsReadableMap(props, synchronousMountProps)
631+
synchronousMountProps.putAll(propsMap)
632+
if (synchronousMountProps.isEmpty()) {
633+
tagToSynchronousMountProps.remove(reactTag)
632634
} else {
633-
synchronousMountProps = propsMap
635+
tagToSynchronousMountProps[reactTag] = synchronousMountProps
634636
}
635-
tagToSynchronousMountProps[reactTag] = synchronousMountProps
636637
}
637638
}
638639

@@ -1332,7 +1333,10 @@ internal constructor(
13321333
for ((propKey, propValue) in patchMap) {
13331334
if (outputReadableMap.hasKey(propKey)) {
13341335
if (propKey == PROP_TRANSFORM) {
1335-
assert(outputReadableMap.getType(propKey) == ReadableType.Array && propValue is List<*>)
1336+
val outputType = outputReadableMap.getType(propKey)
1337+
assert(
1338+
(outputType == ReadableType.Array || outputType == ReadableType.Null) &&
1339+
propValue is List<*>)
13361340
val array = WritableNativeArray()
13371341
for (item in propValue as List<*>) {
13381342
if (item is Map<*, *>) {
@@ -1350,7 +1354,10 @@ internal constructor(
13501354
}
13511355
outputReadableMap.putArray(propKey, array)
13521356
} else if (propKey == PROP_OPACITY) {
1353-
assert(outputReadableMap.getType(propKey) == ReadableType.Number && propValue is Number)
1357+
val outputType = outputReadableMap.getType(propKey)
1358+
assert(
1359+
(outputType == ReadableType.Number || outputType == ReadableType.Null) &&
1360+
propValue is Number)
13541361
outputReadableMap.putDouble(propKey, (propValue as Number).toDouble())
13551362
}
13561363
}
@@ -1387,6 +1394,19 @@ internal constructor(
13871394
return outputMap
13881395
}
13891396

1397+
private fun removeNullPropsFromPropsReadableMap(
1398+
readableMap: ReadableMap,
1399+
outputMap: MutableMap<String, Any>,
1400+
) {
1401+
val iterator = readableMap.keySetIterator()
1402+
while (iterator.hasNextKey()) {
1403+
val propKey = iterator.nextKey()
1404+
if (readableMap.getType(propKey) == ReadableType.Null) {
1405+
outputMap.remove(propKey)
1406+
}
1407+
}
1408+
}
1409+
13901410
// prevents unchecked conversion warn of the <ViewGroup> type
13911411
private fun getViewGroupManager(viewState: ViewState): IViewGroupManager<View> {
13921412
val viewManager =

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,7 @@ class NativeAnimatedNodeTraversalTest {
10761076
}
10771077

10781078
@Test
1079-
fun testRestoreDefaultPropsIsNoOp() {
1079+
fun testRestoreDefaultPropsSendsNullUpdate() {
10801080
val viewTag: Int = 1001
10811081
val propsNodeTag = 3
10821082
nativeAnimatedNodesManager.createAnimatedNode(
@@ -1097,7 +1097,9 @@ class NativeAnimatedNodeTraversalTest {
10971097

10981098
reset(uiManagerMock)
10991099
nativeAnimatedNodesManager.restoreDefaultValues(propsNodeTag)
1100-
verify(uiManagerMock, never()).synchronouslyUpdateViewOnUIThread(anyInt(), any<ReadableMap>())
1100+
val stylesCaptor: ArgumentCaptor<ReadableMap> = ArgumentCaptor.forClass(ReadableMap::class.java)
1101+
verify(uiManagerMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture())
1102+
assertThat(stylesCaptor.value.isNull("opacity")).isTrue()
11011103
}
11021104

11031105
/**

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/SurfaceMountingManagerSynchronousMountPropsTest.kt‎

Lines changed: 98 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,17 @@
1010
package com.facebook.react.fabric
1111

1212
import com.facebook.react.ReactRootView
13+
import com.facebook.react.bridge.JavaOnlyArray
1314
import com.facebook.react.bridge.JavaOnlyMap
15+
import com.facebook.react.bridge.ReadableArray
1416
import com.facebook.react.bridge.ReactTestHelper
1517
import com.facebook.react.fabric.mounting.MountingManager
16-
import com.facebook.react.fabric.mounting.MountingManager.MountItemExecutor
1718
import com.facebook.react.fabric.mounting.SurfaceMountingManager
1819
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
1920
import com.facebook.react.uimanager.ThemedReactContext
2021
import com.facebook.react.uimanager.ViewManager
2122
import com.facebook.react.uimanager.ViewManagerRegistry
23+
import com.facebook.react.views.view.ReactViewGroup
2224
import com.facebook.react.views.view.ReactViewManager
2325
import com.facebook.testutils.shadows.ShadowNativeLoader
2426
import com.facebook.testutils.shadows.ShadowNativeMap
@@ -67,8 +69,8 @@ class SurfaceMountingManagerSynchronousMountPropsTest {
6769
themedReactContext = ThemedReactContext(reactContext, reactContext, null, -1)
6870
mountingManager =
6971
MountingManager(
70-
ViewManagerRegistry(listOf<ViewManager<*, *>>(ReactViewManager())),
71-
MountItemExecutor {},
72+
ViewManagerRegistry(listOf<ViewManager<*, *>>(TestReactViewManager())),
73+
{},
7274
)
7375
}
7476

@@ -83,6 +85,25 @@ class SurfaceMountingManagerSynchronousMountPropsTest {
8385
smm.addViewAt(surfaceId, tag, 0)
8486
}
8587

88+
private class TestReactViewManager : ReactViewManager() {
89+
override fun setTransformProperty(
90+
view: ReactViewGroup,
91+
transforms: ReadableArray?,
92+
transformOrigin: ReadableArray?,
93+
) {
94+
view.translationY = 0.0f
95+
if (transforms == null) {
96+
return
97+
}
98+
for (i in 0..<transforms.size()) {
99+
val transform = transforms.getMap(i) ?: continue
100+
if (transform.hasKey("translateY")) {
101+
view.translationY = transform.getDouble("translateY").toFloat()
102+
}
103+
}
104+
}
105+
}
106+
86107
/** Stored synchronous opacity should override a stale Fabric mount update. */
87108
@Test
88109
fun storeSynchronousProps_overridesStaleOpacityInUpdateProps() {
@@ -137,6 +158,80 @@ class SurfaceMountingManagerSynchronousMountPropsTest {
137158
assertThat(smm.getView(tag).alpha).isEqualTo(0.2f)
138159
}
139160

161+
/** A null Fabric prop update should not clear the stored synchronous opacity override. */
162+
@Test
163+
fun updateProps_withNullOpacity_keepsStoredSynchronousProp() {
164+
val smm = startSurface()
165+
val tag = 42
166+
createAndAttachView(smm, tag)
167+
168+
smm.storeSynchronousMountPropsOverride(tag, JavaOnlyMap.of("opacity", 0.3))
169+
smm.updatePropsSynchronously(tag, JavaOnlyMap.of("opacity", 0.3))
170+
assertThat(smm.getView(tag).alpha).isEqualTo(0.3f)
171+
172+
smm.updateProps(tag, JavaOnlyMap.of("opacity", null))
173+
174+
assertThat(smm.getView(tag).alpha).isEqualTo(0.3f)
175+
}
176+
177+
/** A synchronous null update should clear the stored synchronous opacity override. */
178+
@Test
179+
fun updatePropsSynchronously_withNullOpacity_removesStoredSynchronousProp() {
180+
val smm = startSurface()
181+
val tag = 42
182+
createAndAttachView(smm, tag)
183+
184+
smm.storeSynchronousMountPropsOverride(tag, JavaOnlyMap.of("opacity", 0.3))
185+
smm.updatePropsSynchronously(tag, JavaOnlyMap.of("opacity", 0.3))
186+
assertThat(smm.getView(tag).alpha).isEqualTo(0.3f)
187+
188+
smm.storeSynchronousMountPropsOverride(tag, JavaOnlyMap.of("opacity", null))
189+
smm.updatePropsSynchronously(tag, JavaOnlyMap.of("opacity", null))
190+
assertThat(smm.getView(tag).alpha).isEqualTo(1.0f)
191+
192+
smm.updateProps(tag, JavaOnlyMap.of("opacity", null))
193+
194+
assertThat(smm.getView(tag).alpha).isEqualTo(1.0f)
195+
}
196+
197+
/** A null Fabric prop update should not clear the stored synchronous transform override. */
198+
@Test
199+
fun updateProps_withNullTransform_keepsStoredSynchronousProp() {
200+
val smm = startSurface()
201+
val tag = 42
202+
val transform = JavaOnlyArray.of(JavaOnlyMap.of("translateY", 40.0))
203+
createAndAttachView(smm, tag)
204+
205+
smm.storeSynchronousMountPropsOverride(tag, JavaOnlyMap.of("transform", transform))
206+
smm.updatePropsSynchronously(tag, JavaOnlyMap.of("transform", transform))
207+
assertThat(smm.getView(tag).translationY).isEqualTo(40.0f)
208+
209+
smm.updateProps(tag, JavaOnlyMap.of("transform", null))
210+
211+
assertThat(smm.getView(tag).translationY).isEqualTo(40.0f)
212+
}
213+
214+
/** A synchronous null update should clear the stored synchronous transform override. */
215+
@Test
216+
fun updatePropsSynchronously_withNullTransform_removesStoredSynchronousProp() {
217+
val smm = startSurface()
218+
val tag = 42
219+
val transform = JavaOnlyArray.of(JavaOnlyMap.of("translateY", 40.0))
220+
createAndAttachView(smm, tag)
221+
222+
smm.storeSynchronousMountPropsOverride(tag, JavaOnlyMap.of("transform", transform))
223+
smm.updatePropsSynchronously(tag, JavaOnlyMap.of("transform", transform))
224+
assertThat(smm.getView(tag).translationY).isEqualTo(40.0f)
225+
226+
smm.storeSynchronousMountPropsOverride(tag, JavaOnlyMap.of("transform", null))
227+
smm.updatePropsSynchronously(tag, JavaOnlyMap.of("transform", null))
228+
assertThat(smm.getView(tag).translationY).isEqualTo(0.0f)
229+
230+
smm.updateProps(tag, JavaOnlyMap.of("transform", null))
231+
232+
assertThat(smm.getView(tag).translationY).isEqualTo(0.0f)
233+
}
234+
140235
/**
141236
* When a view is deleted, stored synchronous props should be cleaned up. A recreated view with
142237
* the same tag should not be affected by the old stored props.

0 commit comments

Comments
 (0)