Fix refreshing state

[cpunion]

When RefreshControl.refreshing change twice within 250ms, it ignores the second changing.

Test plan (required)

refresh () {
  this.setState({
    refreshing: true
  })

  fetch('/api')
  .then(() => {
    this.setState({
      refreshing: false
    })
  })
  .catch((error) => {
    this.setState({
      refreshing: false
    })
  })
}

render() {
  return (
    <ScrollView
      refreshControl={
        <RefreshControl
          refreshing={this.state.refreshing}
          onRefresh={this.refresh.bind(this)}
        />
      }>
      <TouchableHighlight onPress={this.refresh.bind(this)}>
        <View>
          <Text>Touch Me!</Text>
        </View>
      </TouchableHighlight>
    </ScrollView>
  )
}

• Test Case 1: Touch "Touch Me!", if get response less than 250ms, the state is always refreshing.
• Test Case 2: Close network, Touch "Touch Me!", the state is always refreshing.

[facebook-github-bot]

By analyzing the blame information on this pull request, we identified @janicduplessis to be a potential reviewer.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

[janicduplessis]

Thanks @cpunion, LGTM

Cc @nicklockwood

[cpunion]

Add a RNPLAY: https://rnplay.org/apps/_Pb4hA

[cpunion]

This patch brings another problem, when pull down to refresh, the view will shake, because _refreshingState not updated to true when pull down. If updates it in refreshControlValueChanged, it not CURRENT state. So we need updates _refreshingState when pull down, but cannot through refreshing prop.

Then DO NOT USE THIS PATCH NOW.

[janicduplessis]

You can check super.refreshing in refreshControlValueChanged to update the value I think.

[cpunion]

@janicduplessis super.refreshing is delayed by animation, can't check if it changed by pull down. In refreshControlValueChanged only can do _refreshingState = super.refreshing, it just past simple tests, but I'm not sure it's right.

A example sequence:

1. Pull down
2. setState({freshing: true})
3. _freshingState = true
4. do animation
5. call refreshControlValueChanged after 250ms
6. _freshingSate = true

If finish an API call after 10ms of pull down:

1. Pull down
2. setState({freshing: true})
3. _freshingState = true
4. do animation
5. setState({freshing: false})
6. _freshingState = false
7. do animation
8. call refreshControlValueChanged after 250ms
9. _freshingSate = true
10. call refreshControlValueChanged after 260ms
11. _freshingSate = false

_freshingState changed to true, false, true, false, seems not good.

[janicduplessis]

What should happen normally is:

1. Pull down
2. refreshControlValueChanged is called with self.refreshing = true. At this point we need to set _currentRefreshingState to true.
3. setState({refreshing: true}) is called then setRefreshing needs to do nothing because _currentRefreshingState == refreshing. (this line)
4. setState({refreshing: false}) -> setRefreshing -> _currentRefreshingState = false
5. endRefresing is called, 250ms anime -> self.refreshing = false

It's important that beginRefreshing is not called when doing a pull down because it is already refreshing and we don't want to do the animation.

[cpunion]

You are right, I did some tests and traced state changes, seems OK.

[facebook-github-bot]

@cpunion updated the pull request.

[facebook-github-bot]

@cpunion updated the pull request.

[facebook-github-bot]

@cpunion updated the pull request.

[cpunion]

@janicduplessis Can't pass CI, I don't know how to fix it.

[janicduplessis]

Can you try rebasing on master? The failure seems unrelated.

[facebook-github-bot]

@cpunion updated the pull request.

[cpunion]

@janicduplessis passed. Would be merge? or need improve?

[janicduplessis]

Looks good, I'll test it and we can get this merged!

[janicduplessis]

@facebook-github-bot shipit

[facebook-github-bot]

Thanks for importing. If you are an FB employee go to Phabricator to review.