-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
fix(iOS): fire onMomentumScrollEnd when UIScrollView is removed from window #46277
fix(iOS): fire onMomentumScrollEnd when UIScrollView is removed from window #46277
Conversation
Hi @cipolleschi, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this!
[super didMoveToWindow]; | ||
if (self.window == nil) { | ||
// Check if the ScrollView was in motion | ||
if (_scrollView.isDecelerating || _scrollView.isTracking == NO) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit for consistency:
if (_scrollView.isDecelerating || _scrollView.isTracking == NO) { | |
if (_scrollView.isDecelerating || !_scrollView.isTracking) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes done.
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@cipolleschi merged this pull request in b98b9f1. |
This pull request was successfully merged by @shubhamguptadream11 in b98b9f1 When will my fix make it into a release? | How to file a pick request? |
Hey @cipolleschi , Thanks in advance! |
0.72 is out of support, so no, sadly. |
@cipolleschi That's fantastic! Supporting this up until version 0.73 sounds like a solid plan. If there's anything I can do to assist or contribute in any way, I am happy to help!! |
I already opened cherry pick requests for 0.76. See reactwg/react-native-releases#510 and reactwg/react-native-releases#511 |
…window (#46277) Summary: Solves this issue: #46276 ## Changelog: <!-- Help reviewers and the release process by writing your own changelog entry. Pick one each for the category and type tags: For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [IOS] [ADDED] - fire onMomentumScrollEnd when UIScrollView is removed from window **Why the issue is happening?** The `onMomentumScrollEnd` event is typically triggered by the `UIScrollView` delegate methods `scrollViewDidEndDecelerating` and `scrollViewDidEndScrollingAnimation`. However, if the scroll view is removed from the window while navigating away, these delegate methods are not called, resulting in the event not being dispatched. This behaviour was particularly problematic in scenarios where a scroll view is in motion, and the user navigates away from the screen before the scrolling completes. In such cases, the `onMomentumScrollEnd` event would never fire, which further make scroll area un touchable or un responsive. **What we changed?** In the didMoveToWindow method, we added logic to handle the scenario where the UIScrollView is being removed from the window (i.e., when the component is unmounted or the user navigates away). Here’s a breakdown of the changes: - **Added a Check for Scroll State:** We check if the UIScrollView was decelerating or had stopped tracking (_scrollView.isDecelerating || _scrollView.isTracking == NO). - **Manually Triggered onMomentumScrollEnd:** If the scroll view was in motion and is being removed from the window, we manually trigger the `onMomentumScrollEnd` event to ensure that the final scroll state is captured. **_I had fixed this issue on both Old and New arch._** Pull Request resolved: #46277 Test Plan: Attaching a video with working solution: https://github.com/user-attachments/assets/1a1f3765-3f11-46c3-af18-330c88478db8 Reviewed By: andrewdacenko Differential Revision: D62374798 Pulled By: cipolleschi fbshipit-source-id: 014be8d313bab0257459dc4e53f5b0386a39d5e0
@cipolleschi I'm not familiar with the exact process. Could you kindly guide me through the steps involved in doing this? |
Just following up on my previous message. I understand you’re likely busy, but I was hoping to get your guidance on the steps for this process whenever you have a moment. |
Summary:
Solves this issue: #46276
Changelog:
[IOS] [ADDED] - fire onMomentumScrollEnd when UIScrollView is removed from window
Why the issue is happening?
The
onMomentumScrollEnd
event is typically triggered by theUIScrollView
delegate methodsscrollViewDidEndDecelerating
andscrollViewDidEndScrollingAnimation
. However, if the scroll view is removed from the window while navigating away, these delegate methods are not called, resulting in the event not being dispatched.This behaviour was particularly problematic in scenarios where a scroll view is in motion, and the user navigates away from the screen before the scrolling completes. In such cases, the
onMomentumScrollEnd
event would never fire, which further make scroll area un touchable or un responsive.What we changed?
In the didMoveToWindow method, we added logic to handle the scenario where the UIScrollView is being removed from the window (i.e., when the component is unmounted or the user navigates away). Here’s a breakdown of the changes:
Added a Check for Scroll State: We check if the UIScrollView was decelerating or had stopped tracking (_scrollView.isDecelerating || _scrollView.isTracking == NO).
Manually Triggered onMomentumScrollEnd: If the scroll view was in motion and is being removed from the window, we manually trigger the
onMomentumScrollEnd
event to ensure that the final scroll state is captured.I had fixed this issue on both Old and New arch.
Test Plan:
Attaching a video with working solution:
working.MP4