From cb9ce13fe4de0c88a2399a5531b375631f96457e Mon Sep 17 00:00:00 2001 From: Aryan Singhal Date: Sun, 27 Sep 2026 10:58:21 +0000 Subject: [PATCH 1/4] Fix visible scroll anchor after content shrinks --- .../ScrollView/RCTScrollViewComponentView.mm | 12 +- .../MaintainVisibleScrollPositionHelper.kt | 8 +- ...MaintainVisibleScrollPositionHelperTest.kt | 116 ++++++++++++++++++ 3 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelperTest.kt 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..2bedbc29154d 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm @@ -110,6 +110,7 @@ @implementation RCTScrollViewComponentView { __weak UIView *_contentView; CGRect _prevFirstVisibleFrame; + CGPoint _prevContentOffset; __weak UIView *_firstVisibleView; NSInteger _firstVisibleViewTag; @@ -708,6 +709,7 @@ - (void)prepareForRecycle self.frame = oldFrame; _contentView = nil; _prevFirstVisibleFrame = CGRectZero; + _prevContentOffset = CGPointZero; _firstVisibleView = nil; _firstVisibleViewTag = 0; _virtualViewContainerState = nil; @@ -1077,6 +1079,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 +1124,9 @@ - (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); + _scrollView.contentOffset = CGPointMake(x + deltaX, _scrollView.contentOffset.y); if (autoscrollThreshold) { // If the offset WAS within the threshold of the start, animate to the start. if (x <= autoscrollThreshold.value()) { @@ -1134,9 +1138,9 @@ - (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); + _scrollView.contentOffset = CGPointMake(_scrollView.contentOffset.x, y + deltaY); 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/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..6cf5056b14eb 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 prevScrollOffset: Int? = null private var isListening = false private val contentView: ReactViewGroup? @@ -91,6 +92,7 @@ internal class MaintainVisibleScrollPositionHelper( val config = config ?: return val firstVisibleViewRef = firstVisibleViewRef ?: return val prevFirstVisibleFrame = prevFirstVisibleFrame ?: return + val prevScrollOffset = prevScrollOffset ?: return val firstVisibleView = firstVisibleViewRef.get() ?: return val scrollView = scrollView ?: return @@ -100,7 +102,7 @@ internal class MaintainVisibleScrollPositionHelper( if (horizontal) { val deltaX = newFrame.left - prevFirstVisibleFrame.left if (deltaX != 0) { - val scrollX = scrollView.scrollX + val scrollX = prevScrollOffset scrollView.scrollToPreservingMomentum(scrollX + deltaX, scrollView.scrollY) this.prevFirstVisibleFrame = newFrame if (config.autoScrollToTopThreshold != null && scrollX <= config.autoScrollToTopThreshold) { @@ -110,7 +112,7 @@ internal class MaintainVisibleScrollPositionHelper( } else { val deltaY = newFrame.top - prevFirstVisibleFrame.top if (deltaY != 0) { - val scrollY = scrollView.scrollY + val scrollY = prevScrollOffset scrollView.scrollToPreservingMomentum(scrollView.scrollX, scrollY + deltaY) this.prevFirstVisibleFrame = newFrame if (config.autoScrollToTopThreshold != null && scrollY <= config.autoScrollToTopThreshold) { @@ -138,6 +140,8 @@ internal class MaintainVisibleScrollPositionHelper( val frame = Rect() child.getHitRect(frame) prevFirstVisibleFrame = frame + // A smaller content size can clamp the live offset before didMountItems. + prevScrollOffset = 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..ccf50bb3210e --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelperTest.kt @@ -0,0 +1,116 @@ +/* + * 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.content.Context +import android.view.View +import android.widget.FrameLayout +import com.facebook.react.bridge.UIManager +import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests +import com.facebook.react.views.scroll.ReactScrollViewHelper.HasScrollEventThrottle +import com.facebook.react.views.scroll.ReactScrollViewHelper.HasSmoothScroll +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.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment + +@RunWith(RobolectricTestRunner::class) +class MaintainVisibleScrollPositionHelperTest { + private lateinit var context: Context + private val uiManager: UIManager = mock() + + @Before + fun setUp() { + ReactNativeFeatureFlagsForTests.setUp() + context = RuntimeEnvironment.getApplication() + } + + @Test + fun shrinkingContentAdjustsFromOffsetBeforeLayoutClamp() { + val scrollView = TestScrollView(context) + val content = ReactViewGroup(context) + val anchor = View(context) + content.addView(anchor) + scrollView.addView(content) + anchor.layout(0, 900, 100, 1000) + scrollView.scrollTo(0, 900) + + val helper = MaintainVisibleScrollPositionHelper(scrollView, horizontal = false) + helper.config = MaintainVisibleScrollPositionHelper.Config(0, null) + helper.willMountItems(uiManager) + + anchor.layout(0, 300, 100, 400) + scrollView.scrollTo(0, 100) // Layout clamped the old offset before didMountItems. + helper.didMountItems(uiManager) + + assertThat(scrollView.requestedY).isEqualTo(300) + } + + @Test + fun shrinkingHorizontalContentAdjustsFromOffsetBeforeLayoutClamp() { + val scrollView = TestScrollView(context) + val content = ReactViewGroup(context) + val anchor = View(context) + content.addView(anchor) + scrollView.addView(content) + anchor.layout(900, 0, 1000, 100) + scrollView.scrollTo(900, 0) + + val helper = MaintainVisibleScrollPositionHelper(scrollView, horizontal = true) + helper.config = MaintainVisibleScrollPositionHelper.Config(0, null) + helper.willMountItems(uiManager) + + anchor.layout(300, 0, 400, 100) + scrollView.scrollTo(100, 0) + helper.didMountItems(uiManager) + + assertThat(scrollView.requestedX).isEqualTo(300) + } + + @Test + fun growingContentStillAdjustsFromOffsetBeforeMount() { + val scrollView = TestScrollView(context) + val content = ReactViewGroup(context) + val anchor = View(context) + content.addView(anchor) + scrollView.addView(content) + anchor.layout(0, 300, 100, 400) + scrollView.scrollTo(0, 300) + + val helper = MaintainVisibleScrollPositionHelper(scrollView, horizontal = false) + helper.config = MaintainVisibleScrollPositionHelper.Config(0, null) + helper.willMountItems(uiManager) + + anchor.layout(0, 900, 100, 1000) + helper.didMountItems(uiManager) + + assertThat(scrollView.requestedY).isEqualTo(900) + } + + private class TestScrollView(context: Context) : + FrameLayout(context), HasScrollEventThrottle, HasSmoothScroll { + override var scrollEventThrottle: Int = 0 + override var lastScrollDispatchTime: Long = 0 + var requestedX: Int? = null + var requestedY: Int? = null + + override fun reactSmoothScrollTo(x: Int, y: Int) { + scrollTo(x, y) + } + + override fun scrollToPreservingMomentum(x: Int, y: Int) { + requestedX = x + requestedY = y + scrollTo(x, y) + } + } +} From 455f58d7a105bdf644499e47fe85abd1027e71fc Mon Sep 17 00:00:00 2001 From: Aryan Date: Sun, 27 Sep 2026 17:28:58 +0530 Subject: [PATCH 2/4] test(scrollview): cover iOS anchor after content shrink --- .../RCTScrollViewComponentViewTests.mm | 52 +++++++++++++++++++ .../RNTesterPods.xcodeproj/project.pbxproj | 12 +++++ 2 files changed, 64 insertions(+) diff --git a/packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm b/packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm index 82b350755668..0475c85391ec 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,56 @@ @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)testAutomaticallyAdjustKeyboardInsetsAcrossRecycling { RCTScrollViewComponentView *view = [[RCTScrollViewComponentView alloc] initWithFrame:CGRectMake(0, 0, 100, 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; }; From 1e7118965e4c936c30bae73167453a5dbaadb553 Mon Sep 17 00:00:00 2001 From: Aryan Date: Mon, 28 Sep 2026 12:12:44 +0530 Subject: [PATCH 3/4] Clamp iOS anchor offset and limit Android offset to vertical iOS: clamp the maintainVisibleContentPosition target to the scrollable range so restoring the pre-mount offset after a shrink cannot overscroll. Android: only the vertical path needs the pre-mount offset, because ReactScrollView.onLayoutChange clamps scrollY before didMountItems. ReactHorizontalScrollView does not clamp, so horizontal reads scrollX live as before. Tests now drive a real ReactScrollView so they go through that clamp. --- .../ScrollView/RCTScrollViewComponentView.mm | 21 ++- .../RCTScrollViewComponentViewTests.mm | 52 +++++++ .../MaintainVisibleScrollPositionHelper.kt | 14 +- ...MaintainVisibleScrollPositionHelperTest.kt | 144 ++++++++++-------- 4 files changed, 160 insertions(+), 71 deletions(-) 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 2bedbc29154d..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, @@ -1126,7 +1141,8 @@ - (void)_adjustForMaintainVisibleContentPosition if (ABS(deltaX) > 0.5) { CGFloat x = _prevContentOffset.x; [self _forceDispatchNextScrollEvent]; - _scrollView.contentOffset = CGPointMake(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()) { @@ -1140,7 +1156,8 @@ - (void)_adjustForMaintainVisibleContentPosition if (ABS(deltaY) > 0.5) { CGFloat y = _prevContentOffset.y; [self _forceDispatchNextScrollEvent]; - _scrollView.contentOffset = CGPointMake(_scrollView.contentOffset.x, 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 0475c85391ec..025be37f11fe 100644 --- a/packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm +++ b/packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm @@ -84,6 +84,58 @@ - (void)testMaintainVisibleContentPositionAfterHorizontalShrink 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 6cf5056b14eb..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,7 +41,7 @@ internal class MaintainVisibleScrollPositionHelper( var config: Config? = null private var firstVisibleViewRef: WeakReference? = null private var prevFirstVisibleFrame: Rect? = null - private var prevScrollOffset: Int? = null + private var prevScrollY: Int? = null private var isListening = false private val contentView: ReactViewGroup? @@ -92,7 +92,6 @@ internal class MaintainVisibleScrollPositionHelper( val config = config ?: return val firstVisibleViewRef = firstVisibleViewRef ?: return val prevFirstVisibleFrame = prevFirstVisibleFrame ?: return - val prevScrollOffset = prevScrollOffset ?: return val firstVisibleView = firstVisibleViewRef.get() ?: return val scrollView = scrollView ?: return @@ -102,7 +101,7 @@ internal class MaintainVisibleScrollPositionHelper( if (horizontal) { val deltaX = newFrame.left - prevFirstVisibleFrame.left if (deltaX != 0) { - val scrollX = prevScrollOffset + val scrollX = scrollView.scrollX scrollView.scrollToPreservingMomentum(scrollX + deltaX, scrollView.scrollY) this.prevFirstVisibleFrame = newFrame if (config.autoScrollToTopThreshold != null && scrollX <= config.autoScrollToTopThreshold) { @@ -112,7 +111,9 @@ internal class MaintainVisibleScrollPositionHelper( } else { val deltaY = newFrame.top - prevFirstVisibleFrame.top if (deltaY != 0) { - val scrollY = prevScrollOffset + // 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) { @@ -140,8 +141,9 @@ internal class MaintainVisibleScrollPositionHelper( val frame = Rect() child.getHitRect(frame) prevFirstVisibleFrame = frame - // A smaller content size can clamp the live offset before didMountItems. - prevScrollOffset = currentScroll + 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 index ccf50bb3210e..5309f3fb8524 100644 --- 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 @@ -7,110 +7,128 @@ package com.facebook.react.views.scroll +import android.app.Activity import android.content.Context import android.view.View -import android.widget.FrameLayout +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.views.scroll.ReactScrollViewHelper.HasScrollEventThrottle -import com.facebook.react.views.scroll.ReactScrollViewHelper.HasSmoothScroll +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 -import org.robolectric.RuntimeEnvironment @RunWith(RobolectricTestRunner::class) class MaintainVisibleScrollPositionHelperTest { - private lateinit var context: Context + 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() - context = RuntimeEnvironment.getApplication() + 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 shrinkingContentAdjustsFromOffsetBeforeLayoutClamp() { - val scrollView = TestScrollView(context) - val content = ReactViewGroup(context) - val anchor = View(context) - content.addView(anchor) - scrollView.addView(content) - anchor.layout(0, 900, 100, 1000) - scrollView.scrollTo(0, 900) + fun shrinkingContentAboveAnchorKeepsAnchorInPlace() { + layoutRows(headerHeight = 700, footerHeight = 200) + scrollView.scrollTo(0, 750) + val helper = createHelper() - val helper = MaintainVisibleScrollPositionHelper(scrollView, horizontal = false) - helper.config = MaintainVisibleScrollPositionHelper.Config(0, null) helper.willMountItems(uiManager) - - anchor.layout(0, 300, 100, 400) - scrollView.scrollTo(0, 100) // Layout clamped the old offset before didMountItems. + // Laying out the smaller content clamps scrollY in ReactScrollView.onLayoutChange. + layoutRows(headerHeight = 100, footerHeight = 100) helper.didMountItems(uiManager) - assertThat(scrollView.requestedY).isEqualTo(300) + assertThat(scrollView.scrollY).isEqualTo(150) } @Test - fun shrinkingHorizontalContentAdjustsFromOffsetBeforeLayoutClamp() { - val scrollView = TestScrollView(context) - val content = ReactViewGroup(context) - val anchor = View(context) - content.addView(anchor) - scrollView.addView(content) - anchor.layout(900, 0, 1000, 100) - scrollView.scrollTo(900, 0) + fun shrinkingContentBelowAnchorStaysWithinScrollRange() { + layoutRows(headerHeight = 700, footerHeight = 200) + scrollView.scrollTo(0, 750) + val helper = createHelper() - val helper = MaintainVisibleScrollPositionHelper(scrollView, horizontal = true) - helper.config = MaintainVisibleScrollPositionHelper.Config(0, null) helper.willMountItems(uiManager) - - anchor.layout(300, 0, 400, 100) - scrollView.scrollTo(100, 0) + layoutRows(headerHeight = 700, footerHeight = 0) helper.didMountItems(uiManager) - assertThat(scrollView.requestedX).isEqualTo(300) + assertThat(scrollView.scrollY).isEqualTo(700) } @Test - fun growingContentStillAdjustsFromOffsetBeforeMount() { - val scrollView = TestScrollView(context) - val content = ReactViewGroup(context) - val anchor = View(context) - content.addView(anchor) - scrollView.addView(content) - anchor.layout(0, 300, 100, 400) - scrollView.scrollTo(0, 300) + fun growingContentAboveAnchorKeepsAnchorInPlace() { + layoutRows(headerHeight = 700, footerHeight = 200) + scrollView.scrollTo(0, 750) + val helper = createHelper() - val helper = MaintainVisibleScrollPositionHelper(scrollView, horizontal = false) - helper.config = MaintainVisibleScrollPositionHelper.Config(0, null) helper.willMountItems(uiManager) - - anchor.layout(0, 900, 100, 1000) + layoutRows(headerHeight = 900, footerHeight = 200) helper.didMountItems(uiManager) - assertThat(scrollView.requestedY).isEqualTo(900) + 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 class TestScrollView(context: Context) : - FrameLayout(context), HasScrollEventThrottle, HasSmoothScroll { - override var scrollEventThrottle: Int = 0 - override var lastScrollDispatchTime: Long = 0 - var requestedX: Int? = null - var requestedY: Int? = null - - override fun reactSmoothScrollTo(x: Int, y: Int) { - scrollTo(x, y) - } - - override fun scrollToPreservingMomentum(x: Int, y: Int) { - requestedX = x - requestedY = y - scrollTo(x, y) - } + private companion object { + const val VIEWPORT = 100 + const val ROW_HEIGHT = 100 } } From 7daddd04a46ca1b215d37ef46de94ef2e1af4cbb Mon Sep 17 00:00:00 2001 From: Aryan Date: Tue, 29 Sep 2026 10:48:17 +0530 Subject: [PATCH 4/4] Record Android anchor offset at layout clamp, not willMountItems Fabric runs queued view commands after willMountItems, so recording scrollY there discarded a scrollTo dispatched alongside the shrink (e.g. collapse then scroll to top) and restored the old anchor instead. ReactScrollView now hands the helper scrollY right before onLayoutChange clamps it, which is after view commands have run. The helper uses it once in didMountItems and clears it there and in willMountItems. Tests enable MVCP through setMaintainVisibleContentPosition and cover the scrollTo-during-shrink case. --- .../MaintainVisibleScrollPositionHelper.kt | 22 ++++++---- .../react/views/scroll/ReactScrollView.kt | 1 + ...MaintainVisibleScrollPositionHelperTest.kt | 41 ++++++++++++++++--- 3 files changed, 51 insertions(+), 13 deletions(-) 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 ee8c22ab81f6..1aada3a657e8 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,7 +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 scrollYBeforeLayoutClamp: Int? = null private var isListening = false private val contentView: ReactViewGroup? @@ -88,6 +88,17 @@ internal class MaintainVisibleScrollPositionHelper( uIManager.removeUIManagerEventListener(this) } + /** + * Called by ReactScrollView just before it clamps scrollY because the content got smaller. This + * happens during mounting, after any queued view commands have run, so it is the offset the + * anchor delta should be applied to in didMountItems. + */ + fun onWillClampScrollY(scrollY: Int) { + if (scrollYBeforeLayoutClamp == null) { + scrollYBeforeLayoutClamp = scrollY + } + } + private fun updateScrollPositionInternal() { val config = config ?: return val firstVisibleViewRef = firstVisibleViewRef ?: return @@ -111,9 +122,7 @@ internal class MaintainVisibleScrollPositionHelper( } else { val deltaY = newFrame.top - prevFirstVisibleFrame.top if (deltaY != 0) { - // 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 + val scrollY = scrollYBeforeLayoutClamp ?: scrollView.scrollY scrollView.scrollToPreservingMomentum(scrollView.scrollX, scrollY + deltaY) this.prevFirstVisibleFrame = newFrame if (config.autoScrollToTopThreshold != null && scrollY <= config.autoScrollToTopThreshold) { @@ -141,9 +150,6 @@ internal class MaintainVisibleScrollPositionHelper( val frame = Rect() child.getHitRect(frame) prevFirstVisibleFrame = frame - if (!horizontal) { - prevScrollY = currentScroll - } break } } @@ -155,11 +161,13 @@ internal class MaintainVisibleScrollPositionHelper( } override fun willMountItems(uiManager: UIManager) { + scrollYBeforeLayoutClamp = null computeTargetView() } override fun didMountItems(uiManager: UIManager) { updateScrollPositionInternal() + scrollYBeforeLayoutClamp = null } override fun didDispatchMountItems(uiManager: UIManager) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.kt index b553f6af997d..f1d0f939dd4a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.kt @@ -1207,6 +1207,7 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) : val currentScrollY = scrollY val maxScrollY = getMaxScrollY() if (currentScrollY > maxScrollY) { + maintainVisibleContentPositionHelper?.onWillClampScrollY(currentScrollY) scrollTo(scrollX, maxScrollY) } } 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 index 5309f3fb8524..8919fd1563e7 100644 --- 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 @@ -14,6 +14,8 @@ 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.bridge.UIManagerListener +import com.facebook.react.common.annotations.UnstableReactNativeAPI import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests import com.facebook.react.uimanager.events.BlackHoleEventDispatcher import com.facebook.react.uimanager.events.EventDispatcher @@ -23,10 +25,13 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.mock +import org.mockito.kotlin.verify import org.robolectric.Robolectric import org.robolectric.RobolectricTestRunner +@OptIn(UnstableReactNativeAPI::class) @RunWith(RobolectricTestRunner::class) class MaintainVisibleScrollPositionHelperTest { private lateinit var activity: Activity @@ -44,7 +49,7 @@ class MaintainVisibleScrollPositionHelperTest { activity = Robolectric.buildActivity(Activity::class.java).setup().get() // ReactScrollView dispatches scroll events through its ReactContext. val reactContext = - TestReactContext(activity).apply { + TestReactContext(activity, uiManager).apply { initializeWithInstance(ReactTestHelper.createMockCatalystInstance()) } @@ -90,6 +95,21 @@ class MaintainVisibleScrollPositionHelperTest { assertThat(scrollView.scrollY).isEqualTo(700) } + @Test + fun scrollToTopDispatchedWithShrinkIsNotOverwritten() { + layoutRows(headerHeight = 700, footerHeight = 200) + scrollView.scrollTo(0, 750) + val helper = createHelper() + + helper.willMountItems(uiManager) + // Fabric runs queued view commands after willMountItems and before layout mount items. + scrollView.scrollTo(0, 0) + layoutRows(headerHeight = 100, footerHeight = 100) + helper.didMountItems(uiManager) + + assertThat(scrollView.scrollY).isEqualTo(0) + } + @Test fun growingContentAboveAnchorKeepsAnchorInPlace() { layoutRows(headerHeight = 700, footerHeight = 200) @@ -103,10 +123,15 @@ class MaintainVisibleScrollPositionHelperTest { assertThat(scrollView.scrollY).isEqualTo(950) } - private fun createHelper(): MaintainVisibleScrollPositionHelper = - MaintainVisibleScrollPositionHelper(scrollView, horizontal = false).apply { - config = MaintainVisibleScrollPositionHelper.Config(0, null) - } + /** Enables MVCP on the scroll view and returns the helper it registered with the UIManager. */ + private fun createHelper(): MaintainVisibleScrollPositionHelper<*> { + scrollView.setMaintainVisibleContentPosition( + MaintainVisibleScrollPositionHelper.Config(0, null), + ) + val listener = argumentCaptor() + verify(uiManager).addUIManagerEventListener(listener.capture()) + return listener.firstValue as MaintainVisibleScrollPositionHelper<*> + } /** Lays out header, a 100px anchor and footer, as the Fabric updateLayout mount items would. */ private fun layoutRows(headerHeight: Int, footerHeight: Int) { @@ -122,9 +147,13 @@ class MaintainVisibleScrollPositionHelperTest { private fun exactly(size: Int) = MeasureSpec.makeMeasureSpec(size, MeasureSpec.EXACTLY) - private class TestReactContext(base: Context) : + private class TestReactContext(base: Context, private val uiManager: UIManager) : BridgeReactContext(base), EventDispatcherProvider { override fun getEventDispatcher(): EventDispatcher = BlackHoleEventDispatcher + + override fun hasActiveReactInstance(): Boolean = true + + override fun getFabricUIManager(): UIManager = uiManager } private companion object {