react-native-reanimated icon indicating copy to clipboard operation
react-native-reanimated copied to clipboard

Fix animation not running on mount

Open graszka22 opened this issue 2 years ago • 5 comments

Description

This PR fixes a bug where animation is not always run on component mount.

Fixes #3296 .

Changes

Probably there is a race condition where the component is mounted and maybeFlushUpdateBuffer is called but then isNotNativeViewFullyMounted:viewTag returns that it's not yet mounted. isNotNativeViewFullyMounted relies on UIView's superview. Then on the next frame it returns it's mounted (because iOS updated superview?) but the current code just updates the buffer and doesn't apply the changes to the view. It assumes that maybeFlushUpdateBuffer will be called but it was called already and won't be called when animation is running.

I added logic covering that case: if component is mounted (has superview) and has an update buffer, apply new props to the buffer, remove buffer for that view from _componentUpdateBuffer dict and run updateProps again with the new props.

Test code and steps to reproduce

Test on iOS on Paper architecture. Press the top button a few times, the red square sometimes doesn't move.

import React, { useState, useEffect } from 'react';
import { Text, View, StyleSheet, TouchableOpacity } from 'react-native';
import Animated, { useAnimatedStyle, useDerivedValue, useSharedValue, withSpring} from 'react-native-reanimated'

const AnimatedLine = React.memo(() => {
  const containerWidth = useSharedValue(0)
  const pctAnim = useSharedValue(0)
  const percentage = 50

  useEffect(() => {
    pctAnim.value = percentage
  }, [percentage, pctAnim])

  const animatedPct = useDerivedValue(() => {
    return withSpring(pctAnim.value, { stiffness: 200, damping: 1000 })
  }, [pctAnim])

  const animStyle = useAnimatedStyle(() => {
    const translateX = (animatedPct.value / 100) * containerWidth.value
    return {
      transform: [{ translateX }],
    }
  }, [])

  return (
    <View style={styles.chartItem}>
      <View
        style={styles.graphics}
        onLayout={({ nativeEvent: { layout } }) => {
          containerWidth.value = layout.width
        }}
      >
        <Animated.View style={[styles.iconContainer, animStyle]}>
          <View style={{ right: 20, width: 34, height: 34, backgroundColor: 'tomato' }} />
        </Animated.View>
      </View>
    </View>
  )
})

const styles = StyleSheet.create({
  chartItem: {
    flexDirection: 'row',
    alignItems: 'center',
  },
  graphics: {
    flex: 1,
  },
  iconContainer: {
    zIndex: 1,
    position: 'absolute',
    top: 0,
    bottom: 0,
    justifyContent: 'center',
  },
})

export default function App() {
  const [hide, setHide] = useState(false);

  return (
    <View style={{ flex: 1 }}>
      <TouchableOpacity
        onPress={() => setHide((prev) => !prev)}
        style={{ padding: 10, backgroundColor: 'gray', marginTop: 50 }}>
        <Text>Toggle</Text>
      </TouchableOpacity>
      {!hide && (
        <>
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
          <AnimatedLine />
        </>
      )}
    </View>
  );
}

Checklist

  • [ ] Included code example that can be used to test this change
  • [ ] Updated TS types
  • [ ] Added TS types tests
  • [ ] Added unit / integration tests
  • [ ] Updated documentation
  • [ ] Ensured that CI passes

graszka22 avatar Jun 20 '22 18:06 graszka22

Doesn't work when applied on 2.8.0 (or it addresses another bug than the one I'm having)

Titozzz avatar Jun 21 '22 16:06 Titozzz

@Titozzz do you mean this bug: https://github.com/software-mansion/react-native-reanimated/issues/3209? I haven't looked deeply into that one yet but it looks to me it's a different issue. This one is about animations running on initialisation, that one is about merging styles.

graszka22 avatar Jun 22 '22 18:06 graszka22

My understanding of my issue is that if I nest array of styles like [{width}, [useAnimatedStyle1, useAnimatedStyle2]], the first useAnimatedStyle will not be merged with the second one, but first render only, so I end up missing a style for no reason.

So it's a bit in between 😅. My bug was 100% reproducible on iOS and Android, and if I would use console.log inside style1 it would then be correctly merged ...

Dark magic... 🧙

Titozzz avatar Jun 22 '22 18:06 Titozzz

Any updates on getting this in? We're running into #3296 I think and would love to get it fixed if possible.

grantmagdanz avatar Jul 26 '22 16:07 grantmagdanz

This patch file applies the same changes as the PR for v2.9.1, you can apply it using patch-package, it should fix #3296 :

diff --git a/node_modules/react-native-reanimated/ios/REANodesManager.m b/node_modules/react-native-reanimated/ios/REANodesManager.m
index f40467d..9977be0 100644
--- a/node_modules/react-native-reanimated/ios/REANodesManager.m
+++ b/node_modules/react-native-reanimated/ios/REANodesManager.m
@@ -537,12 +537,18 @@
       propsSnapshot.viewName = viewName;
       _componentUpdateBuffer[viewTag] = propsSnapshot;
       atomic_store(&_shouldFlushUpdateBuffer, true);
+      return;
     } else {
       NSMutableDictionary *lastProps = lastSnapshot.props;
       for (NSString *key in props) {
         [lastProps setValue:props[key] forKey:key];
       }
     }
+  }
+
+  if (lastSnapshot != nil) {
+    [_componentUpdateBuffer removeObjectForKey:viewTag];
+    [self updateProps:lastSnapshot.props ofViewWithTag:viewTag withName:viewName];
     return;
   }
 

inanc-g avatar Oct 06 '22 15:10 inanc-g

This patch file applies the same changes as the PR for v2.9.1, you can apply it using patch-package, it should fix #3296 :

diff --git a/node_modules/react-native-reanimated/ios/REANodesManager.m b/node_modules/react-native-reanimated/ios/REANodesManager.m
index f40467d..9977be0 100644
--- a/node_modules/react-native-reanimated/ios/REANodesManager.m
+++ b/node_modules/react-native-reanimated/ios/REANodesManager.m
@@ -537,12 +537,18 @@
       propsSnapshot.viewName = viewName;
       _componentUpdateBuffer[viewTag] = propsSnapshot;
       atomic_store(&_shouldFlushUpdateBuffer, true);
+      return;
     } else {
       NSMutableDictionary *lastProps = lastSnapshot.props;
       for (NSString *key in props) {
         [lastProps setValue:props[key] forKey:key];
       }
     }
+  }
+
+  if (lastSnapshot != nil) {
+    [_componentUpdateBuffer removeObjectForKey:viewTag];
+    [self updateProps:lastSnapshot.props ofViewWithTag:viewTag withName:viewName];
     return;
   }
 

it doesn't fix the issue for me

andreibahachenka avatar Oct 17 '22 09:10 andreibahachenka