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

Pull to Refresh indicator stops displaying after initial use in FlatList (iOS, Fabric, 0.71.7) #37308

Closed
skinsapp opened this issue May 8, 2023 · 12 comments
Labels
Component: FlatList Needs: Triage 🔍 Platform: iOS iOS applications. Stale There has been a lack of activity on this issue and it may be closed soon. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@skinsapp
Copy link

skinsapp commented May 8, 2023

Description

Standard usage of FlatList with pull to refresh functionality displays correctly the first few times, but something causes it to eventually no longer be present.

I've been trying to determine what's causing it. It's not just refreshing === true as is described here:
#35779

To reproduce it however is easy: during development, just reload the bundle by pressing "R" from the terminal, and RefreshControl indicators will be gone.

This obviously happens under other real circumstances as well:

In my application, one way to trigger it is by toggling whether it renders from a top level application Provider. My app will determine whether to render the Admin panel from the Provider file or the main App. When you go to the Admin panel and back, refresh controls are now gone. But if you move the conditional from Provider.js to App.js, the issue doesn't happen.

Lastly, if you kill the app, and re-open it, it will be there again. Once FlatList determines not to display it, it won't be displayed again for the remainder of the session, which is what leads me to think there's some native caching problem going on.

Pull to Refresh functionality seems to need a revisit in the new Fabric / new Architecture world. Color props also don't work.

React Native Version

0.71.7

Output of npx react-native info

System:
OS: macOS 13.3.1
CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
Memory: 340.67 MB / 32.00 GB
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 18.9.0 - ~/.nvm/versions/node/v18.9.0/bin/node
Yarn: 3.5.0 - ~/.nvm/versions/node/v18.9.0/bin/yarn
npm: 8.19.1 - ~/.nvm/versions/node/v18.9.0/bin/npm
Watchman: 2022.10.03.00 - /usr/local/bin/watchman
Managers:
CocoaPods: 1.12.0 - /Users/jamesgillmore/.rvm/gems/ruby-3.1.2/bin/pod
SDKs:
iOS SDK:
Platforms: DriverKit 22.4, iOS 16.4, macOS 13.3, tvOS 16.4, watchOS 9.4
Android SDK: Not Found
IDEs:
Android Studio: Not Found
Xcode: 14.3/14E222b - /usr/bin/xcodebuild
Languages:
Java: Not Found
npmPackages:
@react-native-community/cli: Not Found
react: 18.2.0 => 18.2.0
react-native: 0.71.7 => 0.71.7
react-native-macos: Not Found
npmGlobalPackages:
react-native: Not Found

Steps to reproduce

import React, { useState } from 'react';
import {
  SafeAreaView,
  View,
  FlatList,
  StyleSheet,
  Text,
  StatusBar,
  Pressable
} from 'react-native';

const DATA = [
  {
    id: 'bd7acbea-c1b1-46c2-aed5-3ad53abb28ba',
    title: 'First Item',
  },
  {
    id: '3ac68afc-c605-48d3-a4f8-fbd91aa97f63',
    title: 'Second Item',
  },
  {
    id: '58694a0f-3da1-471f-bd96-145571e29d72',
    title: 'Third Item',
  },
];

const Item = ({title}) => (
  <View style={styles.item}>
    <Text style={styles.title}>{title}</Text>
  </View>
);



const App = () => {
  return (
    <SafeAreaView style={styles.container}>
      <FlatListWithPullToRefresh />
    </SafeAreaView>
  );
};



// repro

const FlatListWithPullToRefresh = () => {
  const [refreshing, setRefreshing] = useState(false)

  const onRefresh = () => {
    setRefreshing(true)
    setTimeout(() => setRefreshing(false), 1000)
  }

  return (
    <FlatList
      refreshing={refreshing}
      onRefresh={onRefresh}
      data={DATA}
      renderItem={({item}) => <Item title={item.title} />}
      keyExtractor={item => item.id}
    />
  )
}




const styles = StyleSheet.create({
  container: {
    flex: 1,
    marginTop: StatusBar.currentHeight || 0,
  },
  item: {
    backgroundColor: '#f9c2ff',
    padding: 20,
    marginVertical: 8,
    marginHorizontal: 16,
  },
  title: {
    fontSize: 32,
  },

  button: {
    height: 50,
    width: '90%',
    alignItems: 'center',
    justifyContent: 'center',
    backgroundColor: 'red',
    alignSelf: 'center'
  }
});

export default App;

Snack, code example, screenshot, or link to a repository

https://snack.expo.dev/@skinsengineering/flatlist-simple

NOTE: you will need to run the repro locally in development mode (rather than via expo) and reload the bundle from the terminal, to see the refresh controls disappear. Hopefully solving it in this case will solve it in the many others people seem to be having.

@skinsapp skinsapp added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels May 8, 2023
@cortinico
Copy link
Contributor

Pull to Refresh functionality seems to need a revisit in the new Fabric / new Architecture world. Color props also don't work.

Is your issue happening only on New Architecture?

@coolersham
Copy link

@the-skins-app Although not 100% the same issue description, the problem might be caused by the same root problem as described and demonstrated in #36173.

@skinsapp
Copy link
Author

@cortinico my app can't run in the old architecture, so I don't know if the issue "only" occurs with fabric enabled. But yes, it's occurring with the new architecture enabled on RN 0.71.7.

@coolersham the issue definitely is related and probably shares the same root cause. That's not my issue however, as I saw the issue you posted, and verified that there was noScrollView (or other scrolling view) above FlatList. I had wished that was the issue, so I could +1 your original issue.

My issue + your issue + the refreshing === true issue and maybe a few others -- i would say -- makes this a bonafied real issue, rather than developer mistakes :)

Basically professional apps can't ship Pull to Refresh functionality (in a new architecture app) as it currently stands.

@zackify
Copy link

zackify commented Jul 3, 2023

I’m not sure how more people aren’t seeing this.

Simply using refreshing and onRefresh does not work correctly in iOS.

It works as expected on android and shows the indicator every time refreshing is true, but on iOS it doesn’t stay up.

@EduardSavchenko
Copy link

Hi all.
Please tell me,
did someone manage to solve the problem?

@ori9998
Copy link

ori9998 commented Aug 3, 2023

Anyone? I'm having the same issue. With the same code, on IOS, RN version 0.71.6 works fine but not working on 0.72.3.

@Danushka50
Copy link

Any update on this?
Used 0.71.7 and error occurred in SectionList also.
When the list is empty and set the refresh using state we can see the indicator but if the list is loaded with items the indicator now shown.

@dongsuo
Copy link

dongsuo commented Aug 6, 2023

Same here, looking forward a solution. I guess the reason is the wrapper doesn't scroll down so the refresh indicator is hidden under something else.

@Danushka50
Copy link

Danushka50 commented Aug 7, 2023

I have changed the node_modules/react-native/React/Views/RefreshControl/RCTRefreshControl.m file beginRefreshingProgrammatically function like below and build again and this work as expected.

But I don’t know this animations can be a problem. This cannot be a perfect solution but it works....

- (void)beginRefreshingProgrammatically
{
  UInt64 beginRefreshingTimestamp = _currentRefreshingStateTimestamp;
  _refreshingProgrammatically = YES;

  // Fix for bug #24855
  [self sizeToFit];

  if (self.scrollView) {
    // When using begin refreshing we need to adjust the ScrollView content offset manually.
    // UIScrollView *scrollView = (UIScrollView *)self.scrollView;

    // CGPoint offset = {scrollView.contentOffset.x, scrollView.contentOffset.y - self.frame.size.height};

    // // `beginRefreshing` must be called after the animation is done. This is why it is impossible
    // // to use `setContentOffset` with `animated:YES`.
    // [UIView animateWithDuration:0.25
    //     delay:0
    //     options:UIViewAnimationOptionBeginFromCurrentState
    //     animations:^(void) {
    //       [scrollView setContentOffset:offset];
    //     }
    //     completion:^(__unused BOOL finished) {
    //       if (beginRefreshingTimestamp == self->_currentRefreshingStateTimestamp) {
    //         [super beginRefreshing];
    //         [self setCurrentRefreshingState:super.refreshing];
    //       }
    //     }];

        UIScrollView *scrollView = (UIScrollView *)self.superview;
        CGPoint offset = {scrollView.contentOffset.x, scrollView.contentOffset.y - self.frame.size.height};
        [scrollView setContentOffset:offset animated:YES];
        if (beginRefreshingTimestamp == self->_currentRefreshingStateTimestamp) {
          [super beginRefreshing];
          [self setCurrentRefreshingState:super.refreshing];
         }


  } else if (beginRefreshingTimestamp == self->_currentRefreshingStateTimestamp) {
    [super beginRefreshing];
    [self setCurrentRefreshingState:super.refreshing];
  }
}

@steve228uk
Copy link

steve228uk commented Aug 15, 2023

I have changed the node_modules/react-native/React/Views/RefreshControl/RCTRefreshControl.m file beginRefreshingProgrammatically function like below and build again and this work as expected.

But I don’t know this animations can be a problem. This cannot be a perfect solution but it works....

- (void)beginRefreshingProgrammatically
{
  UInt64 beginRefreshingTimestamp = _currentRefreshingStateTimestamp;
  _refreshingProgrammatically = YES;

  // Fix for bug #24855
  [self sizeToFit];

  if (self.scrollView) {
    // When using begin refreshing we need to adjust the ScrollView content offset manually.
    // UIScrollView *scrollView = (UIScrollView *)self.scrollView;

    // CGPoint offset = {scrollView.contentOffset.x, scrollView.contentOffset.y - self.frame.size.height};

    // // `beginRefreshing` must be called after the animation is done. This is why it is impossible
    // // to use `setContentOffset` with `animated:YES`.
    // [UIView animateWithDuration:0.25
    //     delay:0
    //     options:UIViewAnimationOptionBeginFromCurrentState
    //     animations:^(void) {
    //       [scrollView setContentOffset:offset];
    //     }
    //     completion:^(__unused BOOL finished) {
    //       if (beginRefreshingTimestamp == self->_currentRefreshingStateTimestamp) {
    //         [super beginRefreshing];
    //         [self setCurrentRefreshingState:super.refreshing];
    //       }
    //     }];

        UIScrollView *scrollView = (UIScrollView *)self.superview;
        CGPoint offset = {scrollView.contentOffset.x, scrollView.contentOffset.y - self.frame.size.height};
        [scrollView setContentOffset:offset animated:YES];
        if (beginRefreshingTimestamp == self->_currentRefreshingStateTimestamp) {
          [super beginRefreshing];
          [self setCurrentRefreshingState:super.refreshing];
         }


  } else if (beginRefreshingTimestamp == self->_currentRefreshingStateTimestamp) {
    [super beginRefreshing];
    [self setCurrentRefreshingState:super.refreshing];
  }
}

Can confirm this worked but I am getting some glitchy animations when used with React Navigation and Large Titles.

update: This doesn't seem to work for me with Expo Router unfortunately.

Copy link

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Feb 12, 2024
Copy link

This issue was closed because it has been stalled for 7 days with no activity.

cipolleschi added a commit to cipolleschi/react-native that referenced this issue Apr 11, 2024
Summary:
This change properly recycles the RCTPullToRefreshViewComponentView so that it fixes some misbehaviors of the UIRefreshControl in OSS.

This should fix facebook#37308 and facebook#36173

## Changelog:
[iOS][Fixed] - Properly recycle the RCTPullToRefreshViewComponentView

Differential Revision: D56018924
cipolleschi added a commit to cipolleschi/react-native that referenced this issue Apr 11, 2024
Summary:

This change properly recycles the RCTPullToRefreshViewComponentView so that it fixes some misbehaviors of the UIRefreshControl in OSS.

This should fix facebook#37308 and facebook#36173

## Changelog:
[iOS][Fixed] - Properly recycle the RCTPullToRefreshViewComponentView

Differential Revision: D56018924
facebook-github-bot pushed a commit that referenced this issue Apr 12, 2024
Summary:
Pull Request resolved: #44047

This change properly recycles the RCTPullToRefreshViewComponentView so that it fixes some misbehaviors of the UIRefreshControl in OSS.

This should fix #37308 and #36173

## Changelog:
[iOS][Fixed] - Properly recycle the RCTPullToRefreshViewComponentView

Reviewed By: sammy-SC

Differential Revision: D56018924

fbshipit-source-id: 3c71328aa0f6fb2a98a19593f0f06419e04e9cae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: FlatList Needs: Triage 🔍 Platform: iOS iOS applications. Stale There has been a lack of activity on this issue and it may be closed soon. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants