diff --git a/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js b/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js index ed87348b02b2..4350293a7540 100644 --- a/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js +++ b/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js @@ -2553,6 +2553,7 @@ describe('horizontal ScrollView in RTL script', () => { contentOffset={{x: 100, y: 0}}> + , ); }); @@ -2567,6 +2568,35 @@ describe('horizontal ScrollView in RTL script', () => { 'Insert {type: "ScrollView", parentNativeID: (root), index: 0, nativeID: (N/A)}', ]); }); + + it('measures scroll position from the left edge', () => { + const root = Fantom.createRoot({viewportWidth: 100, viewportHeight: 100}); + const nodeRef = createRef(); + + Fantom.runTask(() => { + root.render( + + + + , + ); + }); + + root.takeMountingManagerLogs(); + + Fantom.scrollTo(nodeRef, {x: 0, y: 0}); + + expect(root.takeMountingManagerLogs()).toEqual([ + 'Update {type: "ScrollView", nativeID: (N/A)}', + 'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "item1"}', + 'Delete {type: "View", nativeID: "item1"}', + 'Create {type: "View", nativeID: "item2"}', + 'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "item2"}', + ]); + }); }); describe('Views with no layout', () => { 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..4a59a3b5f432 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm @@ -106,6 +106,11 @@ @implementation RCTScrollViewComponentView { CGPoint _contentOffsetWhenClipped; + // Set in `finalizeUpdates:`, when the content size and direction are known. + std::optional _pendingContentOffset; + BOOL _isPendingContentOffsetFromState; + BOOL _contentOffsetNeedsStateUpdate; + __weak UIView *_contentView; CGRect _prevFirstVisibleFrame; @@ -305,7 +310,15 @@ + (ComponentDescriptorProvider)componentDescriptorProvider - (void)updateLayoutMetrics:(const LayoutMetrics &)layoutMetrics oldLayoutMetrics:(const LayoutMetrics &)oldLayoutMetrics { + if (layoutMetrics.layoutDirection != oldLayoutMetrics.layoutDirection && !_pendingContentOffset) { + _pendingContentOffset = [self _mirrorContentOffsetIfNeeded:_scrollView.contentOffset forState:YES]; + _isPendingContentOffsetFromState = YES; + } [super updateLayoutMetrics:layoutMetrics oldLayoutMetrics:oldLayoutMetrics]; + if (layoutMetrics.layoutDirection == LayoutDirection::RightToLeft && + layoutMetrics.frame.size.width != oldLayoutMetrics.frame.size.width) { + _contentOffsetNeedsStateUpdate = YES; + } if (layoutMetrics.layoutDirection != oldLayoutMetrics.layoutDirection) { CGAffineTransform transform = (layoutMetrics.layoutDirection == LayoutDirection::LeftToRight) ? CGAffineTransformIdentity @@ -404,8 +417,11 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared & if (oldScrollViewProps.contentInset != newScrollViewProps.contentInset && !newScrollViewProps.centerContent) { _scrollView.contentInset = RCTUIEdgeInsetsFromEdgeInsets(newScrollViewProps.contentInset); } - if (oldScrollViewProps.contentOffset != newScrollViewProps.contentOffset) { - _scrollView.contentOffset = RCTCGPointFromPoint(newScrollViewProps.contentOffset); + // Skip a zero offset on mount, so RTL content starts on the right. + if (oldProps ? oldScrollViewProps.contentOffset != newScrollViewProps.contentOffset + : newScrollViewProps.contentOffset != facebook::react::Point{}) { + _pendingContentOffset = RCTCGPointFromPoint(newScrollViewProps.contentOffset); + _isPendingContentOffsetFromState = NO; } // RCTEnhancedScrollView specific props @@ -465,29 +481,11 @@ - (void)updateState:(const State::Shared &)state oldState:(const State::Shared & _state = std::static_pointer_cast(state); auto &data = _state->getData(); - auto contentOffset = RCTCGPointFromPoint(data.contentOffset); - if (!oldState && !CGPointEqualToPoint(contentOffset, CGPointZero)) { - /* - * When is suspended, it is removed from view hierarchy and its offset is stored in - * state. We want to restore this offset from the state but it must be snapped to be within UIScrollView's - * content to remove any overscroll. - * - * This can happen, for example, with pull to refresh. The UIScrollView will be overscrolled into negative offset. - * If the offset is not adjusted to be within the content area, it leads to a gap and UIScrollView does not adjust - * its offset until user scrolls. - */ - - // Adjusting overscroll on the top. - contentOffset.y = fmax(contentOffset.y, -_scrollView.contentInset.top); - - // Adjusting overscroll on the left. - contentOffset.x = fmax(contentOffset.x, -_scrollView.contentInset.left); - - // TODO: T190695447 - Protect against over scroll on the bottom and right as well. - // This is not easily done because we need to flip the order of method calls for - // ShadowViewMutation::Insert. updateLayout must come before updateState. - - _scrollView.contentOffset = contentOffset; + if (!oldState) { + // When is suspended, it is removed from view hierarchy and its offset is stored in state. + // `finalizeUpdates:` restores it. + _pendingContentOffset = RCTCGPointFromPoint(data.contentOffset); + _isPendingContentOffsetFromState = YES; } CGSize contentSize = RCTCGSizeFromSize(data.getContentSize()); @@ -496,6 +494,9 @@ - (void)updateState:(const State::Shared &)state oldState:(const State::Shared & return; } + if (_layoutMetrics.layoutDirection == LayoutDirection::RightToLeft && contentSize.width != _contentSize.width) { + _contentOffsetNeedsStateUpdate = YES; + } _contentSize = contentSize; _containerView.frame = CGRect{RCTCGPointFromPoint(data.contentBoundingRect.origin), contentSize}; @@ -504,6 +505,33 @@ - (void)updateState:(const State::Shared &)state oldState:(const State::Shared & }]; } +- (void)finalizeUpdates:(RNComponentViewUpdateMask)updateMask +{ + [super finalizeUpdates:updateMask]; + + if (_pendingContentOffset) { + CGPoint contentOffset = [self _mirrorContentOffsetIfNeeded:*_pendingContentOffset + forState:_isPendingContentOffsetFromState]; + if (_isPendingContentOffsetFromState) { + // Snap a restored offset into the content to remove any overscroll, for example after pull to refresh. + // TODO: T190695447 - Protect against over scroll on the bottom and right as well. + contentOffset.x = fmax(contentOffset.x, -_scrollView.contentInset.left); + contentOffset.y = fmax(contentOffset.y, -_scrollView.contentInset.top); + } + _pendingContentOffset.reset(); + if (!CGPointEqualToPoint(_scrollView.contentOffset, contentOffset)) { + // `scrollViewDidScroll:` updates the state. + _scrollView.contentOffset = contentOffset; + _contentOffsetNeedsStateUpdate = NO; + } + } + + if (_contentOffsetNeedsStateUpdate) { + _contentOffsetNeedsStateUpdate = NO; + [self _updateStateWithContentOffset]; + } +} + - (UIView *)betterHitTest:(CGPoint)point withEvent:(UIEvent *)event { // This is the same algorithm as in the RCTViewComponentView with the exception of @@ -590,21 +618,31 @@ - (BOOL)_shouldDisableScrollInteraction return NO; } +/* + * In RTL, UIKit offsets start on the right, but JS and state offsets start on the left. + * When the content is narrower than the view, the JS offset is 0, like on Android. + * The state offset is negative, because the shadow tree puts the content on the left. + */ +- (CGPoint)_mirrorContentOffsetIfNeeded:(CGPoint)offset forState:(BOOL)forState +{ + if (_layoutMetrics.layoutDirection == LayoutDirection::RightToLeft) { + CGFloat maxOffset = _scrollView.contentSize.width - _scrollView.bounds.size.width; + offset.x = (forState ? maxOffset : MAX(maxOffset, 0)) - offset.x; + } + return offset; +} + - (ScrollViewEventEmitter::Metrics)_scrollViewMetrics { auto metrics = ScrollViewEventEmitter::Metrics{}; metrics.contentSize = RCTSizeFromCGSize(_scrollView.contentSize); - metrics.contentOffset = RCTPointFromCGPoint(_scrollView.contentOffset); + metrics.contentOffset = RCTPointFromCGPoint([self _mirrorContentOffsetIfNeeded:_scrollView.contentOffset + forState:NO]); metrics.contentInset = RCTEdgeInsetsFromUIEdgeInsets(_scrollView.contentInset); metrics.containerSize = RCTSizeFromCGSize(_scrollView.bounds.size); metrics.zoomScale = _scrollView.zoomScale; metrics.timestamp = CACurrentMediaTime(); metrics.responderIgnoreScroll = !_isUserTriggeredScrolling; - - if (_layoutMetrics.layoutDirection == LayoutDirection::RightToLeft) { - metrics.contentOffset.x = metrics.contentSize.width - metrics.containerSize.width - metrics.contentOffset.x; - } - return metrics; } @@ -662,7 +700,7 @@ - (void)_updateStateWithContentOffset _avoidAdjustmentForMaintainVisibleContentPosition = enableImmediateUpdateModeForContentOffsetChanges; - auto contentOffset = RCTPointFromCGPoint(_scrollView.contentOffset); + auto contentOffset = RCTPointFromCGPoint([self _mirrorContentOffsetIfNeeded:_scrollView.contentOffset forState:YES]); BOOL isAccessibilityAPIUsed = _isAccessibilityAPIUsed; _state->updateState( [contentOffset, isAccessibilityAPIUsed]( @@ -695,7 +733,9 @@ - (void)prepareForRecycle // in RTL the mirroring transform makes UIKit report zoomScale == -1, and changing // it to 1 makes UIKit move contentOffset. _scrollView.zoomScale = 1.0; - _scrollView.contentOffset = RCTCGPointFromPoint(props.contentOffset); + _scrollView.contentOffset = CGPointZero; + _pendingContentOffset.reset(); + _contentOffsetNeedsStateUpdate = NO; // Invalidate cached content size so that updateState: recalculates the // container frame after zoomScale reset (which may have mutated it in RTL). _contentSize = CGSizeZero; @@ -1024,10 +1064,7 @@ - (void)scrollToOffset:(CGPoint)offset - (void)scrollToOffset:(CGPoint)offset animated:(BOOL)animated { - if (_layoutMetrics.layoutDirection == LayoutDirection::RightToLeft) { - // Adjusting offset.x in right to left layout direction. - offset.x = self.contentSize.width - _scrollView.frame.size.width - offset.x; - } + offset = [self _mirrorContentOffsetIfNeeded:offset forState:NO]; if (CGPointEqualToPoint(_scrollView.contentOffset, offset)) { return; diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp index 623f32874854..b6defb668391 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp @@ -27,6 +27,15 @@ void ScrollViewShadowNode::updateStateIfNeeded() { if (state.contentBoundingRect != contentBoundingRect) { state.contentBoundingRect = contentBoundingRect; +#ifndef ANDROID + // RTL scroll views start on the right until the native view reports. + if (getState()->getRevision() == State::initialRevisionValue && + getLayoutMetrics().layoutDirection == LayoutDirection::RightToLeft && + state.contentOffset.x == 0) { + state.contentOffset.x = + contentBoundingRect.size.width - getLayoutMetrics().frame.size.width; + } +#endif setStateData(std::move(state)); } } diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/internal/CullingContext.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/internal/CullingContext.cpp index 609fc268e122..362fff413707 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/internal/CullingContext.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/internal/CullingContext.cpp @@ -26,7 +26,6 @@ CullingContext CullingContext::adjustCullingContextIfNeeded( if (scrollViewShadowNode->getConcreteProps().yogaStyle.overflow() != yoga::Overflow::Visible && !scrollViewShadowNode->getStateData().disableViewCulling) { - auto layoutMetrics = scrollViewShadowNode->getLayoutMetrics(); cullingContext.frame.origin = -scrollViewShadowNode->getContentOriginOffset( /* includeTransform */ true); @@ -48,16 +47,17 @@ CullingContext CullingContext::adjustCullingContextIfNeeded( cullingContext.transform = Transform::Identity(); +#ifdef ANDROID + auto layoutMetrics = scrollViewShadowNode->getLayoutMetrics(); if (layoutMetrics.layoutDirection == LayoutDirection::RightToLeft) { // In RTL, content offset is flipped horizontally. // We need to flip the culling context frame to match. - // See: - // https://github.com/facebook/react-native/blob/c2f39cfdd87c32b9a59efe8a788b8a03f02b0ea0/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm#L579 auto stateData = scrollViewShadowNode->getStateData(); cullingContext.frame.origin.x = stateData.contentBoundingRect.size.width - layoutMetrics.frame.size.width - cullingContext.frame.origin.x; } +#endif } else { cullingContext = {}; } diff --git a/packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js b/packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js index 72580d9c0485..d9f1f4e0c0c4 100644 --- a/packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js +++ b/packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js @@ -9,6 +9,7 @@ */ import type {RNTesterModuleExample} from '../../types/RNTesterTypes'; +import type {HostInstance} from 'react-native'; import RNTesterText from '../../components/RNTesterText'; import ScrollViewPressableStickyHeaderExample from './ScrollViewPressableStickyHeaderExample'; @@ -17,6 +18,7 @@ import * as React from 'react'; import {cloneElement, useCallback, useRef, useState} from 'react'; import { Platform, + Pressable, RefreshControl, ScrollView, StyleSheet, @@ -294,7 +296,7 @@ const examples: Array = [ name: 'stubbyHorizontalScrollView', title: ' (horizontal = true) in RTL not filling content\n', description: - 'A horizontal RTL ScrollView whose content is smaller thatn its containner', + 'A horizontal RTL ScrollView whose content is smaller than its container', render(): React.Node { return ( @@ -303,6 +305,24 @@ const examples: Array = [ ); }, }, + { + name: 'horizontalScrollViewMeasureRTL', + title: ' (horizontal = true) in RTL measure\n', + description: + 'Scroll to the end and tap the button. Touch x and measureInWindow x should match.', + render(): React.Node { + return ; + }, + }, + { + name: 'horizontalScrollViewContentOffsetRTL', + title: ' (horizontal = true) in RTL contentOffset\n', + description: + 'contentOffset, scrollTo and onScroll all count x from the left edge. Remounting and tapping scrollTo should show the same page.', + render(): React.Node { + return ; + }, + }, { title: ' enable & disable\n', description: 'ScrollView scrolling behaviour can be disabled and enabled', @@ -810,6 +830,104 @@ const HorizontalScrollView = (props: { ); }; +const HorizontalScrollViewMeasureRTL = () => { + const buttonRef = useRef(null); + const [touchX, setTouchX] = useState(null); + const [measuredX, setMeasuredX] = useState(null); + + return ( + + + + { + const {pageX, locationX} = event.nativeEvent; + setTouchX(Math.round(pageX - locationX)); + buttonRef.current?.measureInWindow(x => { + setMeasuredX(Math.round(x)); + }); + }}> + + Tap to measure + + + + + Touch x: {touchX ?? '-'} + measureInWindow x: {measuredX ?? '-'} + + ); +}; + +const RTL_PAGE_WIDTH = 200; +const RTL_PAGE_OFFSET = RTL_PAGE_WIDTH * 3; + +const HorizontalScrollViewContentOffsetRTL = () => { + const scrollRef = useRef>(); + const [mountKey, setMountKey] = useState(0); + const [scrollX, setScrollX] = useState(null); + + return ( + + {/* $FlowFixMe[incompatible-use] */} + { + setScrollX(Math.round(event.nativeEvent.contentOffset.x)); + }} + pagingEnabled={true} + scrollEventThrottle={16} + style={[styles.horizontalScrollView, {width: RTL_PAGE_WIDTH}]}> + {[0, 1, 2, 3, 4, 5].map(page => ( + + Page {page} + + ))} + + + onScroll x: {scrollX ?? '-'} + +