diff --git a/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-maintainVisibleContentPosition-itest.js b/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-maintainVisibleContentPosition-itest.js index 642c9a8263d9..d251599ff98b 100644 --- a/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-maintainVisibleContentPosition-itest.js +++ b/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-maintainVisibleContentPosition-itest.js @@ -348,6 +348,134 @@ test('maintainVisibleContentPosition with minIndexForVisible > 0 skips early ite expect(logs2.length).toBeGreaterThan(0); }); +function renderItemWithZIndex(item: Item, height: number = ITEM_HEIGHT) { + return ( + + + + ); +} + +// 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(); + + const items = makeItems(NUM_ITEMS); + const anchorId = 8; + let anchorHeight = ITEM_HEIGHT; + + Fantom.runTask(() => { + root.render( + + {items.map(item => + item.id === anchorId + ? renderItemWithZIndex(item, anchorHeight) + : renderItemWithZIndex(item), + )} + , + ); + }); + + root.takeMountingManagerLogs(); + + Fantom.scrollTo(nodeRef, { + x: 0, + y: ITEM_HEIGHT * anchorId, + }); + + root.takeMountingManagerLogs(); + + anchorHeight = ITEM_HEIGHT * 2; + + Fantom.runTask(() => { + root.render( + + {items.map(item => + item.id === anchorId + ? renderItemWithZIndex(item, anchorHeight) + : renderItemWithZIndex(item), + )} + , + ); + }); + + 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(); + + const items = makeItems(NUM_ITEMS); + + Fantom.runTask(() => { + root.render( + + {items.map(renderItemWithZIndex)} + , + ); + }); + + root.takeMountingManagerLogs(); + + Fantom.scrollTo(nodeRef, { + x: 0, + y: ITEM_HEIGHT * 8, + }); + + root.takeMountingManagerLogs(); + + const itemsAfterPrepend = [...makeItems(3, NUM_ITEMS), ...items]; + + Fantom.runTask(() => { + root.render( + + {itemsAfterPrepend.map(renderItemWithZIndex)} + , + ); + }); + + 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. 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 00653abc9fd0..81fa879f10b1 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm @@ -1069,23 +1069,78 @@ - (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; + int minIndex = props.maintainVisibleContentPosition.value().minIndexForVisible; + NSArray *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 we encounter a zero-sized view as the first subview, assume it's the anchor without + // checking anything else. + // + // This is the easiest way to accommodate LegendList without introducing new APIs. LegendList + // renders a zero-sized, absolutely positioned `ScrollAdjust` view as the first child of the + // scroll content. It sits at a large biased offset and moves by the amount the list wants + // the scroll position adjusted. Because its end position is past every other child, the + // smallest-end scan below would otherwise pick a stationary sibling and skip the adjustment. + // See: + // https://github.com/react/react-native/pull/57959#issuecomment-6341492122 + // https://github.com/LegendApp/legend-list/blob/v3.7.0/src/components/ScrollAdjust.native.tsx#L15-L24 + // https://github.com/LegendApp/legend-list/blob/v3.7.0/src/components/ListComponent.tsx#L246-L257 + if (index == (NSUInteger)minIndex && CGSizeEqualToSize(subview.frame.size, CGSizeZero) && + position > currentScroll) { + firstVisibleView = subview; break; } + + // 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 *)_sortedContentSubviewsWithHorizontal:(BOOL)horizontal +{ + NSArray *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 @@ -1120,7 +1175,6 @@ - (void)_adjustForMaintainVisibleContentPosition std::optional 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) { 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..8988ca6d53d5 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 @@ -84,6 +84,7 @@ internal class MaintainVisibleScrollPositionHelper( return } isListening = false + firstVisibleViewRef = null uIManager.removeUIManagerEventListener(this) } @@ -125,22 +126,78 @@ internal class MaintainVisibleScrollPositionHelper( 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({ 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 - for (i in config.minIndexForVisible until contentView.childCount) { - val child = contentView.getChildAt(i) + 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 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 || i == contentView.childCount - 1) { - firstVisibleViewRef = WeakReference(child) - val frame = Rect() - child.getHitRect(frame) - prevFirstVisibleFrame = frame + // If we encounter a zero-sized view as the first subview, assume it's the anchor without + // checking anything else. + // + // This is the easiest way to accommodate LegendList without introducing new APIs. LegendList + // renders a zero-sized, absolutely positioned `ScrollAdjust` view as the first child of the + // scroll content. It sits at a large biased offset and moves by the amount the list wants + // the scroll position adjusted. Because its end position is past every other child, the + // smallest-end scan below would otherwise pick a stationary sibling and skip the adjustment. + // See: + // https://github.com/react/react-native/pull/57959#issuecomment-6341492122 + // https://github.com/LegendApp/legend-list/blob/v3.7.0/src/components/ScrollAdjust.native.tsx#L15-L24 + // https://github.com/LegendApp/legend-list/blob/v3.7.0/src/components/ListComponent.tsx#L246-L257 + if ( + i == config.minIndexForVisible && + child.width == 0 && + child.height == 0 && + position > currentScroll + ) { + firstVisibleView = child break } + + // 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 == subviews.size - 1) + ) { + firstVisibleView = child + firstVisibleViewPosition = position + } + } + + if (firstVisibleView == null) { + return } + + firstVisibleViewRef = WeakReference(firstVisibleView) + val frame = Rect() + firstVisibleView.getHitRect(frame) + prevFirstVisibleFrame = frame } // UIManagerListener diff --git a/packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js b/packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js index 72580d9c0485..5e68863e1764 100644 --- a/packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js +++ b/packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js @@ -14,7 +14,7 @@ import RNTesterText from '../../components/RNTesterText'; import ScrollViewPressableStickyHeaderExample from './ScrollViewPressableStickyHeaderExample'; import nullthrows from 'nullthrows'; import * as React from 'react'; -import {cloneElement, useCallback, useRef, useState} from 'react'; +import {useCallback, useRef, useState} from 'react'; import { Platform, RefreshControl, @@ -63,114 +63,153 @@ class EnableDisableList extends React.Component<{}, {scrollEnabled: boolean}> { } let AppendingListItemCount = 6; -class AppendingList extends React.Component< - {}, - {items: ReadonlyArray}, -> { - state: {items: ReadonlyArray} = { - items: [...Array(AppendingListItemCount)].map((_, ii) => ( - - )), - }; - render(): React.Node { - return ( - - >(() => + [...Array(AppendingListItemCount)].map((_, ii) => ({ + id: ii, + })), + ); + + const renderItem = (item: ItemInfo, horizontal: boolean) => ( + + ); + + return ( + + + {items.map(item => renderItem(item, false))} + + + {items.map(item => renderItem(item, true))} + + +