Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add maintainVisibleContentPosition support on Android for legacy renderer #35049

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Libraries/Components/ScrollView/ScrollView.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ type IOSProps = $ReadOnly<{|
* visibility. Occlusion, transforms, and other complexity won't be taken into account as to
* whether content is "visible" or not.
*
* @platform ios
*/
maintainVisibleContentPosition?: ?$ReadOnly<{|
minIndexForVisible: number,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.views.scroll;

import android.graphics.Rect;
import android.view.View;
import android.view.ViewGroup;

import androidx.annotation.Nullable;

import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.bridge.UIManager;
import com.facebook.react.bridge.UIManagerListener;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.uimanager.UIManagerHelper;
import com.facebook.react.uimanager.common.ViewUtil;
import com.facebook.react.views.view.ReactViewGroup;
import com.facebook.react.views.scroll.ReactScrollViewHelper.HasSmoothScroll;

import java.lang.ref.WeakReference;

/**
* Manage state for the maintainVisibleContentPosition prop.
*
* This uses UIManager to listen to updates and capture position of items before and after layout.
*/
public class MaintainVisibleScrollPositionHelper<ScrollViewT extends ViewGroup & HasSmoothScroll> implements UIManagerListener {
private final ScrollViewT mScrollView;
private final boolean mHorizontal;
private @Nullable Config mConfig;
private @Nullable WeakReference<View> mFirstVisibleView = null;
private @Nullable Rect mPrevFirstVisibleFrame = null;
private boolean mListening = false;

public static class Config {
public final int minIndexForVisible;
public final @Nullable Integer autoScrollToTopThreshold;

Config(int minIndexForVisible, @Nullable Integer autoScrollToTopThreshold) {
this.minIndexForVisible = minIndexForVisible;
this.autoScrollToTopThreshold = autoScrollToTopThreshold;
}

static Config fromReadableMap(ReadableMap value) {
int minIndexForVisible = value.getInt("minIndexForVisible");
Integer autoScrollToTopThreshold =
value.hasKey("autoscrollToTopThreshold")
? value.getInt("autoscrollToTopThreshold")
: null;
return new Config(minIndexForVisible, autoScrollToTopThreshold);
}
}

public MaintainVisibleScrollPositionHelper(ScrollViewT scrollView, boolean horizontal) {
mScrollView = scrollView;
mHorizontal = horizontal;
}

public void setConfig(@Nullable Config config) {
mConfig = config;
}

/**
* Start listening to view hierarchy updates. Should be called when this is created.
*/
public void start() {
if (mListening) {
return;
}
mListening = true;
getUIManagerModule().addUIManagerEventListener(this);
}

/**
* Stop listening to view hierarchy updates. Should be called before this is destroyed.
*/
public void stop() {
if (!mListening) {
return;
}
mListening = false;
getUIManagerModule().removeUIManagerEventListener(this);
}

/**
* Update the scroll position of the managed ScrollView. This should be called after layout
* has been updated.
*/
public void updateScrollPosition() {
if (mConfig == null
|| mFirstVisibleView == null
|| mPrevFirstVisibleFrame == null) {
return;
}

View firstVisibleView = mFirstVisibleView.get();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateScrollPosition is getting called on layout change. Is this always going to happen after computing mFirstVisibleView, which here it does in listener to willDispatchViewUpdates?

In theory I think it should, as we will dispatch the mounting instruction in JS thread or UI thread (when sync update) before the native platform to update the layout. However, since we are doing the computing on UI thread asynchronously, I am not sure if there will be a race condition where the first visible view is computed after we called this method.

cc @javache what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If willDispatchViewUpdates is called from ui thread (which I’m not sure if it ever does) the dispatch to ui thread will be done synchronously by runOnUIThread. Ideally we could have a way to observe before / after views are updated on the ui thread but wanted to avoid adding more methods to UIManagerObserver for old arch. In my tests this always worked so I was fine with this solution.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janicduplessis firstVisibleView may be null, i ran into.
E/unknown:ReactNative: Exception in native call java.lang.NullPointerException: Attempt to invoke virtual method 'void android.view.View.getHitRect(android.graphics.Rect)' on a null object reference at com.facebook.react.views.scroll.MaintainVisibleScrollPositionHelper.updateScrollPositionInternal(MaintainVisibleScrollPositionHelper.java:107) at com.facebook.react.views.scroll.MaintainVisibleScrollPositionHelper.updateScrollPosition(MaintainVisibleScrollPositionHelper.java:97) at com.facebook.react.views.scroll.ReactScrollView.onLayoutChange(ReactScrollView.java:1128) at android.view.View.layout(View.java:20717) at android.view.ViewGroup.layout(ViewGroup.java:6198) at com.facebook.react.uimanager.NativeViewHierarchyManager.updateLayout(NativeViewHierarchyManager.java:254) at com.facebook.react.uimanager.NativeViewHierarchyManager.updateLayout(NativeViewHierarchyManager.java:222)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, yes it is missing a null check, I'll open a PR to fix it.

Rect newFrame = new Rect();
firstVisibleView.getHitRect(newFrame);

if (mHorizontal) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you could get rid of the branching logic for horizontal vs vertical scroll view, that would be great!

Branching and interface are two different ways we handle variants. Adding branching will usually be cleaner, but will create duplicate code easily. On the other hand, using interface to abstract the logic will be more challenging in coding, but the result is more flexible.
You may use ReactScrollViewHelper as an example. There's no branching there so duplication is reduced.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example of the interface you have in mind?

My thinking when organizing the code like this was to reduce duplication between the 2 implementations while also moving all code related to maintain visible content position out of the scrollview classes to avoid making them too big. The common logic when the variant specific is also very intertwined in some cases, see https://github.com/facebook/react-native/pull/35049/files/c069ae3e39595e7d7e7a3259c6004d1313ad6ba8#diff-b0c8b7c192810637724d9d2ed35db2b8921c45e153ebbda1ef8f34cb53837473R153 for example.

int deltaX = newFrame.left - mPrevFirstVisibleFrame.left;
if (deltaX != 0) {
int scrollX = mScrollView.getScrollX();
mScrollView.scrollTo(scrollX + deltaX, mScrollView.getScrollY());
mPrevFirstVisibleFrame = newFrame;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, the newFrame is the frame for the new firstVisibleView, and you've updated the scroll position back to the previous first visible frame. Why do you need to update the previous frame here?

I thought you want to keep the scroll position unchanged, which means you would preserve the first visible frame?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to avoid adjusting the scroll position multiple times. I had cases where this method would be called multiple times in a row.

if (mConfig.autoScrollToTopThreshold != null && scrollX <= mConfig.autoScrollToTopThreshold) {
mScrollView.reactSmoothScrollTo(0, mScrollView.getScrollY());
}
}
} else {
int deltaY = newFrame.top - mPrevFirstVisibleFrame.top;
if (deltaY != 0) {
int scrollY = mScrollView.getScrollY();
mScrollView.scrollTo(mScrollView.getScrollX(), scrollY + deltaY);
mPrevFirstVisibleFrame = newFrame;
if (mConfig.autoScrollToTopThreshold != null && scrollY <= mConfig.autoScrollToTopThreshold) {
mScrollView.reactSmoothScrollTo(mScrollView.getScrollX(), 0);
}
}
}
}

private @Nullable ReactViewGroup getContentView() {
return (ReactViewGroup) mScrollView.getChildAt(0);
}

private UIManager getUIManagerModule() {
return Assertions.assertNotNull(
UIManagerHelper.getUIManager(
(ReactContext) mScrollView.getContext(),
ViewUtil.getUIManagerType(mScrollView.getId())));
}

private void computeTargetView() {
if (mConfig == null) {
return;
}
ReactViewGroup contentView = getContentView();
if (contentView == null) {
return;
}

int currentScroll = mHorizontal ? mScrollView.getScrollX() : mScrollView.getScrollY();
for (int i = mConfig.minIndexForVisible; i < contentView.getChildCount(); i++) {
View child = contentView.getChildAt(i);
float position = mHorizontal ? child.getX() : child.getY();
if (position > currentScroll || i == contentView.getChildCount() - 1) {
mFirstVisibleView = new WeakReference<>(child);
Rect frame = new Rect();
child.getHitRect(frame);
mPrevFirstVisibleFrame = frame;
break;
}
}
}

// UIManagerListener

@Override
public void willDispatchViewUpdates(final UIManager uiManager) {
UiThreadUtil.runOnUiThread(
new Runnable() {
@Override
public void run() {
computeTargetView();
}
});
}

@Override
public void didDispatchMountItems(UIManager uiManager) {
// noop
}

@Override
public void didScheduleMountItems(UIManager uiManager) {
// noop
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.facebook.react.views.scroll.ReactScrollViewHelper.HasFlingAnimator;
import com.facebook.react.views.scroll.ReactScrollViewHelper.HasScrollEventThrottle;
import com.facebook.react.views.scroll.ReactScrollViewHelper.HasScrollState;
import com.facebook.react.views.scroll.ReactScrollViewHelper.HasSmoothScroll;
import com.facebook.react.views.scroll.ReactScrollViewHelper.ReactScrollViewScrollState;
import com.facebook.react.views.view.ReactViewBackgroundManager;
import java.lang.reflect.Field;
Expand All @@ -54,11 +55,14 @@
/** Similar to {@link ReactScrollView} but only supports horizontal scrolling. */
public class ReactHorizontalScrollView extends HorizontalScrollView
implements ReactClippingViewGroup,
ViewGroup.OnHierarchyChangeListener,
View.OnLayoutChangeListener,
FabricViewStateManager.HasFabricViewStateManager,
ReactOverflowViewWithInset,
HasScrollState,
HasFlingAnimator,
HasScrollEventThrottle {
HasScrollEventThrottle,
HasSmoothScroll {

private static boolean DEBUG_MODE = false && ReactBuildConfig.DEBUG;
private static String TAG = ReactHorizontalScrollView.class.getSimpleName();
Expand Down Expand Up @@ -107,6 +111,8 @@ public class ReactHorizontalScrollView extends HorizontalScrollView
private PointerEvents mPointerEvents = PointerEvents.AUTO;
private long mLastScrollDispatchTime = 0;
private int mScrollEventThrottle = 0;
private @Nullable View mContentView;
private @Nullable MaintainVisibleScrollPositionHelper mMaintainVisibleContentPositionHelper;

private final Rect mTempRect = new Rect();

Expand All @@ -127,6 +133,8 @@ public ReactHorizontalScrollView(Context context, @Nullable FpsListener fpsListe
I18nUtil.getInstance().isRTL(context)
? ViewCompat.LAYOUT_DIRECTION_RTL
: ViewCompat.LAYOUT_DIRECTION_LTR);

setOnHierarchyChangeListener(this);
}

public boolean getScrollEnabled() {
Expand Down Expand Up @@ -243,6 +251,19 @@ public void setOverflow(String overflow) {
invalidate();
}

public void setMaintainVisibleContentPosition(@Nullable MaintainVisibleScrollPositionHelper.Config config) {
if (config != null && mMaintainVisibleContentPositionHelper == null) {
mMaintainVisibleContentPositionHelper = new MaintainVisibleScrollPositionHelper(this, true);
mMaintainVisibleContentPositionHelper.start();
} else if (config == null && mMaintainVisibleContentPositionHelper != null) {
mMaintainVisibleContentPositionHelper.stop();
mMaintainVisibleContentPositionHelper = null;
}
if (mMaintainVisibleContentPositionHelper != null) {
mMaintainVisibleContentPositionHelper.setConfig(config);
}
}

@Override
public @Nullable String getOverflow() {
return mOverflow;
Expand Down Expand Up @@ -635,6 +656,17 @@ protected void onAttachedToWindow() {
if (mRemoveClippedSubviews) {
updateClippingRect();
}
if (mMaintainVisibleContentPositionHelper != null) {
mMaintainVisibleContentPositionHelper.start();
}
}

@Override
protected void onDetachedFromWindow() {
super.onDetachedFromWindow();
if (mMaintainVisibleContentPositionHelper != null) {
mMaintainVisibleContentPositionHelper.stop();
}
}

@Override
Expand Down Expand Up @@ -714,6 +746,18 @@ protected void onOverScrolled(int scrollX, int scrollY, boolean clampedX, boolea
super.onOverScrolled(scrollX, scrollY, clampedX, clampedY);
}

@Override
public void onChildViewAdded(View parent, View child) {
mContentView = child;
mContentView.addOnLayoutChangeListener(this);
}

@Override
public void onChildViewRemoved(View parent, View child) {
mContentView.removeOnLayoutChangeListener(this);
mContentView = null;
}

private void enableFpsListener() {
if (isScrollPerfLoggingEnabled()) {
Assertions.assertNotNull(mFpsListener);
Expand Down Expand Up @@ -1237,6 +1281,26 @@ private void setPendingContentOffsets(int x, int y) {
}
}

@Override
public void onLayoutChange(
View v,
int left,
int top,
int right,
int bottom,
int oldLeft,
int oldTop,
int oldRight,
int oldBottom) {
if (mContentView == null) {
return;
}

if (mMaintainVisibleContentPositionHelper != null) {
mMaintainVisibleContentPositionHelper.updateScrollPosition();
}
}

@Override
public FabricViewStateManager getFabricViewStateManager() {
return mFabricViewStateManager;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,16 @@ public void setContentOffset(ReactHorizontalScrollView view, ReadableMap value)
}
}

@ReactProp(name = "maintainVisibleContentPosition")
public void setMaintainVisibleContentPosition(ReactHorizontalScrollView view, ReadableMap value) {
if (value != null) {
view.setMaintainVisibleContentPosition(
MaintainVisibleScrollPositionHelper.Config.fromReadableMap(value));
} else {
view.setMaintainVisibleContentPosition(null);
}
}

@ReactProp(name = ViewProps.POINTER_EVENTS)
public void setPointerEvents(ReactHorizontalScrollView view, @Nullable String pointerEventsStr) {
view.setPointerEvents(PointerEvents.parsePointerEvents(pointerEventsStr));
Expand Down
Loading