Skip to content

Do not track Android ScrollView scroll velocity if scroll change...#47046

Closed
DrRefactor wants to merge 1 commit into
react:mainfrom
DrRefactor:android-scroll-view-fling-velocity
Closed

Do not track Android ScrollView scroll velocity if scroll change...#47046
DrRefactor wants to merge 1 commit into
react:mainfrom
DrRefactor:android-scroll-view-fling-velocity

Conversation

@DrRefactor

@DrRefactor DrRefactor commented Oct 16, 2024

Copy link
Copy Markdown

…d outside gestures

Summary:

There's a bug in ScrollView that occasionally causes unexpected fling to happen.
For example:
Horizontal ScrollView created in rtl layout with overScrollMode='never' (this prop makes it easy to reproduce this bug).

Let's assume some data loaded immediately, some after a moment. This causes list size to change, therefore onLayoutChange is called in ReactHorizontalScrollView, which updates scroll offset accordingly:

adjustPositionForContentChangeRTL(left, right, oldLeft, oldRight);

This can e.g. change scroll from offset 5000 to 10000 (initial position).

Due to the way OnScrollDispatchHelper tracks scroll velocity now, this scroll change will be tracked as if it was made during a gesture, and thus its "velocity" is saved.

With saved velocity, making a "zero pixel gesture", e.g. trying to overscroll after the scrollview start, will cause fling to happen, because velocity is non-zero:

public void fling(int velocityX) {
(...)
final int correctedVelocityX =
        Build.VERSION.SDK_INT == Build.VERSION_CODES.P
            ? (int) (Math.abs(velocityX) * Math.signum(mOnScrollDispatchHelper.getXFlingVelocity()))
            : velocityX;

Note: This will happen after a manual .scrollTo() call, too.

Changelog:

Test Plan:

Without fix trying to overscroll list start, unexpected fling happens in the opposite direction:
broken

Fixed, no fling happens when trying to overscroll:
fixed

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 16, 2024
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@DrRefactor DrRefactor force-pushed the android-scroll-view-fling-velocity branch from 85728ce to 0eeb93b Compare October 16, 2024 11:08
@DrRefactor DrRefactor changed the title Do not save track Android ScrollView scroll velocity if scroll change… Do not track Android ScrollView scroll velocity if scroll change... Oct 16, 2024
@aattalla

Copy link
Copy Markdown

Any progress or update on PR to be merged soon?

@react-native-bot

Copy link
Copy Markdown
Collaborator

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.

@react-native-bot react-native-bot added Stale There has been a lack of activity on this issue and it may be closed soon. and removed Stale There has been a lack of activity on this issue and it may be closed soon. labels Oct 24, 2025
@react-native-bot

Copy link
Copy Markdown
Collaborator

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.

@react-native-bot react-native-bot added Stale There has been a lack of activity on this issue and it may be closed soon. and removed Stale There has been a lack of activity on this issue and it may be closed soon. labels Apr 22, 2026
@javache javache closed this May 6, 2026
@javache

javache commented May 6, 2026

Copy link
Copy Markdown
Contributor

This PR has gone stale. Please submit a new one if this is still relevant.

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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants