Skip to content

Commit

Permalink
Fix: Added scroll Bounds check in scrollToOffset method in RCTScrollV…
Browse files Browse the repository at this point in the history
…iew on iOS

Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

The scrollToOffset method of RCTScrollView for iOS does not include bound check for the scroll offset provided to the method. This can cause the whole view to scroll out of screen if the offset provided is greater than the bounds of the View.
The same does not happen on Android, where the scroll halts once the last item of the scrollView is in the viewport.
I have added bounds check so the offset resets to the MaxOffset which makes sure the view does not scroll out of the viewport.

The issue can be observed in the following snack:
https://snack.expo.io/H1363Uo8f

![ezgif com-optimize 1](https://user-images.githubusercontent.com/16662518/36068270-2437ae88-0ef7-11e8-96dd-819b4ae0fd67.gif)

To test my changes I ran the code provided in the snack above with the react-native dependency pointing to my branch. As can be see in the video attached below, the scroll halts once it hits the end of the ScrollView even if the scroll offset provided is higher than the bounds of the ScrollView. It also does not scroll up for negative scroll offset.

![ezgif com-optimize](https://user-images.githubusercontent.com/16662518/36068130-9ae4f918-0ef3-11e8-8728-af7b2888bdb8.gif)

[IOS] [BUGFIX] [ScrollView] - Added bounds check to prevent ScrollView from scrolling to an offset which is out of bounds of the ScrollView for iOS.
Closes #17927

Differential Revision: D7014466

Pulled By: shergin

fbshipit-source-id: a817738d08e1057a4c70f43373132f88fa1461c4
  • Loading branch information
siddhantsoni authored and facebook-github-bot committed Feb 26, 2018
1 parent 59c7b2c commit 16c9e5b
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions React/Views/ScrollView/RCTScrollView.m
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,9 @@ - (void)scrollToOffset:(CGPoint)offset
- (void)scrollToOffset:(CGPoint)offset animated:(BOOL)animated
{
if (!CGPointEqualToPoint(_scrollView.contentOffset, offset)) {
CGFloat maxOffsetX = _scrollView.contentSize.width - _scrollView.bounds.size.width + _scrollView.contentInset.right;
CGFloat maxOffsetY = _scrollView.contentSize.height - _scrollView.bounds.size.height + _scrollView.contentInset.bottom;
offset = CGPointMake(MAX(0, MIN(maxOffsetX, offset.x)), MAX(0, MIN(maxOffsetY, offset.y)));
// Ensure at least one scroll event will fire
_allowNextScrollNoMatterWhat = YES;
[_scrollView setContentOffset:offset animated:animated];
Expand Down

0 comments on commit 16c9e5b

Please sign in to comment.