Fix maintainVisibleContentPosition when ScrollView content shrinks

[aryan1306]

Summary:

Fixes #58578.

When ScrollView content shrinks, layout can clamp the current scroll offset before maintainVisibleContentPosition adjusts the visible anchor. Applying the anchor delta to that clamped offset can move the list to the top.

Capture the scroll offset before mounting and use it when applying the anchor adjustment in Android and iOS Fabric. Add regression tests for vertical and horizontal shrinking content.

Changelog:

[GENERAL] [FIXED] - Preserve the visible ScrollView anchor when content shrinks.

Test Plan:

• Reproduced the jump with the linked Expo app on an Android emulator: the visible child moved from anchor 49 to child 0 after collapse.
• Ran ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests com.facebook.react.views.scroll.MaintainVisibleScrollPositionHelperTest --no-daemon: all three tests passed.
• Ran the RNTesterUnitTests scheme on an iPhone 17 Pro simulator: all three tests in RCTScrollViewComponentViewTests passed, including the vertical and horizontal shrink regressions.
• Confirmed the vertical iOS regression test fails against the pre-fix code: expected offset 300, received -500.

[meta-cla]

Hi @aryan1306!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[meta-cla]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[Abbondanzo]

With this, it's possible to have the content offset greater than the height of the scroll view and you end up overscrolling. The target offset should be clamped to the bounds of the scroll view. You can use a helper like this to precompute the target point:

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));
}

and then the correction becomes

CGPoint targetOffset = CGPointMake(
    x + deltaX,
    _scrollView.contentOffset.y);

[self _forceDispatchNextScrollEvent];
_scrollView.contentOffset =
    RCTClampMaintainVisibleContentOffset(_scrollView, targetOffset);

(same for vertical as well)

[aryan1306]

I added RCTClampMaintainVisibleContentOffset as you suggested and use it for both the horizontal and vertical adjustments. I also added two tests where the target lands past the end of the content. Both fail without the clamp and pass with it.

[Abbondanzo]

To avoid multiple offset states, could you just compute this on the fly in onLayoutChange and only call scrollTo when the scroll position is greater than the allowable offset?

[aryan1306]

Thanks for the suggestion! I dug into this, and ReactScrollView.onLayoutChange already does exactly that check. It's actually what causes this bug.

When the content shrinks, things happen in this order:

1. willMountItems: we record the anchor.
2. The content view gets its new layout, and onLayoutChange clamps scrollY to the new max (750 → 200 in the test).
3. didMountItems: we add the anchor's delta to that already-clamped value (200 − 600), and the list jumps to the top.

So we need the offset from before step 2. I also tried it the other way: no stored offset, and skip the clamp while MVCP is active. That fixed this case, but when rows below the anchor shrink, nothing pulls the view back and it overscrolls.

To keep the extra state small, I've limited it to the vertical path. ReactHorizontalScrollView.onLayoutChange doesn't clamp, so horizontal never needed it. I also rewrote the tests to use a real ReactScrollView, so they go through the actual clamp.

[Abbondanzo]

There's a subtle bug in that this will end up discarding queued scrollTo view commands, overwriting them with the previous anchor point after items finish mounting. You could imagine a use case where a user wants to scroll to the top of the list if the list is collapsed, using the linked issue as an example:

setCollapsed(true);
scrollViewRef.current?.scrollTo({y: 0, animated: false});

Fabric executes view commands between willMount and didMount too, and so you could express this in unit test form:

@Test
fun scrollToTopDispatchedWithShrinkIsNotOverwritten() {
  layoutRows(headerHeight = 700, footerHeight = 200)
  scrollView.scrollTo(0, 750)
  val helper = createHelper()
  // Fabric calls this first
  helper.willMountItems(uiManager)
  // Then executes view comamnds
  scrollView.scrollTo(0, 0)
  layoutRows(headerHeight = 100, footerHeight = 100)
  // Finally, fabric calls didMount
  helper.didMountItems(uiManager)
  // This PR would yield 150
  assertThat(scrollView.scrollY).isEqualTo(0)
}

To work around this from onLayoutChange, you could record the scroll offset before layout finishes clamping, and reset it in didMountItems after using it in the updateScrollPositionInternal call