Allows RefreshControl to be mounted with refreshing = true #5745
Conversation
RefreshControl did not start refreshing when refreshing was set to true initially. It also did not start refreshing on iOS when setting the prop from false to true without doing a pull to refresh gesture. This was a pain in the ass to make work on iOS because UIRefreshControl seems super sensitive to when beginRefreshing can be called, for the initial render I need to call it in layoutSubviews. I also have to manually adjust the scrollview content offset when calling beginRefreshing. The code is pretty hacky but it was the only solution I found that was actually working.
|
By analyzing the blame information on this pull request, we identified @janicduplessis, @bestander and @lexs to be potential reviewers. |
facebook-github-bot
added
GH Review: review-needed
CLA Signed
|
@TheBerg @nicklockwood ping |
| [scrollView setContentOffset:offset animated:NO]; | ||
| [super beginRefreshing]; | ||
| } else { | ||
| [UIView animateWithDuration:0.25 |
There was a problem hiding this comment.
Why use animateWithDuration here instead of setContentOffset:animated:YES ?
There was a problem hiding this comment.
(I daresay there's a good reason, but please add a comment explaining why if there is)
There was a problem hiding this comment.
The reason is that [super beginRefreshing] must be called after the animation is over otherwise it didn't work properly. I'll add a comment to explain this.
|
@janicduplessis updated the pull request. |
|
Ok, I made the changes you suggested, now there is pretty much 1 comment per line of code, speaks to how weird the changes had to be to work :) Let me know if anything isn't clear. |
|
This looks amazing! 👍 |
|
👍 @facebook-github-bot shipit |
|
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/567468353403947/int_phab to review. |
|
@janicduplessis updated the pull request. |
|
I made one last change to use |
ghost
closed this
in
|
@janicduplessis too late, but thanks :-) |
Summary: RefreshControl did not start refreshing when refreshing was set to true initially. It also did not start refreshing on iOS when setting the prop from false to true without doing a pull to refresh gesture. This was a pain in the ass to make work on iOS because UIRefreshControl seems super sensitive to when beginRefreshing can be called, for the initial render I need to call it in layoutSubviews. I also have to manually adjust the scrollview content offset when calling beginRefreshing. The code is a bit hacky but it was the only solution I found that was actually working. Fixes facebook#5716 Closes facebook#5745 Reviewed By: svcscm Differential Revision: D2910716 Pulled By: nicklockwood fb-gh-sync-id: d60e73bcfe8d86bb01249ba5f17e6a23c5a5aff6
|
still have a problem，if you first setRefreshing true and then setRefreshing false,sometimes layoutSubviews function not be called, so the refreshcontrol would refresh for all |
|
Reproduced the original test case #5716 on versions from 0.34.1 to 0.37.0-rc.0 including. |
|
@janicduplessis @nicklockwood I just tested this in the official React-Native docs using the in-browser previews, and setting the initial Did something break again, or was it never fixed? |
|
Any update on this issue? This is still occurring on iOS |
RefreshControl did not start refreshing when refreshing was set to true initially. It also did not start refreshing on iOS when setting the prop from false to true without doing a pull to refresh gesture.
This was a pain in the ass to make work on iOS because UIRefreshControl seems super sensitive to when beginRefreshing can be called, for the initial render I need to call it in layoutSubviews. I also have to manually adjust the scrollview content offset when calling beginRefreshing. The code is a bit hacky but it was the only solution I found that was actually working.
Fixes #5716