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

Fix calling getBoundingClientRect on null #5462

Merged
merged 2 commits into from
Dec 4, 2023
Merged

Conversation

m-bert
Copy link
Contributor

@m-bert m-bert commented Dec 4, 2023

Summary

In implementation of web layout animations I used getSnapshotBeforeUpdate to get position of element before its re-render. However, this function lacks checks for platform and whether component is null or not.

Test plan

Tested on provided repro

Code from issue
import React, {useRef, useState} from 'react'
import {StyleSheet, Text, TouchableOpacity, View, ViewStyle} from 'react-native'
import Animated from "react-native-reanimated"
import FastImage from "react-native-fast-image"

export const ReanimatedFastImage = Animated.createAnimatedComponent(FastImage)
const App = () => {

  const [value, setValue] = useState(false)
  const cardImageRef = useRef<any>(null)

  const onPress = () => {
    setValue(!value)
  }

  return (
    <View style={styles.container}>
      <ReanimatedFastImage ref={cardImageRef} />
      {value ? <View /> : null}
      <TouchableOpacity onPress={onPress} style={styles.buttonStyle}>
        <Text>Toggle Set State</Text>
      </TouchableOpacity>
    </View>
  )
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    backgroundColor: '#ecf0f1',
    padding: 8,
    alignItems: 'center'
  } as ViewStyle,
  buttonStyle: {
    backgroundColor: 'grey',
    width: 200,
    height: 50,
    marginVertical: 20,
    borderRadius: 8,
    justifyContent: 'center',
    alignItems: 'center',
  },
});

export default App

@m-bert m-bert requested a review from tomekzaw December 4, 2023 09:28
@m-bert m-bert added this pull request to the merge queue Dec 4, 2023
Merged via the queue into main with commit 15717e7 Dec 4, 2023
7 checks passed
@m-bert m-bert deleted the @mbert/fix-null-rect branch December 4, 2023 10:23
@td-tomasz
Copy link
Contributor

Could you release it as patch version?

@tomekzaw
Copy link
Member

tomekzaw commented Dec 7, 2023

@td-tomasz-joniec Yep, we plan to release 3.6.2 but not sure yet when exactly. Can you use patch-package for now instead? I'll let you know once we release 3.6.2.

Latropos pushed a commit that referenced this pull request Dec 12, 2023
## Summary

In implementation of web layout animations I used
`getSnapshotBeforeUpdate` to get position of element before its
re-render. However, this function lacks checks for platform and whether
component is null or not.

## Test plan

Tested on provided repro

<details>
<summary> Code from issue </summary>

```jsx
import React, {useRef, useState} from 'react'
import {StyleSheet, Text, TouchableOpacity, View, ViewStyle} from 'react-native'
import Animated from "react-native-reanimated"
import FastImage from "react-native-fast-image"

export const ReanimatedFastImage = Animated.createAnimatedComponent(FastImage)
const App = () => {

  const [value, setValue] = useState(false)
  const cardImageRef = useRef<any>(null)

  const onPress = () => {
    setValue(!value)
  }

  return (
    <View style={styles.container}>
      <ReanimatedFastImage ref={cardImageRef} />
      {value ? <View /> : null}
      <TouchableOpacity onPress={onPress} style={styles.buttonStyle}>
        <Text>Toggle Set State</Text>
      </TouchableOpacity>
    </View>
  )
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    backgroundColor: '#ecf0f1',
    padding: 8,
    alignItems: 'center'
  } as ViewStyle,
  buttonStyle: {
    backgroundColor: 'grey',
    width: 200,
    height: 50,
    marginVertical: 20,
    borderRadius: 8,
    justifyContent: 'center',
    alignItems: 'center',
  },
});

export default App

```

</details>
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.

3 participants