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

Disable by default layout animation #2651

Merged
merged 8 commits into from
Nov 23, 2021

Conversation

piaskowyk
Copy link
Member

@piaskowyk piaskowyk commented Nov 22, 2021

Description

I added the possibility to disable layout animations. By default layout animations are disabled, but you don't need to turn it on manually, because these will be enabled if you will use the entering/exiting/layout prop in an animated component. You also have the possibility to disable/enable it manually.

Example

import { enableLayoutAnimations } from 'react-native-reanimated';

enableLayoutAnimations(false);

@piaskowyk piaskowyk self-assigned this Nov 22, 2021
@piaskowyk piaskowyk marked this pull request as ready for review November 22, 2021 09:58
@piaskowyk piaskowyk requested a review from tomekzaw November 22, 2021 09:59
if (!filteredOptions) {
return;
}
NativeReanimatedModule.setEnableFeatures(filteredOptions);
Copy link
Member Author

Choose a reason for hiding this comment

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

You need to add implementation for web

Copy link
Member Author

Choose a reason for hiding this comment

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

done 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks

Copy link
Member

@kmagiera kmagiera left a comment

Choose a reason for hiding this comment

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

This looks ok, however, I think it'd be easier to add just one flag specifically for turning on layout animation as opposed to designing more generic solution to it.

If instead of setEnableFeatures we'd just have enableLayoutAnimations function we could:

  1. not really care about web implementation too much. Layout Animations does not work on web so we could just no-op there
  2. we now call setEnableFeatures on every component render. If it was just setting a single boolean variable we'd avoid additional complexity of merging and filtering dicts

@piaskowyk piaskowyk merged commit 5a1407c into master Nov 23, 2021
@piaskowyk piaskowyk deleted the @piaskowyk/enable-layout-animation branch November 23, 2021 16:41
piaskowyk added a commit that referenced this pull request 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:
```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 #2758
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.

2 participants