Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

RefreshControl offset not functioning correctly for iOS. #14056

Closed
YoranRoels opened this issue May 19, 2017 · 4 comments
Closed

RefreshControl offset not functioning correctly for iOS. #14056

YoranRoels opened this issue May 19, 2017 · 4 comments
Labels
Help Wanted :octocat: Issues ideal for external contributors. Platform: iOS iOS applications. Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@YoranRoels
Copy link

YoranRoels commented May 19, 2017

Description

When using a ListView / ScrollView with a RefreshControl setting the refreshing prop to true causes the RefreshControl to open appropriately. When doing this again, however, the RefreshControl will not show until the refreshing prop is set back to false.

Reproduction Steps and Sample Code

First of all, because a good example is better than explaining this with a wall of text, here's an example:
https://github.com/YoranRoels/react-native-refresh-control-issue

Clicking on the Refreshing 1 button will start refreshing correctly, then clicking on the Refreshing 0 button will turn off refreshing correctly. When then pressing the Refreshing 1 button AGAIN, the RefreshControl will not show until pressing the Refreshing 0 button again (it will show for a short moment).

Me and my co-worker have tracked this issue down to a problem with only the iOS RefreshControl and the offset.

If you want to log the offsets in the react-native/React/Views/RCTRefreshControl.m, use this code snippet (You need to uncomment the NSLogs):

/**
 * Copyright (c) 2015-present, Facebook, Inc.
 * All rights reserved.
 *
 * This source code is licensed under the BSD-style license found in the
 * LICENSE file in the root directory of this source tree. An additional grant
 * of patent rights can be found in the PATENTS file in the same directory.
 */

#import "RCTRefreshControl.h"

#import "RCTUtils.h"

@implementation RCTRefreshControl {
  BOOL _isInitialRender;
  BOOL _currentRefreshingState;
  NSString *_title;
  UIColor *_titleColor;
}

- (instancetype)init
{
  if ((self = [super init])) {
    [self addTarget:self action:@selector(refreshControlValueChanged) forControlEvents:UIControlEventValueChanged];
    _isInitialRender = true;
    _currentRefreshingState = false;
  }
  return self;
}

RCT_NOT_IMPLEMENTED(- (instancetype)initWithCoder:(NSCoder *)aDecoder)

- (void)layoutSubviews
{
  [super layoutSubviews];

  // Fix for bug #7976
  // TODO: Remove when updating to use iOS 10 refreshControl UIScrollView prop.
  if (self.backgroundColor == nil) {
    self.backgroundColor = [UIColor clearColor];
  }

  // If the control is refreshing when mounted we need to call
  // beginRefreshing in layoutSubview or it doesn't work.
  if (_currentRefreshingState && _isInitialRender) {
    [self beginRefreshing];
  }
  _isInitialRender = false;
}

- (void)beginRefreshing
{
  // NSLog(@"beginRefreshing");
  // When using begin refreshing we need to adjust the ScrollView content offset manually.
  UIScrollView *scrollView = (UIScrollView *)self.superview;
  CGPoint offset = {scrollView.contentOffset.x, scrollView.contentOffset.y - self.frame.size.height};
  //CGPoint offset = {scrollView.contentOffset.x, scrollView.contentOffset.y - 60};

  // NSLog(@"beginRefreshing: offset X %f", scrollView.contentOffset.x);
  // NSLog(@"beginRefreshing: offset Y %f", scrollView.contentOffset.y - self.frame.size.height);
  // NSLog(@"beginRefreshing: offset Y - connentoffset %f", scrollView.contentOffset.y);
  // NSLog(@"beginRefreshing: offset Y - frame.size %f", 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) {
                       [super beginRefreshing];
                     }];
}

- (void)endRefreshing
{
    // NSLog(@"endRefreshing");
  // The contentOffset of the scrollview MUST be greater than 0 before calling
  // endRefreshing otherwise the next pull to refresh will not work properly.
  UIScrollView *scrollView = (UIScrollView *)self.superview;
  // NSLog(@"endRefreshing: scrollView.contentOffset.y %f", scrollView.contentOffset.y);

  if (scrollView.contentOffset.y < 0) {
    CGPoint offset = {scrollView.contentOffset.x, -scrollView.contentInset.top};

    // NSLog(@"endRefreshing: offset X %f", scrollView.contentOffset.x);
    // NSLog(@"endRefreshing: offset Scrollview TOP %f", scrollView.contentInset.top);

    [UIView animateWithDuration:0.25
                          delay:0
                        options:UIViewAnimationOptionBeginFromCurrentState
                     animations:^(void) {
                       [scrollView setContentOffset:offset];
                     } completion:^(__unused BOOL finished) {
                       [super endRefreshing];
                     }];
  } else {
    // NSLog(@"endRefreshing: super");
    [super endRefreshing];
  }
}

- (NSString *)title
{
  return _title;
}

- (void)setTitle:(NSString *)title
{
  _title = title;
  [self _updateTitle];
}

- (void)setTitleColor:(UIColor *)color
{
  _titleColor = color;
  [self _updateTitle];
}

- (void)_updateTitle
{
  if (!_title) {
    return;
  }

  NSMutableDictionary *attributes = [NSMutableDictionary dictionary];
  if (_titleColor) {
    attributes[NSForegroundColorAttributeName] = _titleColor;
  }

  self.attributedTitle = [[NSAttributedString alloc] initWithString:_title attributes:attributes];
}

- (void)setRefreshing:(BOOL)refreshing
{
  // NSLog(@"setRefreshing %d", refreshing);

  if (_currentRefreshingState != refreshing) {
    _currentRefreshingState = refreshing;

    if (refreshing) {
      if (!_isInitialRender) {
        [self beginRefreshing];
      }
    } else {
      [self endRefreshing];
    }
  }
}

- (void)refreshControlValueChanged
{
    // NSLog(@"refreshControlValueChanged");
  _currentRefreshingState = super.refreshing;

  if (_onRefresh) {
    _onRefresh(nil);
  }
}

@end

With these logs we found out that the problem is in-fact that the Y offset is not set correctly. The first time (in our case) it set the Y offset to -60 on the first refresh which shows the refresh correctly but on the second refresh it logs the Y offset as -0.5.

We actually set the Y offset to a steady 60 in the beginRefreshing function and then the issue doesn't occur. (Hard coding this is obviously not the solution, it's just to test if the RefreshControl is actually showing, and it is, the offset is just not set correctly to show it).

So if you look at my example project REALLY closely, you will see the ScrollView move just a tad bit when clicking the Refreshing 1 button for the second time.

Solution

The offset should always be set correctly when the refreshing boolean is true.

Additional Information

  • React Native version: "react-native": "0.44.0"
  • Platform: iOS
  • Development Operating System: macOS
  • Dev tools: Xcode & Atom
@janicduplessis janicduplessis added Platform: iOS iOS applications. Help Wanted :octocat: Issues ideal for external contributors. labels May 30, 2017
@janicduplessis
Copy link
Contributor

Thanks for investigating the issue, do you have an idea how it could be fixed? A PR would definetly be appreciated since I probably won't have time to look at this for a while.

@bdrobinson
Copy link

Any updates on this? I'm running into this too.

@ngandhy
Copy link
Contributor

ngandhy commented Oct 18, 2017

@janicduplessis (& @shergin) any update on this? A few issues (e.g - #15892) and PRs have been opened for this. PR #14259, #10747, #15033 seem to be rejected with no follow up.

@stale
Copy link

stale bot commented Dec 17, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Dec 17, 2017
@stale stale bot closed this as completed Dec 24, 2017
@facebook facebook locked and limited conversation to collaborators May 15, 2018
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Help Wanted :octocat: Issues ideal for external contributors. Platform: iOS iOS applications. Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests

4 participants