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

RefreshControl still is showing even the value of refreshing is false #5839

Closed
pomelio opened this issue Feb 9, 2016 · 48 comments
Closed

RefreshControl still is showing even the value of refreshing is false #5839

pomelio opened this issue Feb 9, 2016 · 48 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@pomelio
Copy link

pomelio commented Feb 9, 2016

version: 0.19.0
ListView refreshControl
IOS9.2

When Pulling down the listview items, the onRefresh is triggered properly with the 'indicator', but the indicator doesn't hide when the refreshing property was changed to 'false'

@facebook-github-bot
Copy link
Contributor

Hey jaynsw, thanks for reporting this issue!

React Native, as you've probably heard, is getting really popular and truth is we're getting a bit overwhelmed by the activity surrounding it. There are just too many issues for us to manage properly.

  • If you don't know how to do something or something is not working as you expect but not sure it's a bug, please ask on StackOverflow with the tag react-native or for more real time interactions, ask on Discord in the #react-native channel.
  • If this is a feature request or a bug that you would like to be fixed, please report it on Product Pains. It has a ranking feature that lets us focus on the most important issues the community is experiencing.
  • We welcome clear issues and PRs that are ready for in-depth discussion. Please provide screenshots where appropriate and always mention the version of React Native you're using. Thank you for your contributions!

@skyride99
Copy link

I am seeing the same issue

@nicklockwood
Copy link
Contributor

cc: @janicduplessis

@janicduplessis
Copy link
Contributor

Could you give me a code sample that reproduces the issue?

@skyride99
Copy link

<ScrollView
style={styles.mainBottomContainer}
contentInset={{bottom:49}}
automaticallyAdjustContentInsets={false}
refreshControl={
<RefreshControl
refreshing={false}
onRefresh={() => this.refreshData()}
tintColor="#EBEBEB"
title="Loading..."
colors={['#ff0000', '#00ff00', '#0000ff']}
progressBackgroundColor="#EBEBEB"
/>
}>

....
refreshData(){

api.getAccountActivity().then((data) => {
  console.log('@@@@@ refesh data ' + JSON.stringify(data));

  if(data.success){
    this.shapeData(data.data);
  }

  this.setState({
    isLoading: false
  });
}).catch((error) => {
  console.warn(error);
  this.setState({
    isLoading: false
  });
});

}

@skyride99
Copy link

The isLoading when false should dismiss the refresh on the screen?

@janicduplessis
Copy link
Contributor

Does it work if you call this at the start of your refreshData function

this.setState({
  isLoading: true
});

@skyride99
Copy link

I put isLoading: true, at the start of the refresh. Still the same result. When the data returns and isLoading is set to false. The loading spinner is still moving on the screen.

screen shot 2016-02-10 at 1 09 32 pm

@janicduplessis
Copy link
Contributor

In your RefreshControl did you set the refreshing prop to false? It should be this.state.isLoading.

In the example you linked

style={styles.mainBottomContainer}
contentInset={{bottom:49}}
automaticallyAdjustContentInsets={false}
refreshControl={
refreshing={false} <-------------------- There
onRefresh={() => this.refreshData()}
tintColor="#EBEBEB"
title="Loading..."
colors={['#ff0000', '#00ff00', '#0000ff']}
progressBackgroundColor="#EBEBEB"
/>
}>

@skyride99
Copy link

I had that before sorry. Its working.

The actual fix is setting

this.setState({
isLoading: true
});

in the refresh.

@janicduplessis
Copy link
Contributor

Good! This isn't really a bug since the refreshing prop is a controlled prop and is supposed to be set to true at the start of the onRefresh event but I'll see if I can think of a way to warn about this error or make it more obvious because the native RefreshControl will still start refreshing even if the refreshing prop is not set to true.

@nicklockwood Any ideas if we can do something about this?

@tyao1
Copy link
Contributor

tyao1 commented Mar 3, 2016

I met the same problem. The refreshing prop is passed as props by redux. In android, the refreshing control will hide properly but in iOS the refreshing control will remain showing most of the time (it does hide sometimes). Seems to be a bug.

@kinhunt
Copy link

kinhunt commented Mar 7, 2016

It doesn't hide with the following code

  _onRefresh() {
            this.setState({isRefreshing: true});

            setTimeout(function(){
                this.setState({
                    isRefreshing: false,

                });
                alert("refreshed")
            }.bind(this), 2000);

        },


 <ListView style={styles.postsList}
                              ref={'listView'}
                              removeClippedSubviews={true}
                              dataSource={this.state.dataSource}
                              renderRow={this._renderRow}
                              onEndReached={this._onEndReached}
                              onEndReachedThreshold={0}
                              pageSize={16}
                              scrollRenderAheadDistance={100}
                              showsVerticalScrollIndicator={false}
                              scrollsToTop={true}
                              renderFooter={this._renderFooter}
                              refreshControl={
                                              <RefreshControl
                                                refreshing={this.state.isRefreshing}
                                                onRefresh={this._onRefresh}
                                                tintColor={Colors.primaryColor}
                                                title={this.state.refreshTitle}
                                                colors={[Colors.primaryColor]}
                                                progressBackgroundColor={Colors.primaryColor}
                                              />
                                            }

@Xwoder
Copy link

Xwoder commented Mar 10, 2016

I also met the problem.

This is my code:

<ListView style={styles.listView}
    dataSource={this.state.dataSource}
    renderHeader={this.renderHeader}
    renderRow={this.renderRow.bind(this)}
    refreshControl={<RefreshControl
    refreshing={this.state.isRefreshing}
    onRefresh={this._onRefresh.bind(this)}
    tintColor="#FF4946"
    title="Loading..."  /> }
/>
_onRefresh() {
    this.setState({isRefreshing: true});
    setTimeout(()=>{
        this.setState({
            isRefreshing: false,
        });
    }, 1000);
}

@thomasrose
Copy link

Seeing the same problem here, also passing a refreshing prop via redux. Interestingly it only seems to happen only every second time the dataSource changes, the other times it is okay, but that may just be my setup. Android is fine

@janicduplessis
Copy link
Contributor

Could someone provide a code sample or a link to rnplay that reproduce the bug so I can look into it?

@kinhunt
Copy link

kinhunt commented Mar 12, 2016

@janicduplessis my problem solved. My shouldComponentUpdate prevented the RefreshControl from being updated. Thank you anyway.

@tyao1
Copy link
Contributor

tyao1 commented Mar 23, 2016

In RN 0.22.2 the problem still occurs, but less often.

@hvsy
Copy link

hvsy commented Mar 24, 2016

@janicduplessis, I met the same problem, and found there is a 0.25s animation after change refreshing status in "RCTRefreshControl.m". If i change the refreshing value by JS during the time. The refreshing of props will inconsistencies with Object-C.

This is a test code. On my device,after the state change to false ,the control still is refreshing. you can change interval time and count.

    class Test extends React.Component{
        state = {
            refreshing : false
        };

        componentDidMount(){
            var count = 0;
            var timer = setInterval(()=>{
                if(count++ == 5){
                    clearInterval(timer);
                }
                this.setState({
                    refreshing : !this.state.refreshing
                })
            },30);
        }

        render(){
            return (<React.View>
                    <React.Text>{ this.state.refreshing  + '' }</React.Text>
                    <React.ScrollView refreshControl={<React.RefreshControl refreshing={this.state.refreshing} /> }>
                        <React.View style={{height:50}} />
                    </React.ScrollView>
            </React.View>);
        }
    }

@corbt
Copy link
Contributor

corbt commented Mar 29, 2016

I can verify that there's a race condition somewhere in there that can leave the RefreshControl refreshing even when refreshing={false}. My use is similar to @hvsy so I'm guessing he's isolated it.

@janicduplessis
Copy link
Contributor

Yea, I see why it happens, I'll work on a fix.

@dvdhsu
Copy link

dvdhsu commented Apr 27, 2016

Any updates @janicduplessis? This seems to still be happening in 0.24.

@hvsy
Copy link

hvsy commented Apr 27, 2016

@dvdhsu fixed in v0.25.0-rc
Fix RefreshControl refreshing state - 93b39b7

@tomprogers
Copy link

I'm still seeing this problem in rn 0.26.3

I've got the render method logging the value of this.props.isRefreshing, which comes from a redux store and is fed directly into the RefreshControl. The render method executes three times in quick succession, with true, false, false, but despite the fact that the last render pass had false, the spinner remains on-screen indefinitely.

@jzzj
Copy link

jzzj commented Jun 20, 2016

I'm seeing the same problem in rn 0.27.2.
It occurred when i pull to refresh and then call listview.getScrollResponder().scroll({y: 0}), the RefreshControl is still in the DOM, but isRefreshing has been changed to false.

@imoreapps
Copy link

I'm seeing the same problem in rn 0.28.0, as following figure shown, the RefreshControl will disappear when i touch the ListView.

a

Below is my codes.

constructor(props) {
    super(props);
    this.state = {
        isRefreshing: false
    };
}

componentDidMount() {
      this._fetchData();
}

render() {
  return (
              <ListView style={styles.list}
                          dataSource={this.state.dataSource}
                          renderHeader={this._renderHeader.bind(this)}
                          renderRow={this._renderRow.bind(this) }
                          renderFooter={this._renderFooter.bind(this)}
                          onEndReached={this._onEndReached.bind(this)}
                          renderSeparator={(sectionID, rowID) => <View key={`${sectionID}-${rowID}`} style={styles.separator}/>}
                          initialListSize={10}
                          pageSize={4}
                          scrollRenderAheadDistance={20}
                          enableEmptySections={true}
                          refreshControl={
                              <RefreshControl
                                  refreshing={this.state.isRefreshing}
                                  onRefresh={this._onRefresh.bind(this) }
                                  tintColor={Color.mainColor}
                                  title={'loading' }
                                  titleColor={Color.grayColor}
                                  colors={[Color.mainColor]}
                                  progressBackgroundColor={Color.lightWhite}
                              />}
                />
     );
}

_onRefresh() {
  this._fetchData();
}

_fetchData() {
  if (this.state.isRefreshing) {
            return;
        }

         this.setState(
             isRefreshing: true
         });

        API.getUserComments().then((comments) => {
               this.setState({
                    isRefreshing: false
               });
       }).catch((error) => {
              console.error(error)
               this.setState({
                    isRefreshing: false
               });
       });
}

@dabbott
Copy link
Contributor

dabbott commented Aug 11, 2016

I see this on 0.28.0 also.

Depending on your app's design, you might be able to work around the issue by setting a background on your ListView/ScrollView's contentContainerStyle to cover up the erroneous RefreshControl

<ListView
    contentContainerStyle={{backgroundColor: 'white'}}
    refreshControl={...}
/>

@Darwinium
Copy link

@imoreapps did you fix it? I have the same bug and can't find any solutions(

@imoreapps
Copy link

imoreapps commented Aug 23, 2016

@Darwinium I always use the InteractionManager.runAfterInteractions to wrap the data reload/fetch actions.


componentWillMount() {
          InteractionManager.runAfterInteractions(() => {this._fetchData()});
}

_fetchData() {
          this.setState({isFetching: true});
          .....
          this.setState({isFetching: false});
}

@Hkmu
Copy link

Hkmu commented Aug 30, 2016

I met the same problem, this helps.

@anton6
Copy link

anton6 commented Oct 4, 2016

I also came across this issue on 0.28.0. Has this been fixed in later versions?

@amrosebirani
Copy link

Facing the same issue on 0.29.2 , any fixes on this so far, or any work arounds?

@imoreapps
Copy link

It seemed that RN 0.34 fixed this issue.

@jonoirwinrsa
Copy link

jonoirwinrsa commented Oct 14, 2016

The issue that @imoreapps was facing @@was not fixed for me in v0.35.0. To get around the issue I set the backgroundColor of a child component to the ScrollView

@awedev
Copy link

awedev commented Jan 29, 2017

For those who are still facing the issue, you can wrap up your onRefresh content with InteractionManager like below:

onRefresh() {
    InteractionManager.runAfterInteractions(() => {
        this.setState({
            isFetching: true
        })


        Service.call(url).then((res) => {
            if (res.data.length === 0) {
                this.setState({
                    isFetching: false,
                })

                return;
            }

            this.page++;
            this.setState({
                dataSource: this.getDataSource(res),
                isFetching: false
            });
        }).catch(() => {
            this.setState({
                isFetching: false
            })
        });
    })
}

@duhseekoh
Copy link

Definitely still an issue.

If I set the refreshing prop to false from outside the onRefresh method, say... somewhere else in the UI that triggers a refresh, the RefreshControl doesn't do anything with that updated prop, just stays spinning.

@ophite
Copy link

ophite commented Mar 30, 2017

the same problem in 0.42 version in official example https://facebook.github.io/react-native/docs/refreshcontrol.html. Always spinning http://i.prntscr.com/39354445e5fd4ab38021894f898c9156.png

@marlti7
Copy link

marlti7 commented Aug 29, 2017

same problem in 0.44

@jhalborg
Copy link

Why was this issue closed?

I'm running on 0.48, and I'm still seeing the issue. I'm using a redux prop to determine if the list is refreshing:

<FeedList
          feedData={this.props.feedData}
          loadMore={this.getMoreData}
          fetching={this.props.fetchingFeedData}
          refresh={this.refreshData}
        />

...
<FlatList
          renderItem={this.renderRow}
          keyExtractor={this.keyExtractor}
          refreshing={this.props.fetching}
          onRefresh={this.props.refresh}
          data={this.props.feedData}
          extraData={this.props}
          onEndReached={this.props.loadMore}
          onEndReachedThreshold={0.8}
        />

...
const mapStateToProps = (state: IAppState, ownProps) => {
  return {
    feedData: state.feed.feedData,
    fetchingFeedData: state.feed.fetchingFeed,
  };
};

Even if the state.feed.fetchingFeed is changed to false after retrieving data, the spinner for the refresh control still shows.

For me, this only happens in conjunction with infinite scroll. If I scroll down the list and it fetches more items, and then back to the top, the pull-to-refresh animation is still going despite the fetchingFeed having been set to false.

It only happens on iOS

@fredbt
Copy link

fredbt commented Oct 14, 2017

I also don't understand why this got closed. I'm also facing the same issue as @jhalborg when using a redux props. My props is clearly false and the spinning is still showing up.

@fredbt
Copy link

fredbt commented Oct 14, 2017

@jhalborg the only solution I've found for now is to move to refresh props to the internal state of the page. Then I set it to true when calling the refresh function, and added a timeout to set it to false after a few seconds. This is a terrible solution, and it's really weird that the FlatList does not work well with the redux state directly.

@iShawnWang
Copy link

0.0

I am facing the same issue, after debug, I find this :
I add logs to native code RCTRefreshControl.m and try to figure out if the refresh state is passed from JS to native correctly.

And the logs like this :

  • Normal :
  1. beginRefreshing
  2. endRefreshing
  3. beginRefreshing_super
  4. endRefreshing_super
  • but in iPhone X simulator :
  1. beginRefreshing
  2. endRefreshing
  3. endRefreshing_Immediately
  4. beginRefreshing_super // keep RefreshControl refreshing

Source code :

- (void)beginRefreshing
{
  [self tempLog:(@"beginRefreshing")]; // Log 1
  UIScrollView *scrollView = (UIScrollView *)self.superview;
  CGPoint offset = {scrollView.contentOffset.x, scrollView.contentOffset.y - self.frame.size.height};

  [UIView animateWithDuration:0.25
                          delay:0
                        options:UIViewAnimationOptionBeginFromCurrentState
                     animations:^(void) {
                       [scrollView setContentOffset:offset];
                     } completion:^(__unused BOOL finished) {
                       [self tempLog:@"beginRefreshing_super]; //Log2
                       [super beginRefreshing];
                     }];
}

- (void)endRefreshing
{
  [self tempLog:(@"endRefreshing")]; //Log 3

  UIScrollView *scrollView = (UIScrollView *)self.superview;
  if (scrollView.contentOffset.y < 0) {
    CGPoint offset = {scrollView.contentOffset.x, -scrollView.contentInset.top};
    [UIView animateWithDuration:0.25
                          delay:0
                        options:UIViewAnimationOptionBeginFromCurrentState
                     animations:^(void) {
                       [scrollView setContentOffset:offset];
                     } completion:^(__unused BOOL finished) {
                       [self tempLog:(@"endRefreshing_super")]; //Log4
                       [super endRefreshing];
                     }];
  } else {
        [self tempLog:(@"endRefreshing_Immediately")]; //Log5
        [super endRefreshing];
  }
}

#define TimeStamp [NSString stringWithFormat:@"%f",[[NSDate date] timeIntervalSince1970] * 1000]
-(void)tempLog:(NSString*)msg{
// for Log with TimeStamp
  NSString *log = [NSString stringWithFormat:@"%@ : %@", msg, TimeStamp];
  RCTLogWarn(log);
}

So, is there a bug of iPhone X schedule [UIView animateWithDuration] animation ?
The beginRefresh get called schedule an animation using [UIView animateWithDuration] but not fired immediately,
so when call endRefresh,
if (scrollView.contentOffset.y < 0) { if false because the beginRefresh animation not start yet,
then [super endRefresh] called immediately.

I can't produce this issue only in iPhone X simulator, and it not happens everytimes.
I am not test on the iPhone X device.

ENV :

OSX : 10.13
Xcode : 9.1
Simulator SDK : 11.1

@jhalborg
Copy link

jhalborg commented Dec 29, 2017

Still present in 0.50+ for me. I awaited any new developments on this issue, but it seems dead so I've resorted to some hacking.

All of the proposed solutions (problem with changing loading state faster than 250 ms, using InteractionManager etc) seemed to point towards the same thing, that the refreshcontrol (still) cannot handle too fast changes. From my experiments, that still seems to be the case.

My refreshing prop comes from redux as a result of a network call, and in the cases where it returns too quickly (which happened often when using infinite scroll on emulator), the problem occured consistently. I tried doing something along the lines of

  const requestWasSentAt = Date.now();
  dispatch(setFetching());

  try {
      const json = await getStuffFromBackend();
      const requestFinishedAt = Date.now();
      const requestTime = requestFinishedAt - requestWasSentAt;
      const timeout =
        MINIMUM_TIME_BETWEEN_REFRESHING_PROP_UPDATES - requestTime;
      await setTimeout(() => {
        dispatch(setData(json));
      }, timeout > 0 ? timeout : 0);
    }

After some experimenting with values for MINIMUM_TIME_BETWEEN_REFRESHING_PROP_UPDATES, I found the sweet spot to be 350 ms. Using 300 ms, the problem could still be consistently reproduced in my app, but 350 fixes the issue.

I hope it helps someone, and that the issue might be fixed in core at some point. Tagging @janicduplessis - I appreciate the effort you've done already, but for some reason the fix is still buggy - can't tell you why, unfortunately :-/ Let me know if you need more code, but it's a very standard redux setup

  1. Dispatch loading
  2. Await network request
  3. Update redux with new data, set loading to false
  4. Hook up the refreshing FlatList prop to the redux boolean

Merry christmas and happy new year to you all from the past! :)

@maieonbrix
Copy link

I'm sorry to notify all of you but thank you a lot @jhalborg !

@tonnguyen
Copy link

Fixed mine also, thank you @jhalborg !

I agree RefreshControl cannot handle too fast change, at least on iOS. My solution, inspired from @jhalborg 's solution, is to turn my action into a thunk, which is basically a setTimeout call.

@sachabest
Copy link

Is this worth re-opening? Delaying incoming prop updates by a certain timeout does not seem like a stable, scalable, nor reliable solution.

@mkayswork
Copy link

mkayswork commented Apr 23, 2018

Why has this issue been closed? There still is no reliable solution for this. I guess a lot of us use a prop "loading" via redux and use this prop within many screens.

Unfortunately, with React Native 0.49.5 I am still facing this problem 😧

@inDream
Copy link

inDream commented May 15, 2018

It seems fixed after added [super sendActionsForControlEvents:UIControlEventValueChanged]; after

[super beginRefreshing];

For postinstall script:

sed -i '' 's/\[super beginRefreshing\];/\[super beginRefreshing\];\
\[super sendActionsForControlEvents:UIControlEventValueChanged\];/' \
./node_modules/react-native/React/Views/RCTRefreshControl.m

The reason is UIControlEventValueChanged isn't called in beginRefreshing, so _currentRefreshingState isn't updated.

@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

Successfully merging a pull request may close this issue.