diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm index 14a08b49cb7c..7b579c9d0712 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm @@ -84,6 +84,21 @@ static UIScrollViewIndicatorStyle RCTUIScrollViewIndicatorStyleFromProps(const S userInfo:userInfo]; } +// Clamps a maintainVisibleContentPosition target offset to the scrollable range so that +// restoring a pre-mount offset after content shrinks does not overscroll. +static CGPoint RCTClampMaintainVisibleContentOffset(UIScrollView *scrollView, CGPoint offset) +{ + UIEdgeInsets insets = scrollView.adjustedContentInset; + + CGFloat minX = -insets.left; + CGFloat maxX = fmax(minX, scrollView.contentSize.width - scrollView.bounds.size.width + insets.right); + + CGFloat minY = -insets.top; + CGFloat maxY = fmax(minY, scrollView.contentSize.height - scrollView.bounds.size.height + insets.bottom); + + return CGPointMake(fmin(fmax(offset.x, minX), maxX), fmin(fmax(offset.y, minY), maxY)); +} + @interface RCTScrollViewComponentView () < UIScrollViewDelegate, RCTScrollViewProtocol, @@ -110,6 +125,7 @@ @implementation RCTScrollViewComponentView { __weak UIView *_contentView; CGRect _prevFirstVisibleFrame; + CGPoint _prevContentOffset; __weak UIView *_firstVisibleView; NSInteger _firstVisibleViewTag; @@ -708,6 +724,7 @@ - (void)prepareForRecycle self.frame = oldFrame; _contentView = nil; _prevFirstVisibleFrame = CGRectZero; + _prevContentOffset = CGPointZero; _firstVisibleView = nil; _firstVisibleViewTag = 0; _virtualViewContainerState = nil; @@ -1077,6 +1094,8 @@ - (void)_prepareForMaintainVisibleScrollPosition } if (hasNewView || ii == _contentView.subviews.count - 1) { _prevFirstVisibleFrame = subview.frame; + // A smaller content size can clamp the live offset before the adjustment. + _prevContentOffset = _scrollView.contentOffset; _firstVisibleView = subview; _firstVisibleViewTag = subview.tag; break; @@ -1120,9 +1139,10 @@ - (void)_adjustForMaintainVisibleContentPosition if (horizontal) { CGFloat deltaX = _firstVisibleView.frame.origin.x - _prevFirstVisibleFrame.origin.x; if (ABS(deltaX) > 0.5) { - CGFloat x = _scrollView.contentOffset.x; + CGFloat x = _prevContentOffset.x; [self _forceDispatchNextScrollEvent]; - _scrollView.contentOffset = CGPointMake(_scrollView.contentOffset.x + deltaX, _scrollView.contentOffset.y); + CGPoint targetOffset = CGPointMake(x + deltaX, _scrollView.contentOffset.y); + _scrollView.contentOffset = RCTClampMaintainVisibleContentOffset(_scrollView, targetOffset); if (autoscrollThreshold) { // If the offset WAS within the threshold of the start, animate to the start. if (x <= autoscrollThreshold.value()) { @@ -1134,9 +1154,10 @@ - (void)_adjustForMaintainVisibleContentPosition CGRect newFrame = _firstVisibleView.frame; CGFloat deltaY = newFrame.origin.y - _prevFirstVisibleFrame.origin.y; if (ABS(deltaY) > 0.5) { - CGFloat y = _scrollView.contentOffset.y; + CGFloat y = _prevContentOffset.y; [self _forceDispatchNextScrollEvent]; - _scrollView.contentOffset = CGPointMake(_scrollView.contentOffset.x, _scrollView.contentOffset.y + deltaY); + CGPoint targetOffset = CGPointMake(_scrollView.contentOffset.x, y + deltaY); + _scrollView.contentOffset = RCTClampMaintainVisibleContentOffset(_scrollView, targetOffset); if (autoscrollThreshold) { // If the offset WAS within the threshold of the start, animate to the start. if (y <= autoscrollThreshold.value()) { diff --git a/packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm b/packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm index 82b350755668..025be37f11fe 100644 --- a/packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm +++ b/packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm @@ -25,6 +25,8 @@ @interface RCTScrollViewComponentView (Tests) - (void)_keyboardWillChangeFrame:(NSNotification *)notification; +- (void)_prepareForMaintainVisibleScrollPosition; +- (void)_adjustForMaintainVisibleContentPosition; @end @interface RCTScrollViewComponentViewTests : XCTestCase @@ -32,6 +34,108 @@ @interface RCTScrollViewComponentViewTests : XCTestCase @implementation RCTScrollViewComponentViewTests +- (void)testMaintainVisibleContentPositionAfterVerticalShrink +{ + RCTScrollViewComponentView *view = [[RCTScrollViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)]; + auto props = std::make_shared(); + props->maintainVisibleContentPosition = facebook::react::ScrollViewMaintainVisibleContentPosition{}; + [view updateProps:props oldProps:ScrollViewShadowNode::defaultSharedProps()]; + + RCTViewComponentView *contentView = [[RCTViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 1000)]; + [view mountChildComponentView:contentView index:0]; + UIView *anchor = [[UIView alloc] initWithFrame:CGRectMake(0, 800, 100, 40)]; + anchor.tag = 42; + [contentView addSubview:anchor]; + + view.scrollView.contentSize = CGSizeMake(100, 1000); + view.scrollView.contentOffset = CGPointMake(0, 800); + [view _prepareForMaintainVisibleScrollPosition]; + + anchor.frame = CGRectMake(0, 300, 100, 40); + view.scrollView.contentSize = CGSizeMake(100, 400); + view.scrollView.contentOffset = CGPointZero; + [view _adjustForMaintainVisibleContentPosition]; + + XCTAssertEqualWithAccuracy(view.scrollView.contentOffset.y, 300, 0.5); +} + +- (void)testMaintainVisibleContentPositionAfterHorizontalShrink +{ + RCTScrollViewComponentView *view = [[RCTScrollViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)]; + auto props = std::make_shared(); + props->maintainVisibleContentPosition = facebook::react::ScrollViewMaintainVisibleContentPosition{}; + [view updateProps:props oldProps:ScrollViewShadowNode::defaultSharedProps()]; + + RCTViewComponentView *contentView = [[RCTViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 1000, 100)]; + [view mountChildComponentView:contentView index:0]; + UIView *anchor = [[UIView alloc] initWithFrame:CGRectMake(800, 0, 40, 100)]; + anchor.tag = 42; + [contentView addSubview:anchor]; + + view.scrollView.contentSize = CGSizeMake(1000, 100); + view.scrollView.contentOffset = CGPointMake(800, 0); + [view _prepareForMaintainVisibleScrollPosition]; + + anchor.frame = CGRectMake(300, 0, 40, 100); + view.scrollView.contentSize = CGSizeMake(400, 100); + view.scrollView.contentOffset = CGPointZero; + [view _adjustForMaintainVisibleContentPosition]; + + XCTAssertEqualWithAccuracy(view.scrollView.contentOffset.x, 300, 0.5); +} + +- (void)testMaintainVisibleContentPositionClampsVerticalOffsetAfterShrink +{ + RCTScrollViewComponentView *view = [[RCTScrollViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)]; + auto props = std::make_shared(); + props->maintainVisibleContentPosition = facebook::react::ScrollViewMaintainVisibleContentPosition{}; + [view updateProps:props oldProps:ScrollViewShadowNode::defaultSharedProps()]; + + RCTViewComponentView *contentView = [[RCTViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 1000)]; + [view mountChildComponentView:contentView index:0]; + UIView *anchor = [[UIView alloc] initWithFrame:CGRectMake(0, 800, 100, 40)]; + anchor.tag = 42; + [contentView addSubview:anchor]; + + view.scrollView.contentSize = CGSizeMake(100, 1000); + view.scrollView.contentOffset = CGPointMake(0, 800); + [view _prepareForMaintainVisibleScrollPosition]; + + // The unclamped target (350) is past the max offset of 400 - 100 = 300. + anchor.frame = CGRectMake(0, 350, 100, 40); + view.scrollView.contentSize = CGSizeMake(100, 400); + view.scrollView.contentOffset = CGPointZero; + [view _adjustForMaintainVisibleContentPosition]; + + XCTAssertEqualWithAccuracy(view.scrollView.contentOffset.y, 300, 0.5); +} + +- (void)testMaintainVisibleContentPositionClampsHorizontalOffsetAfterShrink +{ + RCTScrollViewComponentView *view = [[RCTScrollViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)]; + auto props = std::make_shared(); + props->maintainVisibleContentPosition = facebook::react::ScrollViewMaintainVisibleContentPosition{}; + [view updateProps:props oldProps:ScrollViewShadowNode::defaultSharedProps()]; + + RCTViewComponentView *contentView = [[RCTViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 1000, 100)]; + [view mountChildComponentView:contentView index:0]; + UIView *anchor = [[UIView alloc] initWithFrame:CGRectMake(800, 0, 40, 100)]; + anchor.tag = 42; + [contentView addSubview:anchor]; + + view.scrollView.contentSize = CGSizeMake(1000, 100); + view.scrollView.contentOffset = CGPointMake(800, 0); + [view _prepareForMaintainVisibleScrollPosition]; + + // The unclamped target (350) is past the max offset of 400 - 100 = 300. + anchor.frame = CGRectMake(350, 0, 40, 100); + view.scrollView.contentSize = CGSizeMake(400, 100); + view.scrollView.contentOffset = CGPointZero; + [view _adjustForMaintainVisibleContentPosition]; + + XCTAssertEqualWithAccuracy(view.scrollView.contentOffset.x, 300, 0.5); +} + - (void)testAutomaticallyAdjustKeyboardInsetsAcrossRecycling { RCTScrollViewComponentView *view = [[RCTScrollViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)]; diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelper.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelper.kt index 0b9082233f61..ee8c22ab81f6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelper.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelper.kt @@ -41,6 +41,7 @@ internal class MaintainVisibleScrollPositionHelper( var config: Config? = null private var firstVisibleViewRef: WeakReference? = null private var prevFirstVisibleFrame: Rect? = null + private var prevScrollY: Int? = null private var isListening = false private val contentView: ReactViewGroup? @@ -110,7 +111,9 @@ internal class MaintainVisibleScrollPositionHelper( } else { val deltaY = newFrame.top - prevFirstVisibleFrame.top if (deltaY != 0) { - val scrollY = scrollView.scrollY + // ReactScrollView.onLayoutChange clamps scrollY when content shrinks, which runs before + // didMountItems, so apply the delta to the offset from before the mount. + val scrollY = prevScrollY ?: scrollView.scrollY scrollView.scrollToPreservingMomentum(scrollView.scrollX, scrollY + deltaY) this.prevFirstVisibleFrame = newFrame if (config.autoScrollToTopThreshold != null && scrollY <= config.autoScrollToTopThreshold) { @@ -138,6 +141,9 @@ internal class MaintainVisibleScrollPositionHelper( val frame = Rect() child.getHitRect(frame) prevFirstVisibleFrame = frame + if (!horizontal) { + prevScrollY = currentScroll + } break } } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelperTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelperTest.kt new file mode 100644 index 000000000000..5309f3fb8524 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelperTest.kt @@ -0,0 +1,134 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.views.scroll + +import android.app.Activity +import android.content.Context +import android.view.View +import android.view.View.MeasureSpec +import com.facebook.react.bridge.BridgeReactContext +import com.facebook.react.bridge.ReactTestHelper +import com.facebook.react.bridge.UIManager +import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests +import com.facebook.react.uimanager.events.BlackHoleEventDispatcher +import com.facebook.react.uimanager.events.EventDispatcher +import com.facebook.react.uimanager.events.EventDispatcherProvider +import com.facebook.react.views.view.ReactViewGroup +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.mock +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class MaintainVisibleScrollPositionHelperTest { + private lateinit var activity: Activity + private val uiManager: UIManager = mock() + + private lateinit var scrollView: ReactScrollView + private lateinit var content: ReactViewGroup + private lateinit var header: View + private lateinit var anchor: View + private lateinit var footer: View + + @Before + fun setUp() { + ReactNativeFeatureFlagsForTests.setUp() + activity = Robolectric.buildActivity(Activity::class.java).setup().get() + // ReactScrollView dispatches scroll events through its ReactContext. + val reactContext = + TestReactContext(activity).apply { + initializeWithInstance(ReactTestHelper.createMockCatalystInstance()) + } + + scrollView = ReactScrollView(reactContext) + content = ReactViewGroup(activity) + header = View(activity) + anchor = View(activity) + footer = View(activity) + content.addView(header) + content.addView(anchor) + content.addView(footer) + scrollView.addView(content) + activity.setContentView(scrollView) + + scrollView.measure(exactly(VIEWPORT), exactly(VIEWPORT)) + scrollView.layout(0, 0, VIEWPORT, VIEWPORT) + } + + @Test + fun shrinkingContentAboveAnchorKeepsAnchorInPlace() { + layoutRows(headerHeight = 700, footerHeight = 200) + scrollView.scrollTo(0, 750) + val helper = createHelper() + + helper.willMountItems(uiManager) + // Laying out the smaller content clamps scrollY in ReactScrollView.onLayoutChange. + layoutRows(headerHeight = 100, footerHeight = 100) + helper.didMountItems(uiManager) + + assertThat(scrollView.scrollY).isEqualTo(150) + } + + @Test + fun shrinkingContentBelowAnchorStaysWithinScrollRange() { + layoutRows(headerHeight = 700, footerHeight = 200) + scrollView.scrollTo(0, 750) + val helper = createHelper() + + helper.willMountItems(uiManager) + layoutRows(headerHeight = 700, footerHeight = 0) + helper.didMountItems(uiManager) + + assertThat(scrollView.scrollY).isEqualTo(700) + } + + @Test + fun growingContentAboveAnchorKeepsAnchorInPlace() { + layoutRows(headerHeight = 700, footerHeight = 200) + scrollView.scrollTo(0, 750) + val helper = createHelper() + + helper.willMountItems(uiManager) + layoutRows(headerHeight = 900, footerHeight = 200) + helper.didMountItems(uiManager) + + assertThat(scrollView.scrollY).isEqualTo(950) + } + + private fun createHelper(): MaintainVisibleScrollPositionHelper = + MaintainVisibleScrollPositionHelper(scrollView, horizontal = false).apply { + config = MaintainVisibleScrollPositionHelper.Config(0, null) + } + + /** Lays out header, a 100px anchor and footer, as the Fabric updateLayout mount items would. */ + private fun layoutRows(headerHeight: Int, footerHeight: Int) { + val anchorTop = headerHeight + val footerTop = anchorTop + ROW_HEIGHT + val contentHeight = footerTop + footerHeight + header.layout(0, 0, VIEWPORT, headerHeight) + anchor.layout(0, anchorTop, VIEWPORT, footerTop) + footer.layout(0, footerTop, VIEWPORT, contentHeight) + content.measure(exactly(VIEWPORT), exactly(contentHeight)) + content.layout(0, 0, VIEWPORT, contentHeight) + } + + private fun exactly(size: Int) = MeasureSpec.makeMeasureSpec(size, MeasureSpec.EXACTLY) + + private class TestReactContext(base: Context) : + BridgeReactContext(base), EventDispatcherProvider { + override fun getEventDispatcher(): EventDispatcher = BlackHoleEventDispatcher + } + + private companion object { + const val VIEWPORT = 100 + const val ROW_HEIGHT = 100 + } +} diff --git a/packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj b/packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj index 8248d57b2b2a..445a62f03f13 100644 --- a/packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj +++ b/packages/rn-tester/RNTesterPods.xcodeproj/project.pbxproj @@ -18,6 +18,7 @@ 79B29C2E2E607A99007612A5 /* SceneDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 79B29C2D2E607A99007612A5 /* SceneDelegate.mm */; }; 8145AE06241172D900A3F8DA /* LaunchScreen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 8145AE05241172D900A3F8DA /* LaunchScreen.storyboard */; }; 832F45BB2A8A6E1F0097B4E6 /* SwiftTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 832F45BA2A8A6E1F0097B4E6 /* SwiftTest.swift */; }; + 86178A1D4F95F48D0DC933E2 /* RCTScrollViewComponentViewTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 6250ED894CD7F3B5A761FE63 /* RCTScrollViewComponentViewTests.mm */; }; A975CA6C2C05EADF0043F72A /* RCTNetworkTaskTests.m in Sources */ = {isa = PBXBuildFile; fileRef = A975CA6B2C05EADE0043F72A /* RCTNetworkTaskTests.m */; }; C175B6D9ED9336FB66637943 /* libPods-RNTester.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 4C706D402EE4AF9BE838CBA9 /* libPods-RNTester.a */; }; CD10C7A5290BD4EB0033E1ED /* RCTEventEmitterTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CD10C7A4290BD4EB0033E1ED /* RCTEventEmitterTests.m */; }; @@ -93,6 +94,7 @@ 4C706D402EE4AF9BE838CBA9 /* libPods-RNTester.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-RNTester.a"; sourceTree = BUILT_PRODUCTS_DIR; }; 51BC9297B6C3163C14532020 /* Pods-RNTester.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RNTester.release.xcconfig"; path = "Target Support Files/Pods-RNTester/Pods-RNTester.release.xcconfig"; sourceTree = ""; }; 5C60EB1B226440DB0018C04F /* AppDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = AppDelegate.mm; path = RNTester/AppDelegate.mm; sourceTree = ""; }; + 6250ED894CD7F3B5A761FE63 /* RCTScrollViewComponentViewTests.mm */ = {isa = PBXFileReference; includeInIndex = 1; name = RCTScrollViewComponentViewTests.mm; path = "../react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm"; sourceTree = ""; }; 79B29C2C2E607A99007612A5 /* SceneDelegate.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SceneDelegate.h; path = RNTester/SceneDelegate.h; sourceTree = ""; }; 79B29C2D2E607A99007612A5 /* SceneDelegate.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = SceneDelegate.mm; path = RNTester/SceneDelegate.mm; sourceTree = ""; }; 8145AE05241172D900A3F8DA /* LaunchScreen.storyboard */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.storyboard; name = LaunchScreen.storyboard; path = RNTester/LaunchScreen.storyboard; sourceTree = ""; }; @@ -267,6 +269,14 @@ name = Frameworks; sourceTree = ""; }; + 46F51AC598EF6515ADF3A7D9 /* LocalScrollViewTests */ = { + isa = PBXGroup; + children = ( + 6250ED894CD7F3B5A761FE63 /* RCTScrollViewComponentViewTests.mm */, + ); + name = LocalScrollViewTests; + sourceTree = ""; + }; 680759612239798500290469 /* Fabric */ = { isa = PBXGroup; children = ( @@ -284,6 +294,7 @@ 83CBBA001A601CBA00E9B192 /* Products */, 2DE7E7D81FB2A4F3009E225D /* Frameworks */, E23BD6487B06BD71F1A86914 /* Pods */, + 46F51AC598EF6515ADF3A7D9 /* LocalScrollViewTests */, ); indentWidth = 2; sourceTree = ""; @@ -774,6 +785,7 @@ E7DB20EB22B2BAA6005AC45F /* RCTConvert_YGValueTests.m in Sources */, E7DB20E922B2BAA6005AC45F /* RCTComponentPropsTests.m in Sources */, E7DB20D822B2BAA6005AC45F /* RCTJSONTests.m in Sources */, + 86178A1D4F95F48D0DC933E2 /* RCTScrollViewComponentViewTests.mm in Sources */, ); runOnlyForDeploymentPostprocessing = 0; };