Skip to content

Commit 8afc813

Browse files
committed
Review
1 parent ef04d74 commit 8afc813

3 files changed

Lines changed: 54 additions & 43 deletions

File tree

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

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,10 @@ @implementation RCTScrollViewComponentView {
106106

107107
CGPoint _contentOffsetWhenClipped;
108108

109+
// Set in `finalizeUpdates:`, when the content size and direction are known.
109110
std::optional<CGPoint> _pendingContentOffset;
110-
BOOL _contentOffsetNeedsUpdate;
111+
BOOL _isPendingContentOffsetFromState;
112+
BOOL _contentOffsetNeedsStateUpdate;
111113

112114
__weak UIView *_contentView;
113115

@@ -309,12 +311,13 @@ - (void)updateLayoutMetrics:(const LayoutMetrics &)layoutMetrics
309311
oldLayoutMetrics:(const LayoutMetrics &)oldLayoutMetrics
310312
{
311313
if (layoutMetrics.layoutDirection != oldLayoutMetrics.layoutDirection && !_pendingContentOffset) {
312-
_pendingContentOffset = [self _mirrorContentOffsetIfNeeded:_scrollView.contentOffset];
314+
_pendingContentOffset = [self _mirrorContentOffsetIfNeeded:_scrollView.contentOffset forState:YES];
315+
_isPendingContentOffsetFromState = YES;
313316
}
314317
[super updateLayoutMetrics:layoutMetrics oldLayoutMetrics:oldLayoutMetrics];
315318
if (layoutMetrics.layoutDirection == LayoutDirection::RightToLeft &&
316319
layoutMetrics.frame.size.width != oldLayoutMetrics.frame.size.width) {
317-
_contentOffsetNeedsUpdate = YES;
320+
_contentOffsetNeedsStateUpdate = YES;
318321
}
319322
if (layoutMetrics.layoutDirection != oldLayoutMetrics.layoutDirection) {
320323
CGAffineTransform transform = (layoutMetrics.layoutDirection == LayoutDirection::LeftToRight)
@@ -418,6 +421,7 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
418421
if (oldProps ? oldScrollViewProps.contentOffset != newScrollViewProps.contentOffset
419422
: newScrollViewProps.contentOffset != facebook::react::Point{}) {
420423
_pendingContentOffset = RCTCGPointFromPoint(newScrollViewProps.contentOffset);
424+
_isPendingContentOffsetFromState = NO;
421425
}
422426

423427
// RCTEnhancedScrollView specific props
@@ -477,29 +481,11 @@ - (void)updateState:(const State::Shared &)state oldState:(const State::Shared &
477481
_state = std::static_pointer_cast<const ScrollViewShadowNode::ConcreteState>(state);
478482
auto &data = _state->getData();
479483

480-
auto contentOffset = RCTCGPointFromPoint(data.contentOffset);
481484
if (!oldState) {
482-
/*
483-
* When <ScrollView /> is suspended, it is removed from view hierarchy and its offset is stored in
484-
* state. We want to restore this offset from the state but it must be snapped to be within UIScrollView's
485-
* content to remove any overscroll.
486-
*
487-
* This can happen, for example, with pull to refresh. The UIScrollView will be overscrolled into negative offset.
488-
* If the offset is not adjusted to be within the content area, it leads to a gap and UIScrollView does not adjust
489-
* its offset until user scrolls.
490-
*/
491-
492-
// Adjusting overscroll on the top.
493-
contentOffset.y = fmax(contentOffset.y, -_scrollView.contentInset.top);
494-
495-
// Adjusting overscroll on the left.
496-
contentOffset.x = fmax(contentOffset.x, -_scrollView.contentInset.left);
497-
498-
// TODO: T190695447 - Protect against over scroll on the bottom and right as well.
499-
// This is not easily done because we need to flip the order of method calls for
500-
// ShadowViewMutation::Insert. updateLayout must come before updateState.
501-
502-
_pendingContentOffset = contentOffset;
485+
// When <ScrollView /> is suspended, it is removed from view hierarchy and its offset is stored in state.
486+
// `finalizeUpdates:` restores it.
487+
_pendingContentOffset = RCTCGPointFromPoint(data.contentOffset);
488+
_isPendingContentOffsetFromState = YES;
503489
}
504490

505491
CGSize contentSize = RCTCGSizeFromSize(data.getContentSize());
@@ -509,7 +495,7 @@ - (void)updateState:(const State::Shared &)state oldState:(const State::Shared &
509495
}
510496

511497
if (_layoutMetrics.layoutDirection == LayoutDirection::RightToLeft && contentSize.width != _contentSize.width) {
512-
_contentOffsetNeedsUpdate = YES;
498+
_contentOffsetNeedsStateUpdate = YES;
513499
}
514500
_contentSize = contentSize;
515501
_containerView.frame = CGRect{RCTCGPointFromPoint(data.contentBoundingRect.origin), contentSize};
@@ -524,17 +510,25 @@ - (void)finalizeUpdates:(RNComponentViewUpdateMask)updateMask
524510
[super finalizeUpdates:updateMask];
525511

526512
if (_pendingContentOffset) {
527-
CGPoint contentOffset = [self _mirrorContentOffsetIfNeeded:*_pendingContentOffset];
513+
CGPoint contentOffset = [self _mirrorContentOffsetIfNeeded:*_pendingContentOffset
514+
forState:_isPendingContentOffsetFromState];
515+
if (_isPendingContentOffsetFromState) {
516+
// Snap a restored offset into the content to remove any overscroll, for example after pull to refresh.
517+
// TODO: T190695447 - Protect against over scroll on the bottom and right as well.
518+
contentOffset.x = fmax(contentOffset.x, -_scrollView.contentInset.left);
519+
contentOffset.y = fmax(contentOffset.y, -_scrollView.contentInset.top);
520+
}
528521
_pendingContentOffset.reset();
529522
if (!CGPointEqualToPoint(_scrollView.contentOffset, contentOffset)) {
523+
// `scrollViewDidScroll:` updates the state.
530524
_scrollView.contentOffset = contentOffset;
531-
_contentOffsetNeedsUpdate = NO;
525+
_contentOffsetNeedsStateUpdate = NO;
532526
}
533527
}
534528

535-
if (_contentOffsetNeedsUpdate) {
536-
_contentOffsetNeedsUpdate = NO;
537-
[self scrollViewDidScroll:_scrollView];
529+
if (_contentOffsetNeedsStateUpdate) {
530+
_contentOffsetNeedsStateUpdate = NO;
531+
[self _updateStateWithContentOffset];
538532
}
539533
}
540534

@@ -624,11 +618,16 @@ - (BOOL)_shouldDisableScrollInteraction
624618
return NO;
625619
}
626620

627-
// In RTL, UIKit offsets start on the right, but JS and state offsets start on the left.
628-
- (CGPoint)_mirrorContentOffsetIfNeeded:(CGPoint)offset
621+
/*
622+
* In RTL, UIKit offsets start on the right, but JS and state offsets start on the left.
623+
* When the content is narrower than the view, the JS offset is 0, like on Android.
624+
* The state offset is negative, because the shadow tree puts the content on the left.
625+
*/
626+
- (CGPoint)_mirrorContentOffsetIfNeeded:(CGPoint)offset forState:(BOOL)forState
629627
{
630628
if (_layoutMetrics.layoutDirection == LayoutDirection::RightToLeft) {
631-
offset.x = MAX(_scrollView.contentSize.width - _scrollView.bounds.size.width, 0) - offset.x;
629+
CGFloat maxOffset = _scrollView.contentSize.width - _scrollView.bounds.size.width;
630+
offset.x = (forState ? maxOffset : MAX(maxOffset, 0)) - offset.x;
632631
}
633632
return offset;
634633
}
@@ -637,7 +636,8 @@ - (CGPoint)_mirrorContentOffsetIfNeeded:(CGPoint)offset
637636
{
638637
auto metrics = ScrollViewEventEmitter::Metrics{};
639638
metrics.contentSize = RCTSizeFromCGSize(_scrollView.contentSize);
640-
metrics.contentOffset = RCTPointFromCGPoint([self _mirrorContentOffsetIfNeeded:_scrollView.contentOffset]);
639+
metrics.contentOffset = RCTPointFromCGPoint([self _mirrorContentOffsetIfNeeded:_scrollView.contentOffset
640+
forState:NO]);
641641
metrics.contentInset = RCTEdgeInsetsFromUIEdgeInsets(_scrollView.contentInset);
642642
metrics.containerSize = RCTSizeFromCGSize(_scrollView.bounds.size);
643643
metrics.zoomScale = _scrollView.zoomScale;
@@ -700,7 +700,7 @@ - (void)_updateStateWithContentOffset
700700

701701
_avoidAdjustmentForMaintainVisibleContentPosition = enableImmediateUpdateModeForContentOffsetChanges;
702702

703-
auto contentOffset = RCTPointFromCGPoint([self _mirrorContentOffsetIfNeeded:_scrollView.contentOffset]);
703+
auto contentOffset = RCTPointFromCGPoint([self _mirrorContentOffsetIfNeeded:_scrollView.contentOffset forState:YES]);
704704
BOOL isAccessibilityAPIUsed = _isAccessibilityAPIUsed;
705705
_state->updateState(
706706
[contentOffset, isAccessibilityAPIUsed](
@@ -735,7 +735,7 @@ - (void)prepareForRecycle
735735
_scrollView.zoomScale = 1.0;
736736
_scrollView.contentOffset = CGPointZero;
737737
_pendingContentOffset.reset();
738-
_contentOffsetNeedsUpdate = NO;
738+
_contentOffsetNeedsStateUpdate = NO;
739739
// Invalidate cached content size so that updateState: recalculates the
740740
// container frame after zoomScale reset (which may have mutated it in RTL).
741741
_contentSize = CGSizeZero;
@@ -1064,7 +1064,7 @@ - (void)scrollToOffset:(CGPoint)offset
10641064

10651065
- (void)scrollToOffset:(CGPoint)offset animated:(BOOL)animated
10661066
{
1067-
offset = [self _mirrorContentOffsetIfNeeded:offset];
1067+
offset = [self _mirrorContentOffsetIfNeeded:offset forState:NO];
10681068

10691069
if (CGPointEqualToPoint(_scrollView.contentOffset, offset)) {
10701070
return;

‎packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
#include <react/debug/react_native_assert.h>
1111
#include <react/renderer/core/LayoutMetrics.h>
1212

13-
#include <algorithm>
14-
1513
namespace facebook::react {
1614

1715
// NOLINTNEXTLINE(facebook-hte-CArray)
@@ -29,14 +27,15 @@ void ScrollViewShadowNode::updateStateIfNeeded() {
2927

3028
if (state.contentBoundingRect != contentBoundingRect) {
3129
state.contentBoundingRect = contentBoundingRect;
30+
#ifndef ANDROID
3231
// RTL scroll views start on the right until the native view reports.
3332
if (getState()->getRevision() == State::initialRevisionValue &&
3433
getLayoutMetrics().layoutDirection == LayoutDirection::RightToLeft &&
3534
state.contentOffset.x == 0) {
36-
state.contentOffset.x = std::max<Float>(
37-
0,
38-
contentBoundingRect.size.width - getLayoutMetrics().frame.size.width);
35+
state.contentOffset.x =
36+
contentBoundingRect.size.width - getLayoutMetrics().frame.size.width;
3937
}
38+
#endif
4039
setStateData(std::move(state));
4140
}
4241
}

‎packages/react-native/ReactCommon/react/renderer/mounting/internal/CullingContext.cpp‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,18 @@ CullingContext CullingContext::adjustCullingContextIfNeeded(
4646
}
4747

4848
cullingContext.transform = Transform::Identity();
49+
50+
#ifdef ANDROID
51+
auto layoutMetrics = scrollViewShadowNode->getLayoutMetrics();
52+
if (layoutMetrics.layoutDirection == LayoutDirection::RightToLeft) {
53+
// In RTL, content offset is flipped horizontally.
54+
// We need to flip the culling context frame to match.
55+
auto stateData = scrollViewShadowNode->getStateData();
56+
cullingContext.frame.origin.x =
57+
stateData.contentBoundingRect.size.width -
58+
layoutMetrics.frame.size.width - cullingContext.frame.origin.x;
59+
}
60+
#endif
4961
} else {
5062
cullingContext = {};
5163
}

0 commit comments

Comments
 (0)