Skip to content

Commit f6516d2

Browse files
zhongwuzwgrabbou
zhongwuzw
authored and
grabbou
committedMar 22, 2019
Fix scrollview over bounds of content size (#23427)
Summary: Fix scrollview `offset` out of content size in iOS, Android uses `scrollTo` and `smoothScrollTo` which not have this issue. Fixes like #13594 #22768 #19970 . [iOS] [Fixed] - Fixed scrollView offset out of content size. Pull Request resolved: #23427 Differential Revision: D14162663 Pulled By: cpojer fbshipit-source-id: a95371c8d703b6d5f604af0072f86c01c2018f4a
·
v0.59.10v0.59.2
1 parent 699fad7 commit f6516d2

File tree

1 file changed

+11
-0
lines changed

1 file changed

+11
-0
lines changed
 

‎React/Views/ScrollView/RCTScrollView.m

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,8 +588,19 @@ - (void)scrollToOffset:(CGPoint)offset
588588
- (void)scrollToOffset:(CGPoint)offset animated:(BOOL)animated
589589
{
590590
if (!CGPointEqualToPoint(_scrollView.contentOffset, offset)) {
591+
CGRect maxRect = CGRectMake(fmin(-_scrollView.contentInset.left, 0),
592+
fmin(-_scrollView.contentInset.top, 0),
593+
fmax(_scrollView.contentSize.width - _scrollView.bounds.size.width + _scrollView.contentInset.right + fmax(_scrollView.contentInset.left, 0), 0.01),
594+
fmax(_scrollView.contentSize.height - _scrollView.bounds.size.height + _scrollView.contentInset.bottom + fmax(_scrollView.contentInset.top, 0), 0.01)); // Make width and height greater than 0
591595
// Ensure at least one scroll event will fire
592596
_allowNextScrollNoMatterWhat = YES;
597+
if (!CGRectContainsPoint(maxRect, offset)) {
598+
CGFloat x = fmax(offset.x, CGRectGetMinX(maxRect));
599+
x = fmin(x, CGRectGetMaxX(maxRect));
600+
CGFloat y = fmax(offset.y, CGRectGetMinY(maxRect));
601+
y = fmin(y, CGRectGetMaxY(maxRect));
602+
offset = CGPointMake(x, y);
603+
}
593604
[_scrollView setContentOffset:offset animated:animated];
594605
}
595606
}

9 commit comments

Comments
 (9)

mysport12 commented on Mar 28, 2019

@mysport12
mysport12
on Mar 28, 2019edited by mysport12
Contributor

Any chance this behavior can be made optional (with an opt-in/opt-out prop depending upon the default)? In our apps, we use the over scroll ability to move content out of the way of the virtual keyboard. Not sure if we are the exception here but it may impact other projects that were relying on this ability. @zhongwuzw

zhongwuzw commented on Mar 29, 2019

@zhongwuzw
zhongwuzw
on Mar 29, 2019
ContributorAuthor

I think you can use contentInset to adjust scroll area?

mysport12 commented on Mar 29, 2019

@mysport12
mysport12
on Mar 29, 2019
Contributor

If you do that then the user can scroll all the way down to it. We only want to permit the over scroll programmatically when the onFocus event is fired from a TextInput on the screen.

mysport12 commented on Mar 29, 2019

@mysport12
mysport12
on Mar 29, 2019
Contributor

I can see a way of doing it with contentInset assuming a new state variable is created in my components and keyboard event listeners are used but I was hoping to not go down that road if possible. I can make an attempt at a new prop to opt out of this behavior and create a PR proposing the change. My native skills are shaky at best but I'll give it a go.

fenghengzhi commented on May 7, 2019

@fenghengzhi
fenghengzhi
on May 7, 2019

it's better to add a new boolean param (false by default) to scrollTo method than just change it's old behavior

mysport12 commented on May 7, 2019

@mysport12
mysport12
on May 7, 2019
Contributor

A new Boolean param was added that allows for opting back in to the old behavior

jjhampton commented on Sep 5, 2019

@jjhampton
jjhampton
on Sep 5, 2019

@mysport12 Do you know where the new Boolean param is at, and where it can be set, which allows for opting back in to the old behavior? The app I'm working on is experiencing some unintended side-effects which appear to be caused by this change.

jjhampton commented on Sep 5, 2019

@jjhampton
jjhampton
on Sep 5, 2019

@mysport12 ^ Disregard the above question, I see it in RN 0.59.4: e3ac329 .

mysport12 commented on Sep 5, 2019

@mysport12
mysport12
on Sep 5, 2019
Contributor

@jjhampton To opt back in to the old behavior you can pass a prop to your ScrollView.

scrollToOverflowEnabled={true}

Or set it globally like we do in our apps. In App.js set the defaultProps outside of your component with something like this...

ScrollView.defaultProps = {
...ScrollView.defaultProps,
scrollToOverflowEnabled: true
}

Hope that helps!

Please sign in to comment.