Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed default ViewState #2802

Merged
merged 2 commits into from
Jan 4, 2022
Merged

Conversation

piaskowyk
Copy link
Member

@piaskowyk piaskowyk commented Jan 4, 2022

Description

The problem occurs since #2651
Why?
Layout animations are disabled by default until someone call layout animation. Then layout animation is enabled. The problem is when someone uses layout animation in components that don't appear in the first render. It breaks the previous assumption of layout animation - "every component is sent to onViewCreate function". In effect, we tried to remove the view that wasn't registered in _viewForTag register.

There was also another problem because uses this:

ViewState state = [_states[tag] intValue];

without checking if _states[tag] is nil. Then nil was converted to 0 and then to the first value from ViewState (Appearing), but this is wrong because the default state should be Inactive because not every component has animation.

I added similar changes for Android also.

Reproduction

import React from 'react';
import { useEffect, useState } from 'react';
import { ActivityIndicator, StyleSheet, Text, View, findNodeHandle } from 'react-native';
import Animated, { FadeOutDown } from 'react-native-reanimated';

export default function App() {
  const [isLoading, setIsLoading] = useState<boolean>(true);
  useEffect(() => {
    const timeout = setTimeout(() => setIsLoading(false), 500);
    return () => {
      clearTimeout(timeout);
    };
  }, []);

  return (
    <View style={styles.container}>
      { isLoading ? 
          <ActivityIndicator size='large' color='red' />
        : 
          <Animated.View exiting={FadeOutDown} style={styles.scrollContainer}>
            {[...Array(10).keys()].map((value) => (
              <View style={styles.row} key={value}>
                <Text>row n°{value}</Text>
              </View>
            ))}
          </Animated.View>
      }
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: '#fff',
    alignItems: 'center',
    justifyContent: 'center',
  },
  scrollContainer: {
    flex: 1,
    width: '100%',
  },
  row: {
    flexDirection: 'row',
    backgroundColor: 'transparent',
    height: 50,
  },
});

Fixes #2758

@piaskowyk piaskowyk requested a review from tomekzaw January 4, 2022 12:50
@piaskowyk piaskowyk self-assigned this Jan 4, 2022
@piaskowyk piaskowyk requested a review from kkafar January 4, 2022 12:50
@piaskowyk
Copy link
Member Author

I need to check it on Android also.

@piaskowyk piaskowyk marked this pull request as draft January 4, 2022 12:58
@piaskowyk piaskowyk marked this pull request as draft January 4, 2022 12:58
@piaskowyk piaskowyk marked this pull request as ready for review January 4, 2022 14:14
Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@piaskowyk piaskowyk merged commit 5061ab6 into main Jan 4, 2022
@piaskowyk piaskowyk deleted the @piaskowyk/change-default-animation-state branch January 4, 2022 15:16
@alexnaiman
Copy link

When will this be published? 😄

@skeie
Copy link

skeie commented Feb 28, 2022

Any idea when this will be released? :)

aeddi pushed a commit to aeddi/react-native-reanimated that referenced this pull request Mar 22, 2022
## Description

The problem occurs since software-mansion#2651
Why?
Layout animations are disabled by default until someone call layout animation. Then layout animation is enabled. The problem is when someone uses layout animation in components that don't appear in the first render. It breaks the previous assumption of layout animation - "every component is sent to `onViewCreate` function". In effect, we tried to remove the view that wasn't registered in `_viewForTag` register.

There was also another problem because uses this:
```c++
ViewState state = [_states[tag] intValue];
```
without checking if `_states[tag]` is `nil`. Then `nil` was converted to `0` and then to the first value from `ViewState` (`Appearing`), but this is wrong because the default state should be `Inactive` because not every component has animation.

I added similar changes for Android also.

## Reproduction
```js
import React from 'react';
import { useEffect, useState } from 'react';
import { ActivityIndicator, StyleSheet, Text, View, findNodeHandle } from 'react-native';
import Animated, { FadeOutDown } from 'react-native-reanimated';

export default function App() {
  const [isLoading, setIsLoading] = useState<boolean>(true);
  useEffect(() => {
    const timeout = setTimeout(() => setIsLoading(false), 500);
    return () => {
      clearTimeout(timeout);
    };
  }, []);

  return (
    <View style={styles.container}>
      { isLoading ? 
          <ActivityIndicator size='large' color='red' />
        : 
          <Animated.View exiting={FadeOutDown} style={styles.scrollContainer}>
            {[...Array(10).keys()].map((value) => (
              <View style={styles.row} key={value}>
                <Text>row n°{value}</Text>
              </View>
            ))}
          </Animated.View>
      }
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: '#fff',
    alignItems: 'center',
    justifyContent: 'center',
  },
  scrollContainer: {
    flex: 1,
    width: '100%',
  },
  row: {
    flexDirection: 'row',
    backgroundColor: 'transparent',
    height: 50,
  },
});
```

Fixes software-mansion#2758
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Layout animation leave orphaned component in Expo
4 participants