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 scrollTo on FlashList #4384

Merged
merged 1 commit into from
Apr 21, 2023
Merged

Fix scrollTo on FlashList #4384

merged 1 commit into from
Apr 21, 2023

Conversation

tomekzaw
Copy link
Member

@tomekzaw tomekzaw commented Apr 20, 2023

Summary

Fixes #4376. Analogous to #3988.

Solution: copy the code from createAnimatedComponent.tsx to useAnimatedRef.ts

return this._component?.getScrollableNode
? this._component.getScrollableNode()
: this._component;

Example:

App.tsx
import Animated, {
  runOnUI,
  scrollTo,
  useAnimatedRef,
} from 'react-native-reanimated';
import {Button, StyleSheet, Switch, Text, View} from 'react-native';
import {FlashList} from '@shopify/flash-list';

import React from 'react';

declare const _WORKLET: boolean;

const AnimatedFlashList = Animated.createAnimatedComponent(FlashList);

interface Item {
  label: string;
}

const DATA: Item[] = [...Array(100)].map((_, i) => ({label: String(i)}));

function getRandomOffset() {
  'worklet';
  return Math.random() * 2000;
}

export default function ScrollToExample() {
  const [animated, setAnimated] = React.useState(true);

  const aref = useAnimatedRef<FlashList<Item>>();

  const scrollFromJS = () => {
    console.log(_WORKLET);
    aref.current?.scrollToOffset({offset: getRandomOffset(), animated});
  };

  const scrollFromUI = () => {
    runOnUI(() => {
      'worklet';
      console.log(_WORKLET);
      scrollTo(aref, 0, getRandomOffset(), animated);
    })();
  };

  return (
    <>
      <View style={styles.buttons}>
        <Switch
          value={animated}
          onValueChange={setAnimated}
          style={styles.switch}
        />
        <Button onPress={scrollFromJS} title="Scroll from JS" />
        <Button onPress={scrollFromUI} title="Scroll from UI" />
      </View>
      <AnimatedFlashList
        // @ts-ignore createAnimatedComponent seems to break generic types
        ref={aref}
        data={DATA}
        // @ts-ignore createAnimatedComponent seems to break generic types
        renderItem={({item}: {item: Item}) => (
          <Text key={item.label} style={styles.text}>
            {item.label}
          </Text>
        )}
        estimatedItemSize={200}
      />
    </>
  );
}

const styles = StyleSheet.create({
  switch: {
    marginBottom: 10,
  },
  buttons: {
    marginTop: 80,
    marginBottom: 40,
    alignItems: 'center',
  },
  text: {
    fontSize: 50,
    textAlign: 'center',
  },
});

Before:

Android:

android.mp4

iOS:

ios.mp4

Test plan

  1. Install FlashList in Example app with yarn add @shopify/flash-list && cd ios && pod install

  2. Paste the attached code snippet into App.tsx

Warning
It looks like animated refs don't update properly on render. In this case, pressing "Scroll from UI" button after a fast refresh does nothing. Please reload the r to reload the app and check again.

Comment on lines +35 to +37
component.getScrollableNode
? component.getScrollableNode()
: component
Copy link
Member

Choose a reason for hiding this comment

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

Maybe, we could add some comment why we use .getScrollableNode() or just add information that it comes from FlashList?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hopefully git blame should find this PR which links to the original issue but yeah you're right, adding a comment here would be nice 😄 Also, there's a similar place in createAnimatedComponent.tsx which does exactly the same thing and I'd like to move it to a new utils function, wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

The idea of a new function sound great ☺️

@tomekzaw tomekzaw added this pull request to the merge queue Apr 21, 2023
Merged via the queue into main with commit 690b887 Apr 21, 2023
@tomekzaw tomekzaw deleted the @tomekzaw/fix-flashlist-scrollto branch April 21, 2023 15:41
mstach60161 added a commit that referenced this pull request May 22, 2023
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

- Fixes #4256

### Problem

If the scrollable component is not root, UI `scrollTo` doesn't work on
Fabric.

#4384 - It was fixed here for Paper, but after this change app crashes
on Fabric, because `getScrollableNode` in both architectures returns
viewTag as a number, not component. It's working for Paper, because we
are passing either component or number to `findNodeHandle` method, but
on Fabric we need component ref.

### Fix
 
Instead of `getScrollableNode`, we can use `getNativeScrollRef` and get
scrollable reference.


### TODO

Unfortunately this method is not implemented yet for `Flash-list` ->
https://shopify.github.io/flash-list/docs/usage#:~:text=Unsupported%20methods%3A
so for now, UI `scrollTo` on FlashList works only for Paper
Architecture.

## Test plan

ScrollToExample:

- Paper, ScrollView
- Paper, FlatList
- Fabric, ScrollView
- Fabric, FlatList

---------

Co-authored-by: Tomek Zawadzki <tomasz.zawadzki@swmansion.com>
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
## Summary

Fixes software-mansion#4376. Analogous to software-mansion#3988.

Solution: copy the code from `createAnimatedComponent.tsx` to
`useAnimatedRef.ts`


https://github.com/software-mansion/react-native-reanimated/blob/87fb9c6a1fb6b8c839b65b90ad16f574f45a1294/src/createAnimatedComponent.tsx#L289-L291

Example:

<details>
<summary>App.tsx</summary>

```tsx
import Animated, {
  runOnUI,
  scrollTo,
  useAnimatedRef,
} from 'react-native-reanimated';
import {Button, StyleSheet, Switch, Text, View} from 'react-native';
import {FlashList} from '@shopify/flash-list';

import React from 'react';

declare const _WORKLET: boolean;

const AnimatedFlashList = Animated.createAnimatedComponent(FlashList);

interface Item {
  label: string;
}

const DATA: Item[] = [...Array(100)].map((_, i) => ({label: String(i)}));

function getRandomOffset() {
  'worklet';
  return Math.random() * 2000;
}

export default function ScrollToExample() {
  const [animated, setAnimated] = React.useState(true);

  const aref = useAnimatedRef<FlashList<Item>>();

  const scrollFromJS = () => {
    console.log(_WORKLET);
    aref.current?.scrollToOffset({offset: getRandomOffset(), animated});
  };

  const scrollFromUI = () => {
    runOnUI(() => {
      'worklet';
      console.log(_WORKLET);
      scrollTo(aref, 0, getRandomOffset(), animated);
    })();
  };

  return (
    <>
      <View style={styles.buttons}>
        <Switch
          value={animated}
          onValueChange={setAnimated}
          style={styles.switch}
        />
        <Button onPress={scrollFromJS} title="Scroll from JS" />
        <Button onPress={scrollFromUI} title="Scroll from UI" />
      </View>
      <AnimatedFlashList
        // @ts-ignore createAnimatedComponent seems to break generic types
        ref={aref}
        data={DATA}
        // @ts-ignore createAnimatedComponent seems to break generic types
        renderItem={({item}: {item: Item}) => (
          <Text key={item.label} style={styles.text}>
            {item.label}
          </Text>
        )}
        estimatedItemSize={200}
      />
    </>
  );
}

const styles = StyleSheet.create({
  switch: {
    marginBottom: 10,
  },
  buttons: {
    marginTop: 80,
    marginBottom: 40,
    alignItems: 'center',
  },
  text: {
    fontSize: 50,
    textAlign: 'center',
  },
});
```

</details>

Before:

<img
src="https://user-images.githubusercontent.com/20516055/233323545-427cc3d4-4479-4b02-a24b-5a3039fb3105.png"
width="300" />

Android:


https://user-images.githubusercontent.com/20516055/233322746-637fa2a9-0e93-40ac-baa8-c3c3d2449867.mp4

iOS:


https://user-images.githubusercontent.com/20516055/233322714-20c03ade-1f15-4d76-8a5e-b4b59ebe8a73.mp4

## Test plan

1. Install FlashList in Example app with `yarn add @shopify/flash-list
&& cd ios && pod install`

2. Paste the attached code snippet into App.tsx

> **Warning**
> It looks like animated refs don't update properly on render. In this
case, pressing "Scroll from UI" button after a fast refresh does
nothing. Please reload the <kbd>r</kbd> to reload the app and check
again.
fluiddot pushed a commit to wordpress-mobile/react-native-reanimated that referenced this pull request Jun 5, 2023
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

- Fixes software-mansion#4256

### Problem

If the scrollable component is not root, UI `scrollTo` doesn't work on
Fabric.

software-mansion#4384 - It was fixed here for Paper, but after this change app crashes
on Fabric, because `getScrollableNode` in both architectures returns
viewTag as a number, not component. It's working for Paper, because we
are passing either component or number to `findNodeHandle` method, but
on Fabric we need component ref.

### Fix
 
Instead of `getScrollableNode`, we can use `getNativeScrollRef` and get
scrollable reference.


### TODO

Unfortunately this method is not implemented yet for `Flash-list` ->
https://shopify.github.io/flash-list/docs/usage#:~:text=Unsupported%20methods%3A
so for now, UI `scrollTo` on FlashList works only for Paper
Architecture.

## Test plan

ScrollToExample:

- Paper, ScrollView
- Paper, FlatList
- Fabric, ScrollView
- Fabric, FlatList

---------

Co-authored-by: Tomek Zawadzki <tomasz.zawadzki@swmansion.com>
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.

scrollTo not working with FlashList
2 participants