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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable nested scrolling of horizontal scrollviews on Android #28138

Closed
wants to merge 1 commit into from

Conversation

isnotgood
Copy link

@isnotgood isnotgood commented Feb 20, 2020

Summary

On iOS nested scrolling in horizontal ScrollViews work out of box, on Android it's actually limited by Android's HorizontalScrollView implementation. By slight adjustment in ReactHorizontalScrollView we can have feature parity on Android as well.

Changelog

[Android] [Changed] - Enabled scrolling in nested horizontal ScrollViews

Test Plan

Before After
Only the top ScrollView is consuming the gesture so nested one doesn't have a chance After the fix the bottom one is targeted by gesture and nested scrolling works
android-before android-after

Steps to test

  1. Create fresh react native app with react native compiled from this branch
  2. Replace App.js with this gist
  3. Run android app and observe the nested horizontal scrollviews behaves as expected

I would love to write integration and probably some performance test but I'm having problem running them from Android Studio. Only thing I'm able to achieve is to run all test with buck. I would be grateful for any guidance. Also I understand that that lookup code could be performance bottleneck in some extreme cases and I'm willing to optimize it further if it will be considered worthy馃槃

@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 Feb 20, 2020
@react-native-bot react-native-bot added the Platform: Android Android applications. label Feb 20, 2020
@analysis-bot
Copy link

RNTester.app (iOS): 10657792 bytes

@analysis-bot
Copy link

RNTester (Android/hermes/arm64-v8a): 3289088 bytes
RNTester (Android/hermes/armeabi-v7a): 3129344 bytes
RNTester (Android/hermes/x86): 3461120 bytes
RNTester (Android/hermes/x86_64): 3424256 bytes
RNTester (Android/jsc/arm64-v8a): 4446208 bytes
RNTester (Android/jsc/armeabi-v7a): 4268032 bytes
RNTester (Android/jsc/x86): 4358144 bytes
RNTester (Android/jsc/x86_64): 4644864 bytes

@zpao zpao removed the request for review from olegbl February 20, 2020 17:01
@GuoMeng6
Copy link

I tried, but it is not useful

@isnotgood
Copy link
Author

@GuoMeng6 what do you mean?

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. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants