-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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(); | ||
Rect newFrame = new Rect(); | ||
firstVisibleView.getHitRect(newFrame); | ||
|
||
if (mHorizontal) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand correctly, the I thought you want to keep the scroll position unchanged, which means you would preserve the first visible frame? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
} | ||
} |
There was a problem hiding this comment.
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 computingmFirstVisibleView
, which here it does in listener towillDispatchViewUpdates
?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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.