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

No fling/momentum on scroll (Android Pie 9.0) #22925

Closed
alexandrius opened this issue Jan 9, 2019 · 18 comments
Closed

No fling/momentum on scroll (Android Pie 9.0) #22925

alexandrius opened this issue Jan 9, 2019 · 18 comments
Labels
Bug Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.

Comments

@alexandrius
Copy link

alexandrius commented Jan 9, 2019

Environment

React Native Environment Info:
System:
OS: macOS 10.14.2
CPU: (8) x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz
Memory: 990.90 MB / 16.00 GB
Shell: 5.3 - /bin/zsh
Binaries:
Node: 11.3.0 - /usr/local/bin/node
Yarn: 1.12.3 - /usr/local/bin/yarn
npm: 6.5.0 - /usr/local/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
iOS SDK:
Platforms: iOS 12.1, macOS 10.14, tvOS 12.1, watchOS 5.1
IDEs:
Android Studio: 3.2 AI-181.5540.7.32.5056338
Xcode: 10.1/10B61 - /usr/bin/xcodebuild
npmPackages:
react: 16.6.3 => 16.6.3
react-native: 0.57.8 => 0.57.8
npmGlobalPackages:
react-native-cli: 2.0.1
react-native-git-upgrade: 0.2.7

Description

I installed custom rom PixelExperience on my Xiaomi Redmi 5 Plus.
Since then I'm experiencing scroll issue. If you make the page lose focus - scroll lacks the fling or momentum behaviour.
This only happens in React Native apps.
Live demo.
https://www.youtube.com/watch?v=vG2fTEb_ICg
Emulator works just fine.

I don't think is the custom rom issue since another user (@vforvasile) has reported the same.
Android has some changes for scrolling element https://developer.android.com/about/versions/pie/android-9.0-changes-28#scrolling-element
I tried using SDK 26 and 27 but it didn't solve the issue

Reproducible Demo

The issue described above is present in expo too.
https://snack.expo.io/@alexandrius/c3VwcG

@alexandrius alexandrius changed the title No fling/momentum for scroll Android No fling/momentum on scroll (Android Pie 9.0) Jan 9, 2019
@react-native-bot react-native-bot added the Platform: Android Android applications. label Jan 9, 2019
@esmat-nawahda
Copy link

+1
I also have the same issue on Android 9 devices,

using: react-native 0.54.0

@alexandrius
Copy link
Author

I tried different ROM Resurrection Remix today. It has the same issue with fling.

@hramos hramos removed the Bug Report label Feb 6, 2019
@alexandrius
Copy link
Author

alexandrius commented Feb 7, 2019

I tried react-native 0.58.4 today still same issue. I will try to dig into react native code this Saturday to get the idea

@patrickkempff
Copy link
Contributor

@alexandrius thank you for digging into this

@alexandrius
Copy link
Author

alexandrius commented Feb 16, 2019

I have some updates regarding this issue.
I built React Native from source code and these are my observations:
I logged every value in ReactScrollView.java particularly in methods fling() and onOverScrolled() values didn't change before and after loosing focus.
Then I went ahead and removed code below from fling()

else if (mScroller != null) {
// FB SCROLLVIEW CHANGE

// We provide our own version of fling that uses a different call to the standard OverScroller
// which takes into account the possibility of adding new content while the ScrollView is
// animating. Because we give essentially no max Y for the fling, the fling will continue as long
// as there is content. See #onOverScrolled() to see the second part of this change which properly
// aborts the scroller animation when we get to the bottom of the ScrollView content.

int scrollWindowHeight = getHeight() - getPaddingBottom() - getPaddingTop();
mScroller.fling(
  getScrollX(), // startX
  getScrollY(), // startY
  0, // velocityX
  correctedVelocityY, // velocityY
  0, // minX
  0, // maxX
  Integer.MIN_VALUE, // minY
  Integer.MAX_VALUE, // maxY
  0, // overX
  scrollWindowHeight / 2 // overY
);

ViewCompat.postInvalidateOnAnimation(this);

// END FB SCROLLVIEW CHANGE
}

Everything seemed to start working after this change except https://github.com/wix/react-native-calendars. I couldn't expend calendar of Agenda component.
Also I understand this is useful logic so it was not an option.

Since the behaviour resembles the behaviour of ScrollView inside ScrollView on native Android. I went ahead and extended ReactScrollView with NestedScrollView

public class ReactScrollView extends NestedScrollView implements ReactClippingViewGroup, ViewGroup.OnHierarchyChangeListener, View.OnLayoutChangeListener

Everything started to work just fine after this.
Here's comparison
https://www.diffchecker.com/EQhp2UJY

@alexandrius
Copy link
Author

@patrickkempff Do you suggest creating my own react-native aar package or do you think #23490 can be implemented in short time?

@cpojer
Copy link
Contributor

cpojer commented Feb 18, 2019

See my reply on the PR.

@cpojer cpojer closed this as completed Feb 18, 2019
@alexandrius
Copy link
Author

OK thanks. But please I see no reason to close this issue. I will try to fix the code without switching to NestedScrollView

@alexandrius
Copy link
Author

alexandrius commented Feb 20, 2019

OK this issue is much bigger than I thought. I think it's something related to javascript engine of AOSP roms. I'll try to dig deeper for this issue

@alexandrius
Copy link
Author

0.59.0 rc2 has same issue.
I will try https://github.com/react-native-community/jsc-android-buildscripts on the weekend

@xgAnd
Copy link

xgAnd commented Feb 27, 2019

What is the solution to this problem? @cpojer I can dot find your RP can you support in this ,thanks a lot.

@christianbach
Copy link

christianbach commented Feb 27, 2019

We had a problem with bad scrolling performance on Android 9.

In our case we hade a Swiper component that rendered a couple of screens ~6-7 each screen rendered a ActivityIndicator. Removing the extra ActivityIndicator solved the problem for us.

@alexandrius
Copy link
Author

@cpojer @patrickkempff Can you please give me a tip why might this happening? I just need some information where should I be digging at.

Besides swapping ScrollView to NestedScrollView. I tried to comment out some ReactActivity lifecycle methods but the issue is still there. I have only one idea why this might be happening: When the view is overlapped by another view or window the fling behavior stops working.

@alexandrius
Copy link
Author

alexandrius commented Mar 12, 2019

OK just after writing the comment above I managed to finally fix this issue. It's far from creating PR but here's some insight:
It seems the OverScroller instance is different. Before and after loosing the main focus. I still need to dig more to get why is this even happening.
I added this code to fling()

      OverScroller scroller = getOverScrollerFromParent();
      Log.d("OVERSCROLLER_ALEXANDRIUS = NEW", scroller.toString());
      Log.d("OVERSCROLLER_ALEXANDRIUS = OLD", mScroller.toString());

Here are logs before loosing focus (using the same App.js code I posted when I originally opened the issue). Hash codes are the same

OVERSCROLLER_ALEXANDRIUS = NEW: android.widget.OverScroller@f9abd9d
OVERSCROLLER_ALEXANDRIUS = OLD: android.widget.OverScroller@f9abd9d

And after loosing code

OVERSCROLLER_ALEXANDRIUS = NEW: android.widget.OverScroller@5b3023c
OVERSCROLLER_ALEXANDRIUS = OLD: android.widget.OverScroller@f9abd9d

As you can see the hash codes are different. (@5b3023c vs @f9abd9d)

After I add this code the issue is resolved.

      OverScroller scroller = getOverScrollerFromParent();
      if (mScroller != scroller) {
        mScroller = scroller;
      }

I will try to test native Android app with custom OverScroller fling if the issue is there too. This is probably Android issue not React Native

@alexandrius
Copy link
Author

For anyone experiencing the issue I suggest applying this patch. I understand this is a hacky way of fix so I'm not going to create a PR.

diff --git a/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java
index d39159a..7d0ea33 100644
--- a/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java
+++ b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java
@@ -49,7 +49,7 @@ public class ReactScrollView extends ScrollView implements ReactClippingViewGrou
   private static boolean sTriedToGetScrollerField = false;
 
   private final OnScrollDispatchHelper mOnScrollDispatchHelper = new OnScrollDispatchHelper();
-  private final @Nullable OverScroller mScroller;
+  private @Nullable OverScroller mScroller;
   private final VelocityHelper mVelocityHelper = new VelocityHelper();
   private final Rect mRect = new Rect(); // for reuse to avoid allocation
 
@@ -330,6 +330,12 @@ public class ReactScrollView extends ScrollView implements ReactClippingViewGrou
       // as there is content. See #onOverScrolled() to see the second part of this change which properly
       // aborts the scroller animation when we get to the bottom of the ScrollView content.
 
+      OverScroller scroller = getOverScrollerFromParent();
+      
+      if (mScroller != scroller) {
+        mScroller = scroller;
+      }
+
       int scrollWindowHeight = getHeight() - getPaddingBottom() - getPaddingTop();
 
       mScroller.fling(

facebook-github-bot pushed a commit that referenced this issue Apr 23, 2019
Summary:
React Native Environment Info:
    System:
      OS: Linux 4.15 Ubuntu 18.04.1 LTS (Bionic Beaver)
      CPU: (4) x64 Intel(R) Core(TM) i5-7300U CPU @ 2.60GHz
      Memory: 1.12 GB / 15.55 GB
      Shell: 4.4.19 - /bin/bash
    Binaries:
      Node: 8.10.0 - /usr/bin/node
      Yarn: 1.13.0 - /usr/local/bin/yarn
      npm: 3.5.2 - /usr/bin/npm
    SDKs:
      Android SDK:
        API Levels: 16, 19, 22, 23, 24, 25, 26, 27, 28
        Build Tools: 23.0.1, 23.0.3, 25.0.0, 25.0.2, 25.0.3, 26.0.1, 26.0.3, 27.0.3, 28.0.2, 28.0.3
        System Images: android-16 | Google APIs Intel x86 Atom, android-19 | Google APIs Intel x86 Atom, android-24 | Google Play Intel x86 Atom, android-27 | Google APIs Intel x86 Atom, android-28 | Intel x86 Atom_64, android-28 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom
    npmPackages:
      react: 16.8.6 => 16.8.6
      react-native: git+https://github.com/facebook/react-native.git#v0.59.5 => 0.59.5
    npmGlobalPackages:
      react-native-cli: 2.0.1
      react-native-git-upgrade: 0.2.7

The workaround implemented in #21117 tries to fix
https://issuetracker.google.com/issues/112385925 scroll direction (according to the latest comments, the scroll direction problem has been reverted in security patches so not sure if the workaround is still valid).

But... proposed solution in fling method is using signum which leads to zero computedVelocityY in case of zero mOnScrollDispatchHelper.getYFlingVelocity() on old devices(Samsung s4 mini) even when real velocityY is non zero

```
final int correctedVelocityY = (int)(Math.abs(velocityY) * Math.signum(mOnScrollDispatchHelper.getYFlingVelocity()));
```

Proposed solution is to take signum from original velocityY in case of zero
```
float signum = Math.signum(mOnScrollDispatchHelper.getYFlingVelocity());
if (signum == 0) {
  signum = Math.signum(velocityY);
}
final int correctedVelocityY = (int)(Math.abs(velocityY) * signum);
```

The symptoms are the same as described in issue #22925, but proposed workaround doesn't work.

[Android][fixed] - Fix smooth scrolling on old devices (SDK >=16)
Pull Request resolved: #24545

Differential Revision: D15044834

Pulled By: cpojer

fbshipit-source-id: 3f523eb1a438df774e22387aecded433b9031ab9
@charpeni
Copy link
Contributor

charpeni commented Jun 6, 2019

I can't reproduce that bug on Android 9, even on Expo with https://snack.expo.io/@alexandrius/c3VwcG.

Could anyone provide more information on how to replicate this?

@ReneMarquez
Copy link

I can't reproduce that bug on Android 9, even on Expo with https://snack.expo.io/@alexandrius/c3VwcG.

Could anyone provide more information on how to replicate this?

I'm encountering this bug, I upgraded to the latest react native (0.64) hoping it was fixed but nope, I can't replicate it on emulator but on a real device (nexus on android 9 aosp) its easy to replicate, just navigate from a screen that has a flatlist to another and go back or trigger an alert on that same screen, scrolling its like if you set the decelerationRate to 0

@alexandrius
Copy link
Author

@charpeni It's only happening on AOSP android roms.

@facebook facebook locked as resolved and limited conversation to collaborators Feb 18, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

10 participants