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

Add "propagateSwipe" prop to allow scrolling inside modal #246

Merged
merged 4 commits into from
Feb 6, 2019
Merged

Conversation

cjroth
Copy link
Contributor

@cjroth cjroth commented Feb 5, 2019

Thanks @mmazzarolo for figuring this out.

Fixes #236.

Copy link
Member

@mmazzarolo mmazzarolo left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you for updating the docs as well 🙇
See the comment in the review.

@@ -196,7 +198,9 @@ class ReactNativeModal extends Component {
// work correctly even when the modal has touchable buttons.
// For reference:
// https://github.com/react-native-community/react-native-modal/pull/197
return Math.abs(gestureState.dx) >= 4 || Math.abs(gestureState.dy) >= 4;
Copy link
Member

Choose a reason for hiding this comment

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

Mind adding a link to the PR here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@mmazzarolo mmazzarolo merged commit 4176867 into react-native-modal:master Feb 6, 2019
@mmazzarolo
Copy link
Member

@cjroth published on v7.1.0-beta.1, willing to give it a shot?
Thanks for the contribution!

@taylorkline
Copy link

taylorkline commented Feb 8, 2019

Sad to say this does not work for me. I've got a vertical ScrollView inside a modal with swipeDirection="down". The ScrollView will not scroll. I am logging propagateSwipe and it is true, as expected.

I'm afraid I do not have the time to create a minimal reproducible example at the moment.

@mmazzarolo
Copy link
Member

😢
@jkdf2 thanks for reporting.

@taylorkline
Copy link

Sure. Sorry to be the bearer of bad news.

Also, I would like to suggest that #236 and #109 be meged together. They appear to be about the same issue. I don't think the title is correct for #236, as v6.5.0 also does not allow vertical scrolling a ScrollView inside a modal with swipeDirection="down".

@mmazzarolo
Copy link
Member

Also, I would like to suggest that #236 and #109 be meged together.
You can merge issues on GitHub? 😮 Or maybe you meant to align the titles/discussions?

Thanks for the suggestions btw.

@taylorkline
Copy link

Yeah, sorry, I just mean close one, open the other, and link the two with a comment :)

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.

3 participants