Skip to content

Commit

Permalink
Fix ScrollView centerContent losing taps and causing jitter on iOS (#…
Browse files Browse the repository at this point in the history 
…47591)

Summary:
The React Native `<ScrollView>` has a peculiar `centerContent` prop. It solves a common need — keeping the image "centered" while it's not fully zoomed in (but then allowing full panning after it's sufficiently zoomed in).

This prop sort of works but it has a few wonky behaviors:

- If you start tapping immediately after pinch (and don't stop), the taps will not be recognized until a second after you stop tapping. I suspect this is because the existing `centerContent` implementation hijacks the `contentOffset` setter, but the calling UIKit code _does not know_ it's being hijacked, and so the calling UIKit code _thinks_ it needs to do a momentum animation. This (invisible) momentum animation causes the scroll view to keep eating the tap touches.
- While you're zooming in, once you cross the threshold where `contentOffset` hijacking stops adjusting values, there will be a sudden visual jump during the pinch. This is because the "real" `contentOffset` tracks the accumulated translation from the pinch gesture, and once it gets taken into account with no "correction", the new offset snaps into place.
- While not sufficiently pinched in, the vertical axis is completely rigid. It does not have the natural rubber banding.

The solution to all of these issues is described [here](https://petersteinberger.com/blog/2013/how-to-center-uiscrollview/). Instead of hijacking `contentOffset`, it is more reliable to track zooming, child view, and frame changes, and adjust `contentInsets` instead. This solves all three issues:

- UIKit isn't confused by the content offset changing from under it so it doesn't mistrigger a faux momentum animation.
- There is no sudden jump because it's the insets that are being adjusted.
- Rubber banding just works.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[IOS] [FIXED] - Fixed centerContent losing taps and causing jitter

Pull Request resolved: #47591

Test Plan:
I'm extracting this from [a patch we're applying to Bluesky](https://github.com/bluesky-social/social-app/blob/2ef697fe3d7dec198544ed6834553f33b95790b3/patches/react-native%2B0.74.1.patch). I'll be honest — I have not tested this in isolation, and it likely requires some testing to get merged in. I do not, unfortuntately, have the capacity to do it myself so this is more of a "throw over the wall" kind of patch. Maybe it will be helpful to somebody else.

I've tested these in our real open source app (bluesky-social/social-app#6298). You can reproduce it in any of the lightboxes in the feed or the profile.

### Before the fix

Observe the failing tap gestures, sudden jump while pinching, lack of rubber banding.

https://github.com/user-attachments/assets/c9883201-c9f0-4782-9b80-8e0a9f77c47c

### After the fix

Observe the natural iOS behavior.

https://github.com/user-attachments/assets/c025e1df-6963-40ba-9e28-d48bfa5e631d

Unfortunately I do not have the capacity to verify this fix in other scenarios outside of our app.

Reviewed By: sammy-SC

Differential Revision: D66093472

Pulled By: javache

fbshipit-source-id: 064f0415b8093ff55cb51bdebab2a46ee97f8fa9
  • Loading branch information
@gaearon @facebook-github-bot
gaearon authored and facebook-github-bot committed Nov 25, 2024
1 parent 2a2f58a commit fe7e97a
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 29 deletions.
49 changes: 39 additions & 10 deletions ...ges/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTEnhancedScrollView.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,28 +67,52 @@ - (void)preserveContentOffsetWithBlock:(void (^)())block
* ScrollView, we force it to be centered, but when you zoom or the content otherwise
* becomes larger than the ScrollView, there is no padding around the content but it
* can still fill the whole view.
* This implementation is based on https://petersteinberger.com/blog/2013/how-to-center-uiscrollview/.
*/
- (void)setContentOffset:(CGPoint)contentOffset
- (void)centerContentIfNeeded
{
if (_isSetContentOffsetDisabled) {
if (!_centerContent) {
return;
}

if (_centerContent && !CGSizeEqualToSize(self.contentSize, CGSizeZero)) {
CGSize scrollViewSize = self.bounds.size;
if (self.contentSize.width <= scrollViewSize.width) {
contentOffset.x = -(scrollViewSize.width - self.contentSize.width) / 2.0;
}
if (self.contentSize.height <= scrollViewSize.height) {
contentOffset.y = -(scrollViewSize.height - self.contentSize.height) / 2.0;
}
CGSize contentSize = self.contentSize;
CGSize boundsSize = self.bounds.size;
if (CGSizeEqualToSize(contentSize, CGSizeZero) || CGSizeEqualToSize(boundsSize, CGSizeZero)) {
return;
}

CGFloat top = 0, left = 0;
if (contentSize.width < boundsSize.width) {
left = (boundsSize.width - contentSize.width) * 0.5f;
}
if (contentSize.height < boundsSize.height) {
top = (boundsSize.height - contentSize.height) * 0.5f;
}
self.contentInset = UIEdgeInsetsMake(top, left, top, left);
}

- (void)setContentOffset:(CGPoint)contentOffset
{
if (_isSetContentOffsetDisabled) {
return;
}
super.contentOffset = CGPointMake(
RCTSanitizeNaNValue(contentOffset.x, @"scrollView.contentOffset.x"),
RCTSanitizeNaNValue(contentOffset.y, @"scrollView.contentOffset.y"));
}

- (void)setFrame:(CGRect)frame
{
[super setFrame:frame];
[self centerContentIfNeeded];
}

- (void)didAddSubview:(UIView *)subview
{
[super didAddSubview:subview];
[self centerContentIfNeeded];
}

- (BOOL)touchesShouldCancelInContentView:(UIView *)view
{
if ([_overridingDelegate respondsToSelector:@selector(touchesShouldCancelInContentView:)]) {
Expand Down Expand Up @@ -258,6 +282,11 @@ - (void)scrollViewWillEndDragging:(UIScrollView *)scrollView
}
}

- (void)scrollViewDidZoom:(__unused UIScrollView *)scrollView
{
[self centerContentIfNeeded];
}

#pragma mark -

- (BOOL)isHorizontal:(UIScrollView *)scrollView
Expand Down
65 changes: 46 additions & 19 deletions packages/react-native/React/Views/ScrollView/RCTScrollView.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,26 +159,8 @@ - (BOOL)touchesShouldCancelInContentView:(__unused UIView *)view
return !shouldDisableScrollInteraction;
}

/*
* Automatically centers the content such that if the content is smaller than the
* ScrollView, we force it to be centered, but when you zoom or the content otherwise
* becomes larger than the ScrollView, there is no padding around the content but it
* can still fill the whole view.
*/
- (void)setContentOffset:(CGPoint)contentOffset
{
UIView *contentView = [self contentView];
if (contentView && _centerContent && !CGSizeEqualToSize(contentView.frame.size, CGSizeZero)) {
CGSize subviewSize = contentView.frame.size;
CGSize scrollViewSize = self.bounds.size;
if (subviewSize.width <= scrollViewSize.width) {
contentOffset.x = -(scrollViewSize.width - subviewSize.width) / 2.0;
}
if (subviewSize.height <= scrollViewSize.height) {
contentOffset.y = -(scrollViewSize.height - subviewSize.height) / 2.0;
}
}

super.contentOffset = CGPointMake(
RCTSanitizeNaNValue(contentOffset.x, @"scrollView.contentOffset.x"),
RCTSanitizeNaNValue(contentOffset.y, @"scrollView.contentOffset.y"));
Expand Down Expand Up @@ -433,6 +415,12 @@ - (void)setRemoveClippedSubviews:(__unused BOOL)removeClippedSubviews
// Does nothing
}

- (void)setFrame:(CGRect)frame
{
[super setFrame:frame];
[self centerContentIfNeeded];
}

- (void)insertReactSubview:(UIView *)view atIndex:(NSInteger)atIndex
{
[super insertReactSubview:view atIndex:atIndex];
Expand All @@ -450,6 +438,8 @@ - (void)insertReactSubview:(UIView *)view atIndex:(NSInteger)atIndex
RCTApplyTransformationAccordingLayoutDirection(_contentView, self.reactLayoutDirection);
[_scrollView addSubview:view];
}

[self centerContentIfNeeded];
}

- (void)removeReactSubview:(UIView *)subview
Expand Down Expand Up @@ -658,9 +648,45 @@ -(void)delegateMethod : (UIScrollView *)scrollView \
}

RCT_SCROLL_EVENT_HANDLER(scrollViewWillBeginDecelerating, onMomentumScrollBegin)
RCT_SCROLL_EVENT_HANDLER(scrollViewDidZoom, onScroll)
RCT_SCROLL_EVENT_HANDLER(scrollViewDidScrollToTop, onScrollToTop)

- (void)scrollViewDidZoom:(UIScrollView *)scrollView
{
[self centerContentIfNeeded];

RCT_SEND_SCROLL_EVENT(onScroll, nil);
RCT_FORWARD_SCROLL_EVENT(scrollViewDidZoom : scrollView);
}

/*
* Automatically centers the content such that if the content is smaller than the
* ScrollView, we force it to be centered, but when you zoom or the content otherwise
* becomes larger than the ScrollView, there is no padding around the content but it
* can still fill the whole view.
* This implementation is based on https://petersteinberger.com/blog/2013/how-to-center-uiscrollview/.
*/
- (void)centerContentIfNeeded
{
if (!_scrollView.centerContent) {
return;
}

CGSize contentSize = self.contentSize;
CGSize boundsSize = self.bounds.size;
if (CGSizeEqualToSize(contentSize, CGSizeZero) || CGSizeEqualToSize(boundsSize, CGSizeZero)) {
return;
}

CGFloat top = 0, left = 0;
if (contentSize.width < boundsSize.width) {
left = (boundsSize.width - contentSize.width) * 0.5f;
}
if (contentSize.height < boundsSize.height) {
top = (boundsSize.height - contentSize.height) * 0.5f;
}
_scrollView.contentInset = UIEdgeInsetsMake(top, left, top, left);
}

- (void)addScrollListener:(NSObject<UIScrollViewDelegate> *)scrollListener
{
[_scrollListeners addObject:scrollListener];
Expand Down Expand Up @@ -939,6 +965,7 @@ - (void)updateContentSizeIfNeeded
CGSize contentSize = self.contentSize;
if (!CGSizeEqualToSize(_scrollView.contentSize, contentSize)) {
_scrollView.contentSize = contentSize;
[self centerContentIfNeeded];
}
}

Expand Down

0 comments on commit fe7e97a

Please sign in to comment.