Skip to content

Commit 252ef19

Browse files
David Rickardfacebook-github-bot
authored andcommitted
Allow partially visible items to be picked as scroll anchors for maintainVisibleContentPosition
Summary: The current behavior for `maintainVisibleContentPosition` on ScrollView is to pick the first fully visible item as the scroll anchor. This has a number of disadvantages: * It causes problems for lists with loading indicators and large items. The loading glimmer can be picked as the anchor and pull the scroll down too quickly. This is the case for Marketplace. * It's inconsistent with the [CSS Scroll Anchoring](https://www.w3.org/TR/css-scroll-anchoring-1/) behavior, which is to pick the first partially visible view. This change will switch to picking the first partially visible view as the anchor, to align with the CSS implementation. Discussed the change with yungsters, NickGerleman, and cipolleschi and agreed about the change in behavior. This also enables `maintainVisibleContentPosition` for Android. After adding it to `validAttributes` for Android it appears to be working well. Previously it was not functional at all on Android, as the property change from React was not passed to ReactScrollViewManager.java. ## Changelog: [General] [Changed] - maintainVisibleContentPosition property on ScrollView now selects the first partially visible view as the anchor, rather than the first fully visible view. Reviewed By: NickGerleman Differential Revision: D54223244 fbshipit-source-id: 05ddfc0bbf16d61f9599b9d8066c0bd21b086301
1 parent 32c74fb commit 252ef19

File tree

5 files changed

+42
-34
lines changed

5 files changed

+42
-34
lines changed

packages/react-native/Libraries/Components/ScrollView/ScrollView.js

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -266,33 +266,6 @@ type IOSProps = $ReadOnly<{|
266266
* @platform ios
267267
*/
268268
canCancelContentTouches?: ?boolean,
269-
/**
270-
* When set, the scroll view will adjust the scroll position so that the first child that is
271-
* currently visible and at or beyond `minIndexForVisible` will not change position. This is
272-
* useful for lists that are loading content in both directions, e.g. a chat thread, where new
273-
* messages coming in might otherwise cause the scroll position to jump. A value of 0 is common,
274-
* but other values such as 1 can be used to skip loading spinners or other content that should
275-
* not maintain position.
276-
*
277-
* The optional `autoscrollToTopThreshold` can be used to make the content automatically scroll
278-
* to the top after making the adjustment if the user was within the threshold of the top before
279-
* the adjustment was made. This is also useful for chat-like applications where you want to see
280-
* new messages scroll into place, but not if the user has scrolled up a ways and it would be
281-
* disruptive to scroll a bunch.
282-
*
283-
* Caveat 1: Reordering elements in the scrollview with this enabled will probably cause
284-
* jumpiness and jank. It can be fixed, but there are currently no plans to do so. For now,
285-
* don't re-order the content of any ScrollViews or Lists that use this feature.
286-
*
287-
* Caveat 2: This simply uses `contentOffset` and `frame.origin` in native code to compute
288-
* visibility. Occlusion, transforms, and other complexity won't be taken into account as to
289-
* whether content is "visible" or not.
290-
*
291-
*/
292-
maintainVisibleContentPosition?: ?$ReadOnly<{|
293-
minIndexForVisible: number,
294-
autoscrollToTopThreshold?: ?number,
295-
|}>,
296269
/**
297270
* The maximum allowed zoom scale. The default value is 1.0.
298271
* @platform ios
@@ -505,6 +478,33 @@ export type Props = $ReadOnly<{|
505478
* - `true`, deprecated, use 'always' instead
506479
*/
507480
keyboardShouldPersistTaps?: ?('always' | 'never' | 'handled' | true | false),
481+
/**
482+
* When set, the scroll view will adjust the scroll position so that the first child that is
483+
* partially or fully visible and at or beyond `minIndexForVisible` will not change position.
484+
* This is useful for lists that are loading content in both directions, e.g. a chat thread,
485+
* where new messages coming in might otherwise cause the scroll position to jump. A value of 0
486+
* is common, but other values such as 1 can be used to skip loading spinners or other content
487+
* that should not maintain position.
488+
*
489+
* The optional `autoscrollToTopThreshold` can be used to make the content automatically scroll
490+
* to the top after making the adjustment if the user was within the threshold of the top before
491+
* the adjustment was made. This is also useful for chat-like applications where you want to see
492+
* new messages scroll into place, but not if the user has scrolled up a ways and it would be
493+
* disruptive to scroll a bunch.
494+
*
495+
* Caveat 1: Reordering elements in the scrollview with this enabled will probably cause
496+
* jumpiness and jank. It can be fixed, but there are currently no plans to do so. For now,
497+
* don't re-order the content of any ScrollViews or Lists that use this feature.
498+
*
499+
* Caveat 2: This simply uses `contentOffset` and `frame.origin` in native code to compute
500+
* visibility. Occlusion, transforms, and other complexity won't be taken into account as to
501+
* whether content is "visible" or not.
502+
*
503+
*/
504+
maintainVisibleContentPosition?: ?$ReadOnly<{|
505+
minIndexForVisible: number,
506+
autoscrollToTopThreshold?: ?number,
507+
|}>,
508508
/**
509509
* Called when the momentum scroll starts (scroll which occurs as the ScrollView glides to a stop).
510510
*/

packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponent.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export const __INTERNAL_VIEW_CONFIG: PartialViewConfig =
4646
},
4747
decelerationRate: true,
4848
disableIntervalMomentum: true,
49+
maintainVisibleContentPosition: true,
4950
pagingEnabled: true,
5051
scrollEnabled: true,
5152
showsVerticalScrollIndicator: true,

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -752,13 +752,13 @@ - (void)_prepareForMaintainVisibleScrollPosition
752752
BOOL horizontal = _scrollView.contentSize.width > self.frame.size.width;
753753
int minIdx = props.maintainVisibleContentPosition.value().minIndexForVisible;
754754
for (NSUInteger ii = minIdx; ii < _contentView.subviews.count; ++ii) {
755-
// Find the first entirely visible view.
755+
// Find the first view that is partially or fully visible.
756756
UIView *subview = _contentView.subviews[ii];
757757
BOOL hasNewView = NO;
758758
if (horizontal) {
759-
hasNewView = subview.frame.origin.x > _scrollView.contentOffset.x;
759+
hasNewView = subview.frame.origin.x + subview.frame.size.width > _scrollView.contentOffset.x;
760760
} else {
761-
hasNewView = subview.frame.origin.y > _scrollView.contentOffset.y;
761+
hasNewView = subview.frame.origin.y + subview.frame.size.height > _scrollView.contentOffset.y;
762762
}
763763
if (hasNewView || ii == _contentView.subviews.count - 1) {
764764
_prevFirstVisibleFrame = subview.frame;

packages/react-native/React/Views/ScrollView/RCTScrollView.m

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -939,19 +939,19 @@ - (void)uiManagerWillPerformMounting:(RCTUIManager *)manager
939939
BOOL horz = [self isHorizontal:self->_scrollView];
940940
NSUInteger minIdx = [self->_maintainVisibleContentPosition[@"minIndexForVisible"] integerValue];
941941
for (NSUInteger ii = minIdx; ii < self->_contentView.subviews.count; ++ii) {
942-
// Find the first entirely visible view. This must be done after we update the content offset
942+
// Find the first partially or fully visible view. This must be done after we update the content offset
943943
// or it will tend to grab rows that were made visible by the shift in position
944944
UIView *subview = self->_contentView.subviews[ii];
945945
BOOL hasNewView = NO;
946946
if (horz) {
947947
CGFloat leftInset = self.inverted ? self->_scrollView.contentInset.right : self->_scrollView.contentInset.left;
948948
CGFloat x = self->_scrollView.contentOffset.x + leftInset;
949-
hasNewView = subview.frame.origin.x > x;
949+
hasNewView = subview.frame.origin.x + subview.frame.size.width > x;
950950
} else {
951951
CGFloat bottomInset =
952952
self.inverted ? self->_scrollView.contentInset.top : self->_scrollView.contentInset.bottom;
953953
CGFloat y = self->_scrollView.contentOffset.y + bottomInset;
954-
hasNewView = subview.frame.origin.y > y;
954+
hasNewView = subview.frame.origin.y + subview.frame.size.height > y;
955955
}
956956
if (hasNewView || ii == self->_contentView.subviews.count - 1) {
957957
self->_prevFirstVisibleFrame = subview.frame;

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
@Nullsafe(Nullsafe.Mode.LOCAL)
3535
class MaintainVisibleScrollPositionHelper<ScrollViewT extends ViewGroup & HasSmoothScroll>
3636
implements UIManagerListener {
37+
3738
private final ScrollViewT mScrollView;
3839
private final boolean mHorizontal;
3940
private @Nullable Config mConfig;
@@ -42,6 +43,7 @@ class MaintainVisibleScrollPositionHelper<ScrollViewT extends ViewGroup & HasSmo
4243
private boolean mListening = false;
4344

4445
public static class Config {
46+
4547
public final int minIndexForVisible;
4648
public final @Nullable Integer autoScrollToTopThreshold;
4749

@@ -160,7 +162,12 @@ private void computeTargetView() {
160162
int currentScroll = mHorizontal ? mScrollView.getScrollX() : mScrollView.getScrollY();
161163
for (int i = mConfig.minIndexForVisible; i < contentView.getChildCount(); i++) {
162164
View child = contentView.getChildAt(i);
163-
float position = mHorizontal ? child.getX() : child.getY();
165+
166+
// Compute the position of the end of the child
167+
float position =
168+
mHorizontal ? child.getX() + child.getWidth() : child.getY() + child.getHeight();
169+
170+
// If the child is partially visible or this is the last child, select it as the anchor.
164171
if (position > currentScroll || i == contentView.getChildCount() - 1) {
165172
mFirstVisibleView = new WeakReference<>(child);
166173
Rect frame = new Rect();

0 commit comments

Comments
 (0)