Skip to content

Commit

Permalink
Fix useScrollViewOffset taking wrong viewTag (#5790)
Browse files Browse the repository at this point in the history
## 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

- [x] iOS

- [x] Android

- [x] Fabric

<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}
      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>

---------

Co-authored-by: szydlovsky <9szydlowski9@gmail.com>
  • Loading branch information
tjzel and szydlovsky authored Apr 3, 2024
1 parent 07887d9 commit 380c9cf
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/reanimated2/hook/commonTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface AnimatedRef<T extends Component> {
| ShadowNodeWrapper // Fabric
| HTMLElement; // web
current: T | null;
getTag: () => number;
}

// Might make that type generic if it's ever needed.
Expand Down
17 changes: 14 additions & 3 deletions src/reanimated2/hook/useAnimatedRef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,20 @@ export function useAnimatedRef<
const getTagValueFunction = isFabric()
? getShadowNodeWrapperFromRef
: findNodeHandle;
tag.value = IS_WEB
? getComponentOrScrollable(component)
: getTagValueFunction(getComponentOrScrollable(component));

const getTagOrShadowNodeWrapper = () => {
return IS_WEB
? getComponentOrScrollable(component)
: getTagValueFunction(getComponentOrScrollable(component));
};

tag.value = getTagOrShadowNodeWrapper();

// On Fabric we have to unwrap the tag from the shadow node wrapper
fun.getTag = isFabric()
? () => findNodeHandle(getComponentOrScrollable(component))
: getTagOrShadowNodeWrapper;

fun.current = component;
// viewName is required only on iOS with Paper
if (Platform.OS === 'ios' && !isFabric()) {
Expand Down
6 changes: 2 additions & 4 deletions src/reanimated2/hook/useScrollViewOffset.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';
import { useEffect, useRef, useCallback } from 'react';
import type { SharedValue } from '../commonTypes';
import { findNodeHandle } from 'react-native';
import type { EventHandlerInternal } from './useEvent';
import { useEvent } from './useEvent';
import { useSharedValue } from './useSharedValue';
Expand Down Expand Up @@ -103,9 +102,8 @@ function useScrollViewOffsetNative(
}
scrollRef.current = animatedRef.current;

const component = animatedRef.current;
const viewTag = findNodeHandle(component);
eventHandler.workletEventHandler.registerForEvents(viewTag as number);
const viewTag = animatedRef.getTag();
eventHandler.workletEventHandler.registerForEvents(viewTag);
return () => {
eventHandler.workletEventHandler.unregisterFromEvents();
};
Expand Down

0 comments on commit 380c9cf

Please sign in to comment.