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

Support for ScrollView.maintainVisibleContentPosition on Android #29466

Closed
wants to merge 28 commits into from

Conversation

maxoumime
Copy link
Contributor

@maxoumime maxoumime commented Jul 22, 2020

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.

react-horizontal-compressed
react-vertical-compressed

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 22, 2020
@react-native-bot react-native-bot added Platform: Android Android applications. Type: Enhancement A new feature or enhancement of an existing feature. labels Jul 22, 2020
@analysis-bot
Copy link

analysis-bot commented Jul 22, 2020

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: 9b4f8e0

@analysis-bot
Copy link

analysis-bot commented Jul 22, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 9b4f8e0

@stackia
Copy link

stackia commented Jul 27, 2020

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]);

@maxoumime
Copy link
Contributor Author

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.

@chrisglein
Copy link

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.

@maxoumime
Copy link
Contributor Author

Thank you Chris! I joined the Discord 🤞

@maxoumime
Copy link
Contributor Author

maxoumime commented Feb 2, 2022

Hey all, I'm going to find some time working on this, hopefully this month. Sorry for the delay

@ALexandreM75013
Copy link

Any update ?

@numandev1
Copy link
Contributor

@maxoumime any update?

@bigmoveus
Copy link

bigmoveus commented Jun 12, 2022

@maxoumime +1

1 similar comment
@andresribeiro
Copy link

@maxoumime +1

private ReactViewBackgroundManager mReactBackgroundManager;
private boolean mPagedArrowScrolling = false;
private int pendingContentOffsetX = UNSET_CONTENT_OFFSET;
private int pendingContentOffsetY = UNSET_CONTENT_OFFSET;
private final FabricViewStateManager mFabricViewStateManager = new FabricViewStateManager();
private @Nullable ReactScrollViewMaintainVisibleContentPositionData
mMaintainVisibleContentPositionData;
private @Nullable WeakReference<View> firstVisibleViewForMaintainVisibleContentPosition = null;

Choose a reason for hiding this comment

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

Why did you choose to use a WeakReference here?

@evgeniy-skakun
Copy link

any updates ?

@SwikarBhattarai
Copy link

any update ?

@ponikar
Copy link

ponikar commented Sep 22, 2022

Any updates??

@roryabraham
Copy link

roryabraham commented Sep 22, 2022

@janicduplessis and I have a working version of this feature completed in the Expensify fork of react-native, and are just working on more battle-testing of the feature before submitting PR(s) to the upstream repo. This PR was definitely a helpful start, but ultimately the implementation we landed on is this.

We've also implemented onStartReached in VirtualizedList and fixed some related bugs to support bidirectional pagination in FlatList/SectionList. All of this is still very much in development and has not been production-tested, but feel free to try it out if you want!

Disclaimer: Unfortunately it does not seem to work with Fabric enabled though (nor does the existing iOS implementation, AFAICT) 😞 It's unclear that a native implementation of this prop will be needed in a Fabric world, however. My guess is probably not.

Note: The latest version of the Expensify RN fork is 0.70.4, but really that's based on React Native's 0.70.1

fabOnReact added a commit to fabOnReact/react-native that referenced this pull request Sep 23, 2022
…kBack).

The logic was moved to ReactScrollView onLayoutChange.
Try to import some of the functionalities from PR facebook#29466
Use maintainVisibleContentPosition instead of existing functionality in FlatList implementation

PR 29466 facebook#29466
fabOnReact added a commit to fabOnReact/react-native that referenced this pull request Sep 23, 2022
Use maintainVisibleContentPosition instead of existing functionality in FlatList implementation
PR 29466 facebook#29466
@darkbasic
Copy link

Any plan to upstream your efforts?

@roryabraham
Copy link

roryabraham commented Sep 23, 2022

Yes, as stated above we want to production-test the features before attempting to upstream them. You can help out in that effort by using the Expensify fork of React Native in the meantime, which is available on npm just like the normal React Native repo.

If you do encounter issues specific to these new features, you can open an issue in our fork and we'll do our best to address them.

Edit: I realized that you can't create issues in our fork, so if you need to get our attention the best way will probably be to join our open-source slack

@roryabraham
Copy link

@janicduplessis opened a new PR which implements maintainVisibleContentPosition on Android: #35049. We've been using it in production for some time now 🙂

@github-actions
Copy link

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Needs: React Native Team Attention Platform: Android Android applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Stale There has been a lack of activity on this issue and it may be closed soon. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

maintainVisibleContentPosition is not available on android