-
Notifications
You must be signed in to change notification settings - Fork 453
fix(ios): resolve scrollEnabled prop initialization timing issue #1041
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
base: master
Are you sure you want to change the base?
Conversation
7cf8e41 to
0a4bb73
Compare
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.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 2
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bug: Imperative method bypasses scrollEnabled state synchronization
The setScrollEnabledImperatively method directly modifies scrollView.scrollEnabled without updating the _scrollEnabled ivar. This creates state inconsistency where subsequent calls to applyScrollEnabled (during re-initialization or prop updates) will overwrite the imperatively set value with the stale ivar value, causing the scroll state to revert unexpectedly.
ios/RNCPagerViewComponentView.mm#L400-L403
react-native-pager-view/ios/RNCPagerViewComponentView.mm
Lines 400 to 403 in 0a4bb73
| - (void)setScrollEnabledImperatively:(BOOL)scrollEnabled { | |
| [scrollView setScrollEnabled:scrollEnabled]; | |
| } |
0a4bb73 to
5e3bb6b
Compare
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.
Bug: Imperative scroll method doesn't update state variable
The setScrollEnabledImperatively method directly modifies scrollView.scrollEnabled without updating the _scrollEnabled ivar. This creates inconsistency where calling this method before scrollView initialization has no effect, and the imperative setting won't be applied when applyScrollEnabled is later called during initialization. The method should update _scrollEnabled and call applyScrollEnabled to maintain consistency with the new state management pattern.
ios/RNCPagerViewComponentView.mm#L401-L404
react-native-pager-view/ios/RNCPagerViewComponentView.mm
Lines 401 to 404 in 5e3bb6b
| - (void)setScrollEnabledImperatively:(BOOL)scrollEnabled { | |
| [scrollView setScrollEnabled:scrollEnabled]; | |
| } |
Store scrollEnabled state in an ivar and apply it when the scroll view becomes available, rather than attempting to set it before initialization. This ensures the prop works reliably regardless of when props are updated relative to UIPageViewController setup. Also reset scrollEnabled to its default value (YES) in prepareForRecycle to prevent recycled components from retaining the previous instance's scroll state. Update setScrollEnabledImperatively to use the new state management pattern by updating the ivar and calling applyScrollEnabled, ensuring consistency whether called before or after scrollView initialization.
5e3bb6b to
db0029e
Compare
Summary
Store scrollEnabled state in an ivar and apply it when the scroll view becomes available, rather than attempting to set it before initialization. This ensures the prop works reliably regardless of when props are updated relative to UIPageViewController setup.
Fixes #1029
Test Plan
Check if scroll enabled works correctly.
Note
Store
scrollEnabledin an ivar and apply it once the innerUIScrollViewexists, updating consistently on prop/imperative changes and lifecycle resets.RNCPagerViewComponentView):scrollEnabledin_scrollEnabledwith defaultYES; reset on recycle.applyScrollEnabledand call it afterUIPageViewControllersetup, on prop changes, and in imperative setter.updatePropsto compare old/newscrollEnabled, set_scrollEnabled, then apply.scrollView.scrollEnabledwrites with deferred application via_scrollEnabled.Written by Cursor Bugbot for commit db0029e. This will update automatically on new commits. Configure here.