Uh oh!
There was an error while loading. Please reload this page.
Fix ScrollView momentum not stopping when calling scrollTo programmatically - #36104
Closed
tomekzaw wants to merge 1 commit into
Closed
Fix ScrollView momentum not stopping when calling scrollTo programmatically#36104tomekzaw wants to merge 1 commit into
tomekzaw wants to merge 1 commit into
Conversation
analysis-bot
commented
Feb 9, 2023
Base commit: 6d1667c |
facebook-github-bot
commented
Feb 9, 2023
Contributor
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
facebook-github-bot
commented
Feb 9, 2023
Contributor
@cortinico merged this pull request in 681b35d. |
facebook-github-bot pushed a commit
that referenced
this pull request
Aug 2, 2023
…inside a KeyboardAvoidingView (#38728) Summary: Starting from RN 0.72.0, when we nest a ScrollView inside a KeyboardAvoidingView, the ScrollView doesn't respond properly to the Keyboard on Android. https://github.com/facebook/react-native/assets/32062066/a62b5a42-6817-4093-91a2-7cc9e4a315bb This issue is due to a change made in #36104, which was added to fix#32235. That commit changed this line of code to abort the Scroller animation if a new call to the `scrollTo` method was made: https://github.com/facebook/react-native/blob/aab52859a447a8257b106fe307008af218322e3d/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java#L1073 Apparently, this is the same method that scrolls the ScrollView in response to the Keyboard opening on Android. So, here comes my proposal for a fix that doesn't break #36104 and fixes#38152. When we open the Keyboard, the call stack is as follows: - InputMethodManager - AndroidIME - InsetsController - `ReactScrollView.scrollTo` gets called When we use the ScrollView method `scrollTo` directly from the UI, the call stack is different as it goes through: - ReactScrollViewCommandHelper - ReactScrollViewManager - `ReactScrollView.scrollTo` gets called We can move `mScroller.abortAnimation();` from `ReactScrollView.scrollTo` to the `ReactScrollViewManager.scrollTo` method so that it gets called only when we call `scrollTo` from the UI and not when the `scrollTo` method is called by other sources. https://github.com/facebook/react-native/assets/32062066/9c10ded3-08e5-48e0-9a85-0987d62de011 ## Changelog: [ANDROID] [FIXED] - Fixed ScrollView not responding to Keyboard events when nested inside a KeyboardAvoidingView Pull Request resolved: #38728 Test Plan: You can see the issue and the proposed fixes in this repo: [kav-test-android](https://github.com/andreacassani/kav-test-android). Please refer to the `kav_test_fix` folder and to the [readme](https://github.com/andreacassani/kav-test-android/blob/main/README.md). Reviewed By: NickGerleman Differential Revision: D47972445 Pulled By: ryancat fbshipit-source-id: e58758d4b3d5318b947b42a88a56ad6ae69a539c
lunaleaps pushed a commit
that referenced
this pull request
Aug 7, 2023
…inside a KeyboardAvoidingView (#38728) Summary: Starting from RN 0.72.0, when we nest a ScrollView inside a KeyboardAvoidingView, the ScrollView doesn't respond properly to the Keyboard on Android. https://github.com/facebook/react-native/assets/32062066/a62b5a42-6817-4093-91a2-7cc9e4a315bb This issue is due to a change made in #36104, which was added to fix#32235. That commit changed this line of code to abort the Scroller animation if a new call to the `scrollTo` method was made: https://github.com/facebook/react-native/blob/aab52859a447a8257b106fe307008af218322e3d/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java#L1073 Apparently, this is the same method that scrolls the ScrollView in response to the Keyboard opening on Android. So, here comes my proposal for a fix that doesn't break #36104 and fixes#38152. When we open the Keyboard, the call stack is as follows: - InputMethodManager - AndroidIME - InsetsController - `ReactScrollView.scrollTo` gets called When we use the ScrollView method `scrollTo` directly from the UI, the call stack is different as it goes through: - ReactScrollViewCommandHelper - ReactScrollViewManager - `ReactScrollView.scrollTo` gets called We can move `mScroller.abortAnimation();` from `ReactScrollView.scrollTo` to the `ReactScrollViewManager.scrollTo` method so that it gets called only when we call `scrollTo` from the UI and not when the `scrollTo` method is called by other sources. https://github.com/facebook/react-native/assets/32062066/9c10ded3-08e5-48e0-9a85-0987d62de011 ## Changelog: [ANDROID] [FIXED] - Fixed ScrollView not responding to Keyboard events when nested inside a KeyboardAvoidingView Pull Request resolved: #38728 Test Plan: You can see the issue and the proposed fixes in this repo: [kav-test-android](https://github.com/andreacassani/kav-test-android). Please refer to the `kav_test_fix` folder and to the [readme](https://github.com/andreacassani/kav-test-android/blob/main/README.md). Reviewed By: NickGerleman Differential Revision: D47972445 Pulled By: ryancat fbshipit-source-id: e58758d4b3d5318b947b42a88a56ad6ae69a539c
Kudo pushed a commit
to expo/react-native
that referenced
this pull request
Aug 21, 2023
…inside a KeyboardAvoidingView (react#38728) Summary: Starting from RN 0.72.0, when we nest a ScrollView inside a KeyboardAvoidingView, the ScrollView doesn't respond properly to the Keyboard on Android. https://github.com/facebook/react-native/assets/32062066/a62b5a42-6817-4093-91a2-7cc9e4a315bb This issue is due to a change made in react#36104, which was added to fixreact#32235. That commit changed this line of code to abort the Scroller animation if a new call to the `scrollTo` method was made: https://github.com/facebook/react-native/blob/aab52859a447a8257b106fe307008af218322e3d/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java#L1073 Apparently, this is the same method that scrolls the ScrollView in response to the Keyboard opening on Android. So, here comes my proposal for a fix that doesn't break react#36104 and fixesreact#38152. When we open the Keyboard, the call stack is as follows: - InputMethodManager - AndroidIME - InsetsController - `ReactScrollView.scrollTo` gets called When we use the ScrollView method `scrollTo` directly from the UI, the call stack is different as it goes through: - ReactScrollViewCommandHelper - ReactScrollViewManager - `ReactScrollView.scrollTo` gets called We can move `mScroller.abortAnimation();` from `ReactScrollView.scrollTo` to the `ReactScrollViewManager.scrollTo` method so that it gets called only when we call `scrollTo` from the UI and not when the `scrollTo` method is called by other sources. https://github.com/facebook/react-native/assets/32062066/9c10ded3-08e5-48e0-9a85-0987d62de011 ## Changelog: [ANDROID] [FIXED] - Fixed ScrollView not responding to Keyboard events when nested inside a KeyboardAvoidingView Pull Request resolved: react#38728 Test Plan: You can see the issue and the proposed fixes in this repo: [kav-test-android](https://github.com/andreacassani/kav-test-android). Please refer to the `kav_test_fix` folder and to the [readme](https://github.com/andreacassani/kav-test-android/blob/main/README.md). Reviewed By: NickGerleman Differential Revision: D47972445 Pulled By: ryancat fbshipit-source-id: e58758d4b3d5318b947b42a88a56ad6ae69a539c
facebook-github-bot pushed a commit
that referenced
this pull request
Sep 20, 2023
…zontal scrollviews (#39529) Summary: ### Motivation My main motivation for this is using nested horizontal `Flashlists` inside a vertical `Flashlist`. Like a `RecyclerView`, since my horizontal lists get recycled, if I scroll say the first horizontal list and scroll down, then when this list gets recycled it continues scrolling even if the content is new:  To handle this, I want to call `scrollTo` everytime a new row appears to reset the scroll offset, however I've realized this doesn't stop the scroll momentum ### The bug When scrolling and calling `scrollTo`, scroll momentum should be stopped and we should scroll to where `scrollTo` asked for. All credit goes to tomekzaw for #36104 who fixed it for vertical scrollviews I realized we had the same issue for - horizontal scroll views - when calling `scrollToEnd` | Vertical scrollview (working ✅) | Horizontal scrollview (before fix, not stopping ❌) | Horizontal scrollview (after fix ✅)| |--------|--------|--| | ||| Based on #38728 I kept all those calls to `abortAnimation` on the View Manager ## Changelog: [ANDROID] [FIXED] - Fixed horizontal ScrollView momentum not stopping when calling scrollTo programmatically [ANDROID] [FIXED] - Fixed ScrollView momentum not stopping when calling scrollToEnd programmatically Pull Request resolved: #39529 Test Plan: My test code is [this](https://gist.github.com/Almouro/3ffcfbda8e2239e64bee93985e243000) Basically: - a scrollview with a few elements - some buttons to trigger a `scrollTo` To reproduce the bug, I scroll then click one of the buttons triggering a `scrollTo` I added `react-native@nightly` to my project, and copy pasted `packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll` folder from this branch to try out my fixes Then tested the following scenarios on Android: | List layout | Method | Not animated | Animated| |--------|--------|--|--| | horizontal | scrollTo | |  | | horizontal | scrollToEnd | | | | vertical | scrollTo | | | | vertical | scrollToEnd |  | | Reviewed By: javache Differential Revision: D49437886 Pulled By: ryancat fbshipit-source-id: 358c9b0eed7dabcbc9b87a15d1a757b414ef514b
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes#32235.
See #32235 (comment) for details.
Here's what I did:
scrollToin classandroid.view.View(make sure the API level of your emulator matchescompileSdkVersionso the sources are aligned with the bytecode)y == 0to the breakpoint so it doesn't pause execution when scrollingscrollTocommand withy = 0ReactScrollViewCommandHelperin the stack tracey == 0from the breakpoint, continue executiony != 0(in my case 49153 or something) – this is incorrect behavior as it overwrites the effect of the previous callmScroller.abortAnimation();just out of curiosityBefore:
recording.mov
After:
after.mov
Changelog
[ANDROID] [FIXED] - Fixed ScrollView momentum not stopping when calling scrollTo programmatically
Test Plan
Reproducer: https://github.com/tomekzaw/Issue32235/blob/master/App.tsx