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 useScrollViewOffset taking wrong viewTag #5790

Merged
merged 4 commits into from
Apr 3, 2024

Conversation

tjzel
Copy link
Collaborator

@tjzel tjzel commented Mar 14, 2024

Summary

Fixes #5777

Currently useScrollViewOffset takes the viewTag to the underlying ScrollView by calling findNodeHandle itself on animatedRef. This doesn't take into consideration a ScrollView that's wrapped in another view. When you specify onRefresh property that's what happens - ScrollView becomes a child of another view. useAnimatedRef takes the correct tag because it uses in this case getScrollableNode member function of the component - but this logic isn't transferred to other hooks that require the tag.

This PR adds a getter function to the object returned by useAnimatedRef, so no function needing the tag should ever acquire it by itself - instead it should use the getter method.

Test plan

  • iOS

  • Android

  • Fabric

Testing code
import React from 'react';
import {
  Text,
  Pressable,
  SafeAreaView,
  StyleSheet,
  Button,
  findNodeHandle,
} from 'react-native';
import Animated, {
  useAnimatedRef,
  useScrollViewOffset,
} from 'react-native-reanimated';

export default function App() {
  const animatedRef = useAnimatedRef<Animated.ScrollView>();
  const offset = useScrollViewOffset(animatedRef);
  const [refreshing, setRefreshing] = React.useState(false);

  function printOffset() {
    console.log('OFFSET VALUE');
    console.log(offset.value);
  }

  return (
    <SafeAreaView>
      <Button
        title="refresh"
        onPress={() => setRefreshing((oldState) => !oldState)}
      />
      <List
        animatedRef={animatedRef}
        printOffset={printOffset}
        refreshing={refreshing}
      />
    </SafeAreaView>
  );
}

const List = ({ animatedRef, printOffset, refreshing }) => {
  console.log('tag', findNodeHandle(animatedRef.current));
  return (
    <Animated.FlatList
      ref={animatedRef}
      refreshing={refreshing}
      onRefresh={() => console.log('refreshing')}
      onViewableItemsChanged={() => {
        printOffset();
      }}
      data={[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]}
      renderItem={({ _, index }) => <Item onPress={printOffset} id={index} />}
    />
  );
};

const Item = ({ onPress, id }) => {
  return (
    <Pressable onPress={onPress} style={styles.pressable}>
      <Text>{id}</Text>
    </Pressable>
  );
};

const styles = StyleSheet.create({
  pressable: {
    padding: 40,
    justifyContent: 'center',
    alignItems: 'center',
    borderWidth: 1,
    borderColor: 'red',
    backgroundColor: 'white',
  },
});

@tjzel tjzel marked this pull request as ready for review March 14, 2024 16:42
@tomekzaw
Copy link
Member

Does it work on Fabric? If yes, should we add a separate method getShadowNodeWrapper? 🤔

@tjzel
Copy link
Collaborator Author

tjzel commented Mar 14, 2024

I forgor about Fabric

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.

i trust you

@tjzel tjzel added this pull request to the merge queue Apr 3, 2024
Merged via the queue into main with commit 380c9cf Apr 3, 2024
7 checks passed
@tjzel tjzel deleted the @tjzel/useScrollViewOffset/fix-taking-wrong-tag branch April 3, 2024 14:58
github-merge-queue bot pushed a commit that referenced this pull request Apr 3, 2024
## Summary

Inspired by
#5790,
it seems like scrolls can be wrapped in other views (by for example
setting their `onRefresh` property), which makes refs to them point to
the wrapping view, not them. Thus I add support for such situations to
the `useScrollViewOffset` hook, which is our only hook that needs to
access the component itself.

## Test plan

<details>
<summary>
Testing code
</summary>

```tsx
import React from 'react';
import {
  Text,
  Pressable,
  SafeAreaView,
  StyleSheet,
  Button,
  findNodeHandle,
} from 'react-native';
import Animated, {
  useAnimatedRef,
  useScrollViewOffset,
} from 'react-native-reanimated';

export default function App() {
  const animatedRef = useAnimatedRef<Animated.ScrollView>();
  const offset = useScrollViewOffset(animatedRef);
  const [refreshing, setRefreshing] = React.useState(false);

  function printOffset() {
    console.log('OFFSET VALUE');
    console.log(offset.value);
  }

  return (
    <SafeAreaView>
      <Button
        title="refresh"
        onPress={() => setRefreshing((oldState) => !oldState)}
      />
      <List
        animatedRef={animatedRef}
        printOffset={printOffset}
        refreshing={refreshing}
      />
    </SafeAreaView>
  );
}

const List = ({ animatedRef, printOffset, refreshing }) => {
  console.log('tag', findNodeHandle(animatedRef.current));
  return (
    <Animated.FlatList
      ref={animatedRef}
      refreshing={refreshing}
      style={{height: 600}}
      onRefresh={() => console.log('refreshing')}
      onViewableItemsChanged={() => {
        printOffset();
      }}
      data={[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]}
      renderItem={({ _, index }) => <Item onPress={printOffset} id={index} />}
    />
  );
};

const Item = ({ onPress, id }) => {
  return (
    <Pressable onPress={onPress} style={styles.pressable}>
      <Text>{id}</Text>
    </Pressable>
  );
};

const styles = StyleSheet.create({
  pressable: {
    padding: 40,
    justifyContent: 'center',
    alignItems: 'center',
    borderWidth: 1,
    borderColor: 'red',
    backgroundColor: 'white',
  },
});
```
</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.

[Android] useScrollViewOffset not updating with Animated.FlatList RefreshControl
3 participants