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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(#722): revert not working back navigation gesture in favour of PagerView inside ScrollView #919

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

MateWW
Copy link
Contributor

@MateWW MateWW commented Nov 21, 2024

Summary

Resolves #722!

Issue

Currently scroll over legacy pager view is not working while nested inside scroll view.
Nesting might be necessary while we have big header which you don't want to be visible while you are swiping pages.

Root cause

The issue arise with 6.2.0 version and support for navigate back gesture.

Solution

I've tested that navigate back gesture and it didn't worked for me as you may see on attached video.
I've also checked if basic react native ScrollView supports that for horizontal mode and it's not.
Based on all above I believe it's not worth to block users and I reverted that change.
I think we can revisit that use case later and fix that long living regression.

Test Plan

Architecture Before After
Fabric
2-pager-view-bug-fabric-ios.mp4
2-pager-view-fix-fabric-ios.mp4
Paper
2-pager-view-bug-paper-ios.mp4
2-pager-view-fix-paper-ios.mp4

Compatibility

OS Implemented
iOS
Android

Checklist

  • I have tested this on a device and a simulator

@MateWW MateWW force-pushed the fix/#722/scroll-view-issue branch from 41a7e80 to 3e5ea73 Compare November 21, 2024 23:01
@MateWW MateWW requested a review from troZee November 21, 2024 23:03
@MateWW MateWW changed the title fix(#722): revert not working back navigation gesture in favour of PagerView inside ScrollView fix(722): revert not working back navigation gesture in favour of PagerView inside ScrollView Nov 21, 2024
@MateWW MateWW changed the title fix(722): revert not working back navigation gesture in favour of PagerView inside ScrollView fix(#722): revert not working back navigation gesture in favour of PagerView inside ScrollView Nov 21, 2024
@MrRefactor MrRefactor self-requested a review November 22, 2024 08:01
@MrRefactor
Copy link
Collaborator

This PR will be merged once we close this one: #918


<View>
<TabView
// TODO: Fix the height measurement for nested pager view
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove that TODO

@troZee
Copy link
Member

troZee commented Nov 22, 2024

@MateWW Could you check this PR https://github.com/callstack/react-native-pager-view/pull/771/files if this is the same thing?

@MateWW
Copy link
Contributor Author

MateWW commented Nov 22, 2024

@MateWW Could you check this PR https://github.com/callstack/react-native-pager-view/pull/771/files if this is the same thing?

yup, just in my case we have additional example in app to test that scenario

@MateWW MateWW force-pushed the fix/#722/scroll-view-issue branch from 3e5ea73 to 18aed2e Compare November 22, 2024 18:01
@MateWW MateWW force-pushed the fix/#722/scroll-view-issue branch from 18aed2e to 0cf7d6f Compare November 22, 2024 18:07
@troZee
Copy link
Member

troZee commented Nov 22, 2024

@MateWW Could you check this PR https://github.com/callstack/react-native-pager-view/pull/771/files if this is the same thing?

yup, just in my case we have additional example in app to test that scenario

Thank you 🙏 we will check that change once we test other PRs, which are already in a queue

@MateWW MateWW merged commit ef4b7da into master Nov 25, 2024
1 of 2 checks passed
@MateWW MateWW deleted the fix/#722/scroll-view-issue branch November 25, 2024 18:31
@MateWW MateWW restored the fix/#722/scroll-view-issue branch November 27, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6.2.0 breaks vertical scrolling of ScrollView parent
3 participants