react-native
react-native copied to clipboard
Support for `ScrollView.maintainVisibleContentPosition` on Android
Summary
This PR adds the support for ScrollView's maintainVisibleContentPosition
property to Android. This property is currently only available on iOS and is especially useful for chat-like scrollviews, where you want the scroll position to stick after layout changes.
Fixes #29055
This PR will impact the documentation, so I opened a draft PR on facebook/react-native-website#2088.
Changelog
[Android] [Added] - ScrollView.maintainVisibleContentPosition
. This property is not iOS-only anymore.
Test Plan
Most of the new code is based on the iOS code. The implementation differs a bit but is working great. You can try it out on the RNTester app, I added a new example called ScrollViewExpandingExample
, available both on Android and iOS. GIFs below.
Platform | Engine | Arch | Size (bytes) | Diff |
---|---|---|---|---|
android | hermes | arm64-v8a | 9,196,911 | +6,442 |
android | hermes | armeabi-v7a | 8,722,544 | +6,440 |
android | hermes | x86 | 9,638,946 | +6,443 |
android | hermes | x86_64 | 9,606,540 | +6,448 |
android | jsc | arm64-v8a | 10,832,618 | +6,166 |
android | jsc | armeabi-v7a | 9,749,519 | +6,170 |
android | jsc | x86 | 10,870,053 | +6,173 |
android | jsc | x86_64 | 11,479,152 | +6,171 |
Base commit: 9b4f8e01442356f820e135fae3849063b2b8c92c
Platform | Engine | Arch | Size (bytes) | Diff |
---|---|---|---|---|
ios | - | universal | n/a | -- |
Base commit: 9b4f8e01442356f820e135fae3849063b2b8c92c
Here is an alternative native module implementation, for those who need maintainVisibleContentPosition
on Android now but don't want to maintain your own RN fork.
package com.yourapp;
import android.view.View;
import com.facebook.react.bridge.Promise;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContextBaseJavaModule;
import com.facebook.react.bridge.ReactMethod;
import com.facebook.react.uimanager.IllegalViewOperationException;
import com.facebook.react.uimanager.NativeViewHierarchyManager;
import com.facebook.react.uimanager.ReactShadowNode;
import com.facebook.react.uimanager.UIBlock;
import com.facebook.react.uimanager.UIImplementation;
import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.uimanager.UIManagerModuleListener;
import com.facebook.react.views.scroll.ReactScrollView;
import com.facebook.react.views.view.ReactViewGroup;
import java.util.HashMap;
public class ScrollViewMagicModule extends ReactContextBaseJavaModule {
private final ReactApplicationContext reactContext;
private HashMap<Integer, UIManagerModuleListener> uiManagerModuleListeners;
public ScrollViewMagicModule(ReactApplicationContext reactContext) {
super(reactContext);
this.reactContext = reactContext;
}
@Override
public String getName() {
return "ScrollViewMagic";
}
@Override
public void initialize() {
super.initialize();
this.uiManagerModuleListeners = new HashMap<>();
}
@ReactMethod
public void enableMaintainVisibleContentPosition(final int viewTag, final Promise promise) {
final UIManagerModule uiManagerModule = this.reactContext.getNativeModule(UIManagerModule.class);
this.reactContext.runOnUiQueueThread(new Runnable() {
@Override
public void run() {
try {
final ReactScrollView scrollView = (ReactScrollView)uiManagerModule.resolveView(viewTag);
final UIManagerModuleListener uiManagerModuleListener = new UIManagerModuleListener() {
private int minIndexForVisible = 0;
private int prevFirstVisibleTop = 0;
private View firstVisibleView = null;
@Override
public void willDispatchViewUpdates(final UIManagerModule uiManagerModule) {
uiManagerModule.prependUIBlock(new UIBlock() {
@Override
public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) {
ReactViewGroup mContentView = (ReactViewGroup)scrollView.getChildAt(0);
if (mContentView == null) return;
for (int ii = minIndexForVisible; ii < mContentView.getChildCount(); ++ii) {
View subview = mContentView.getChildAt(ii);
if (subview.getTop() >= scrollView.getScrollY()) {
prevFirstVisibleTop = subview.getTop();
firstVisibleView = subview;
break;
}
}
}
});
UIImplementation.LayoutUpdateListener layoutUpdateListener = new UIImplementation.LayoutUpdateListener() {
@Override
public void onLayoutUpdated(ReactShadowNode root) {
if (firstVisibleView == null) return;
int deltaY = firstVisibleView.getTop() - prevFirstVisibleTop;
if (Math.abs(deltaY) > 0) {
scrollView.setScrollY(scrollView.getScrollY() + deltaY);
}
uiManagerModule.getUIImplementation().removeLayoutUpdateListener();
}
};
uiManagerModule.getUIImplementation().setLayoutUpdateListener(layoutUpdateListener);
}
};
uiManagerModule.addUIManagerListener(uiManagerModuleListener);
int key = uiManagerModuleListeners.size() + 1;
uiManagerModuleListeners.put(key, uiManagerModuleListener);
promise.resolve(key);
} catch(IllegalViewOperationException e) {
promise.resolve(-1);
}
}
});
}
@ReactMethod
public void disableMaintainVisibleContentPosition(int key, Promise promise) {
if (key >= 0) {
final UIManagerModule uiManagerModule = this.reactContext.getNativeModule(UIManagerModule.class);
uiManagerModule.removeUIManagerListener(uiManagerModuleListeners.remove(key));
}
promise.resolve(null);
}
}
JS side:
const nativeModule: {
enableMaintainVisibleContentPosition(viewTag: number): Promise<number>;
disableMaintainVisibleContentPosition(handle: number): Promise<void>;
} = NativeModules.ScrollViewMagic;
// `ref` is the ref to your ScrollView / FlatList
useEffect(() => {
let cleanupPromise: Promise<number> | undefined;
if (Constants.isAndroid) {
const viewTag = findNodeHandle(ref.current);
cleanupPromise = nativeModule.enableMaintainVisibleContentPosition(
viewTag
);
}
return () => {
void cleanupPromise?.then((handle) => {
void nativeModule.disableMaintainVisibleContentPosition(handle);
});
};
}, [ref]);
Hi @chrisglein I saw you roaming around issue #29055 that this PR aims to fix. Since you are a contributor on this repo do you know if there are any next steps I'm missing to get this PR merged? It's been opened for a bit more than a month now and we'd like to have this scroll improvement for our app.
Hi @chrisglein I saw you roaming around issue #29055 that this PR aims to fix. Since you are a contributor on this repo do you know if there are any next steps I'm missing to get this PR merged? It's been opened for a bit more than a month now and we'd like to have this scroll improvement for our app.
I'm mainly acting as an issue first responder, helping logged issues have the best chance of success. I've tagged the issue for this to see if that helps it get attention. You may also want to ping the discord to see if you can get some eyes on this.
Thank you Chris! I joined the Discord 🤞
I would prefer @stackia's implementation, because it matches iOS implementation, and do not use debounce, which leads to possible lags in implementation
And another thing to check: on android just calling setScrollY()
does not affect scroll animation - if list scrolling by inertia via OverScroller class, altering scrollY on ScrollView does not affects internal values in android.widget.OverScroller
- so you need to stop it and re-launch again.
Note to anyone using @stackia's implementation (thank you @stackia!) with a FlatList: if you add more than (windowSize-1)/2 items to the top of your data, it loses the scroll position (because the previously visible item will have been reclaimed). You can either increase your window size, or ~~add data in (windowSize/2 - 1) sized chunks in a useEffect hook~~. (Turns out the adding-in-chunks thing was just wishful thinking.)
I have no idea if the same problem holds for the code in this pull request, as I have not tried to use it yet.
@maxoumime, have you given up on getting this merged? If not, have you seen @Strate's comments regarding @stackia's implementation? Curious your thoughts.
@shergin any plans of merging this?
@stackia Thanks for the solution provided. Is it okay if I use the code to publish it as npm package? It will be useful for other devs as well who don't want to get into native module setup :)
@stackia Thanks for the solution provided. Is it okay if I use the code to publish it as npm package? It will be useful for other devs as well who don't want to get into native module setup :)
It'll be great to publish as a much-easier-to-use npm package!
Awesome!! I will update here once it is ready
@stackia I have published an easy to use npm package that can be used as an alternative until react-native provides built-in fix for this. I have managed to test it properly for FlatList. Hopefully it will help some devs here :)
- https://www.npmjs.com/package/@stream-io/flat-list-mvcp
- https://github.com/GetStream/flat-list-mvcp
(PRs are welcome)
if you include the following text in your pr summary
fixes https://github.com/facebook/react-native/issues/29055
it will be linked to the relevant issue and other developers will be able to filter issues without an open pr. Thanks a lot for the great work
Hi all, sorry for the (long) delay here.
@Strate I believe we need to debounce so we don't search for the first visible view everytime onScrollChanged gets called. Removing it probably wouldn't impact most of the apps, but I don't think we should try it.
About reactScrollTo
(not setScrollY
) affecting the OverScroller, I didn't notice this issue but I agree it might happen in some scenarios. I don't think this PR should aim at fixing this though.
I like @stackia 's implementation too, it looks more like the iOS implementation but I'm not familiar enough with RN to know if it would be a better candidate to get merged. My initial goal for this PR was to be able to use it with an Expo managed workflow, which doesn't let you write your own Java modules without ejecting.
@maxoumime i've send some feedback directly to your code. I've already faced that kind of issues while implementing this flag in my project. Please check my gist, I've tried to resolve that issues already: https://gist.github.com/Strate/1bfb72b66379db7e73f89c57cd2103e7 This implementation may be not optimal, but works well.
@Strate
Thanks for your implementation! Could you share the VisibleItemsRange
?
@aanation you're welcome! I've added that class to gist: https://gist.github.com/Strate/1bfb72b66379db7e73f89c57cd2103e7
@aanation I've also added ReactScrollViewManager. There is some cool features, like event onVisibleItemsRangeChange
, and initialScroll
based on item index (not contentOffset)
@Strate :heart: :sunglasses: We will try your implementation
@Strate Have you tested your implementation with FlatList?
@aanation Yes, locally in my project both FlatList and ScrollView uses same native component.
Any update on this ? The support of maintainVisibleContentPosition for Android would be a huge improvement for the Flatlist component.
Hey @maxoumime is it possible for you to resolve the conflicts on this one? I am trying to catch attention on this PR!! https://twitter.com/vishtree1992/status/1382064000721162241?s=20
Not sure about the process, but what is the step required for this PR to get merged? Only resolving this conflict? Can't wait for it :)
Thanks @maxoumime :) I am watching CircleCI checks with the same enthusiasm as when I was watching Wimbledon tennis finals
@markoj3s 😂 Thanks for letting me know about the conflict. I'm not sure what the next step is... I think we need the "Needs: React Native team attention" label to get to the review stage. My last PR was merged quickly, but it was small and fixing a crash. This one is a bit more complex and might be breaking current Android apps that expect this property to be iOS-only.
@maxoumime @gabrieldonadel @chrisglein Seems like everything's looking good :)