Fix behavior on iOS too; add better handling for minIndex

Author: hsource

packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-maintainVisibleContentPosition-itest.js

@@ -348,6 +348,132 @@ test('maintainVisibleContentPosition with minIndexForVisible > 0 skips early ite
   expect(logs2.length).toBeGreaterThan(0);
 });
 
+function renderItemWithZIndex(item: Item, height: number = ITEM_HEIGHT) {
+  return (
+    <View
+      key={item.key}
+      nativeID={`item_${item.id}`}
+      style={{height, width: 100, zIndex: -item.id}}>
+      <View
+        nativeID={`inner_${item.id}`}
+        style={{
+          height: height - 2,
+          width: 100 - 2,
+          backgroundColor: '#4CAF50',
+        }}
+      />
+    </View>
+  );
+}
+
+// Trigger: z-index reorders native subviews so hierarchy index != layout order.
+// Expected: MVCP selects the topmost visible anchor by layout position, not hierarchy order.
+test('maintainVisibleContentPosition with zIndex preserves anchor on height change', () => {
+  const root = Fantom.createRoot({
+    viewportWidth: 100,
+    viewportHeight: VIEWPORT_HEIGHT,
+  });
+  const nodeRef = createRef<HostInstance>();
+
+  const items = makeItems(NUM_ITEMS);
+  const anchorId = 8;
+  let anchorHeight = ITEM_HEIGHT;q
+
+  Fantom.runTask(() => {
+    root.render(
+      <ScrollView
+        ref={nodeRef}
+        style={{height: VIEWPORT_HEIGHT, width: 100}}
+        maintainVisibleContentPosition={{minIndexForVisible: 0}}>
+        {items.map(item =>
+          item.id === anchorId
+            ? renderItemWithZIndex(item, anchorHeight)
+            : renderItemWithZIndex(item),
+        )}
+      </ScrollView>,
+    );
+  });
+
+  root.takeMountingManagerLogs();
+
+  Fantom.scrollTo(nodeRef, {
+    x: 0,
+    y: ITEM_HEIGHT * anchorId,
+  });
+
+  root.takeMountingManagerLogs();
+
+  anchorHeight = ITEM_HEIGHT * 2;
+
+  Fantom.runTask(() => {
+    root.render(
+      <ScrollView
+        ref={nodeRef}
+        style={{height: VIEWPORT_HEIGHT, width: 100}}
+        maintainVisibleContentPosition={{minIndexForVisible: 0}}>
+        {items.map(item =>
+          item.id === anchorId
+            ? renderItemWithZIndex(item, anchorHeight)
+            : renderItemWithZIndex(item),
+        )}
+      </ScrollView>,
+    );
+  });
+
+  const heightChangeLogs = root.takeMountingManagerLogs();
+  expect(heightChangeLogs.length).toBeGreaterThan(0);
+  expect(heightChangeLogs.some(log => log.includes(`item_${anchorId}`))).toBe(true);
+});
+
+// Trigger: z-index reorders native subviews when minIndexForVisible > 0.
+// Expected: minIndex skip uses layout order, not native hierarchy order.
+test('maintainVisibleContentPosition with zIndex and minIndexForVisible > 0 skips early items', () => {
+  const root = Fantom.createRoot({
+    viewportWidth: 100,
+    viewportHeight: VIEWPORT_HEIGHT,
+  });
+  const nodeRef = createRef<HostInstance>();
+
+  const items = makeItems(NUM_ITEMS);
+
+  Fantom.runTask(() => {
+    root.render(
+      <ScrollView
+        ref={nodeRef}
+        style={{height: VIEWPORT_HEIGHT, width: 100}}
+        maintainVisibleContentPosition={{minIndexForVisible: 5}}>
+        {items.map(renderItemWithZIndex)}
+      </ScrollView>,
+    );
+  });
+
+  root.takeMountingManagerLogs();
+
+  Fantom.scrollTo(nodeRef, {
+    x: 0,
+    y: ITEM_HEIGHT * 8,
+  });
+
+  root.takeMountingManagerLogs();
+
+  const itemsAfterPrepend = [...makeItems(3, NUM_ITEMS), ...items];
+
+  Fantom.runTask(() => {
+    root.render(
+      <ScrollView
+        ref={nodeRef}
+        style={{height: VIEWPORT_HEIGHT, width: 100}}
+        maintainVisibleContentPosition={{minIndexForVisible: 5}}>
+        {itemsAfterPrepend.map(renderItemWithZIndex)}
+      </ScrollView>,
+    );
+  });
+
+  const prependLogs = root.takeMountingManagerLogs();
+  expect(prependLogs.length).toBeGreaterThan(0);
+  expect(prependLogs.some(log => log.includes('item_8'))).toBe(true);
+});
+
 // Trigger: User actively dragging (touch-scrolling) when data change triggers MVCP.
 // Expected: MVCP correction may compete with user's scroll. Scroll skip guard on `patch/add-scrolling-guard`
 // branch would skip correction during user dragging, but this is NOT merged.

packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm

@@ -1064,23 +1064,60 @@ - (void)_prepareForMaintainVisibleScrollPosition
   }
 
   BOOL horizontal = _scrollView.contentSize.width > self.frame.size.width;
-  int minIdx = props.maintainVisibleContentPosition.value().minIndexForVisible;
-  for (NSUInteger ii = minIdx; ii < _contentView.subviews.count; ++ii) {
-    // Find the first view that is partially or fully visible.
-    UIView *subview = _contentView.subviews[ii];
-    BOOL hasNewView = NO;
-    if (horizontal) {
-      hasNewView = subview.frame.origin.x + subview.frame.size.width > _scrollView.contentOffset.x;
-    } else {
-      hasNewView = subview.frame.origin.y + subview.frame.size.height > _scrollView.contentOffset.y;
-    }
-    if (hasNewView || ii == _contentView.subviews.count - 1) {
-      _prevFirstVisibleFrame = subview.frame;
-      _firstVisibleView = subview;
-      _firstVisibleViewTag = subview.tag;
-      break;
+  int minIndex = props.maintainVisibleContentPosition.value().minIndexForVisible;
+  NSArray<UIView *> *subviews =
+      minIndex > 0 ? [self _sortedContentSubviewsWithHorizontal:horizontal] : _contentView.subviews;
+  if ((NSUInteger)minIndex >= subviews.count) {
+    return;
+  }
+
+  CGFloat currentScroll = horizontal ? _scrollView.contentOffset.x : _scrollView.contentOffset.y;
+  UIView *firstVisibleView = nil;
+  // We cannot assume that the views will be in position order because of things like z-index
+  // which will change the order of views in their parent. This means we need to iterate through
+  // the full children array and find the view with the smallest position that is bigger than
+  // the scroll position.
+  CGFloat firstVisibleViewPosition = CGFLOAT_MAX;
+  for (NSUInteger index = minIndex; index < subviews.count; ++index) {
+    UIView *subview = subviews[index];
+
+    // Compute the position of the end of the child
+    CGFloat position = horizontal ? subview.frame.origin.x + subview.frame.size.width
+                                  : subview.frame.origin.y + subview.frame.size.height;
+
+    // If the child is partially visible or this is the last child, select it as the anchor.
+    if ((position > currentScroll && position < firstVisibleViewPosition) ||
+        (firstVisibleView == nil && index == subviews.count - 1)) {
+      firstVisibleView = subview;
+      firstVisibleViewPosition = position;
     }
   }
+
+  if (firstVisibleView == nil) {
+    return;
+  }
+
+  _prevFirstVisibleFrame = firstVisibleView.frame;
+  _firstVisibleView = firstVisibleView;
+  _firstVisibleViewTag = firstVisibleView.tag;
+}
+
+/// Returns scroll content subviews sorted by layout position along the scroll axis.
+///
+/// Call this only when minIndexForVisible > 0. In those cases, we need to skip in-order items, and
+/// sorting allows us to do that.
+- (NSArray<UIView *> *)_sortedContentSubviewsWithHorizontal:(BOOL)horizontal
+{
+  NSArray<UIView *> *subviews = _contentView.subviews;
+  return [subviews sortedArrayUsingComparator:^NSComparisonResult(UIView *a, UIView *b) {
+    CGFloat offsetA = horizontal ? CGRectGetMinX(a.frame) : CGRectGetMinY(a.frame);
+    CGFloat offsetB = horizontal ? CGRectGetMinX(b.frame) : CGRectGetMinY(b.frame);
+    if (offsetA != offsetB) {
+      return offsetA < offsetB ? NSOrderedAscending : NSOrderedDescending;
+    }
+    // Stable ordering for ties.
+    return [@([subviews indexOfObject:a]) compare:@([subviews indexOfObject:b])];
+  }];
 }
 
 - (void)_adjustForMaintainVisibleContentPosition
@@ -1115,7 +1152,6 @@ - (void)_adjustForMaintainVisibleContentPosition
 
   std::optional<int> autoscrollThreshold = props.maintainVisibleContentPosition.value().autoscrollToTopThreshold;
   BOOL horizontal = _scrollView.contentSize.width > self.frame.size.width;
-  // TODO: detect and handle/ignore re-ordering
   if (horizontal) {
     CGFloat deltaX = _firstVisibleView.frame.origin.x - _prevFirstVisibleFrame.origin.x;
     if (ABS(deltaX) > 0.5) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelper.kt

@@ -125,22 +125,40 @@ internal class MaintainVisibleScrollPositionHelper<ScrollViewT>(
     val scrollView = scrollView ?: return
     val contentView = contentView ?: return
 
+    val children = List(contentView.childCount) { contentView.getChildAt(it) }
+    val subviews =
+        if (config.minIndexForVisible > 0) {
+          // We only need to sort if minIndexForVisible is non-zero. Normally, we just do a full
+          // linear scan and find the earliest item.
+          //
+          // If minIndexForVisible is set though, we have to exclude the first `minIndexForVisible`
+          // items, which requires sorting to do.
+          children.sortedWith(
+              compareBy<View>({ if (horizontal) it.x else it.y }, { contentView.indexOfChild(it) }))
+        } else {
+          children
+        }
+
+    if (config.minIndexForVisible >= subviews.size) {
+      return
+    }
+
     val currentScroll = if (horizontal) scrollView.scrollX else scrollView.scrollY
     var firstVisibleView: View? = null
     // We cannot assume that the views will be in position order because of things like z-index
     // which will change the order of views in their parent. This means we need to iterate through
     // the full children array and find the view with the smallest position that is bigger than
     // the scroll position.
     var firstVisibleViewPosition = Float.MAX_VALUE
-    for (i in config.minIndexForVisible until contentView.childCount) {
-      val child = contentView.getChildAt(i)
+    for (i in config.minIndexForVisible until subviews.size) {
+      val child = subviews[i]
 
       // Compute the position of the end of the child
       val position = if (horizontal) child.x + child.width else child.y + child.height
 
       // If the child is partially visible or this is the last child, select it as the anchor.
       if ((position > currentScroll && position < firstVisibleViewPosition) ||
-          (firstVisibleView == null && i == contentView.childCount - 1)) {
+          (firstVisibleView == null && i == subviews.size - 1)) {
         firstVisibleView = child
         firstVisibleViewPosition = position
       }