-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
If we've disabled scrolling within the map, then don't capture the touch events #664
Conversation
@@ -578,13 +578,16 @@ public boolean dispatchTouchEvent(MotionEvent ev) { | |||
|
|||
switch (action) { | |||
case (MotionEvent.ACTION_DOWN): | |||
this.getParent().requestDisallowInterceptTouchEvent(true); | |||
if (map != null && map.getUiSettings().isScrollGesturesEnabled()) { | |||
this.getParent().requestDisallowInterceptTouchEvent(true); |
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.
Thank you for this PR! It seems that this should be set to false
. I tested it using the included StaticMap
demo.
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.
tbh, I don't really understand what this code does but maybe it should be something like this?
if (map != null && map.getUiSettings().isScrollGesturesEnabled()) {
this.getParent().requestDisallowInterceptTouchEvent(false);
} else {
this.getParent().requestDisallowInterceptTouchEvent(true);
}
@felipecsl any thoughts?
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.
So when you say it should be set to false, you're saying you think the original code was wrong?
The function definition is requestDisallowInterceptTouchEvent(boolean disallowIntercept)
, so true disallows interception.
Previously, we always disallowed the touch event. This ensured that clicks in the map view would scroll in the map view, but the map view itself wouldn't be scrolled inside it's larger container.
In my case, I do <AirMap scrollEnabled=true>
, and my map view does nothing with the clicks. And I'd like the parent scroll container to scroll normally.
This code now ensures that we only disable the parent scroll container iff the map itself has scroll gestures enabled.
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.
@mikelambert All I did was checkout your branch and change true
to false
, and then the included Static Map demo worked as I expected. Did you try testing the Static Map demo? On iOS it allows you to drag over the map and it scrolls the ScrollView. In android, before I made this change it would drag the map, not the ScrollView. After the change, it would scroll the ScrollView with the map remaining unchanged.
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.
So...I'm confused now. :(
StaticMap demo works fine on Android for me, both before and after this pull request.
But this change was definitely required to make MapView work for my project, where the MapView was eating any drag/scroll attempts. I have tried a few things (matching props, simulating some slightly more complex nestings), but haven't been able to repro the "scroll-disabled map eating drag attempts" within the StaticMap demo yet. :(
So I'm probably as confused as you now, in that I don't really understand what the code does either anymore.
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.
quick update: this pr definitely hasn't been forgotten. The current situation is causing a bug in our app, but I haven't had time to loop back to it yet.
…uch events. This allows the containing scrollview to perform a scrollview scroll by dragging on the map. (Previously, the map would absorb the touches, and then not scroll the map *or* the scrollview, which was bad.)
thanks @mikelambert! |
…uch events (#664) * If we've disabled scrolling within the map, then don't capture the touch events. This allows the containing scrollview to perform a scrollview scroll by dragging on the map. (Previously, the map would absorb the touches, and then not scroll the map *or* the scrollview, which was bad.) * Minor simplification
…uch events (react-native-maps#664) * If we've disabled scrolling within the map, then don't capture the touch events. This allows the containing scrollview to perform a scrollview scroll by dragging on the map. (Previously, the map would absorb the touches, and then not scroll the map *or* the scrollview, which was bad.) * Minor simplification
* commit '3d28a7d9fd005d59219668cf61177fe574170b84': (24 commits) examples-setup.md: update android instructions (react-native-maps#743) add example for overlay overpress and docs iOS google maps custom tile support (react-native-maps#770) [Docs] Fix capitalisation of Xcode and CocoaPods (react-native-maps#749) Implements animateToRegion to Google Maps iOS (react-native-maps#779) [RN][iOS][google] Set region only when view has width&height - Fix type issue in AIRMapManager - Setup Gemfile in example/ios dir to avoid problems with different versions of cocoapods - Update examples-setup.md to use bundler - Change MapView so that we only set the native region prop when there is a width and height. GoogleMaps iOS requires the width and height to properly calculate the map zoom level. updates [marker flicker] Fix flicker of map pins on state change (react-native-maps#728) add onPress for polygons and polylines on iOS and Android Use latest Google Play Services (react-native-maps#731) Update installation.md (react-native-maps#742) Add latest patch releases to the changelog (react-native-maps#752) [ios][google] implement fitToSuppliedMarkers and fitToCoordinates (react-native-maps#750) If we've disabled scrolling within the map, then don't capture the touch events (react-native-maps#664) Fix dynamic imageSrc removal, fix flicker in react-native-maps#738 (react-native-maps#737) Fix Anchor point on Google Maps iOS Added ios google maps circle support Added google map type only check Fixed typo in google maps podspec Added ios google maps polygon, polyline, maptype support ...
…uch events (react-native-maps#664) * If we've disabled scrolling within the map, then don't capture the touch events. This allows the containing scrollview to perform a scrollview scroll by dragging on the map. (Previously, the map would absorb the touches, and then not scroll the map *or* the scrollview, which was bad.) * Minor simplification
This allows the containing scrollview to perform a scrollview scroll by dragging on the map. (Previously, the map would absorb the touches, and then not scroll the map or the scrollview, which was bad.)