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

If we've disabled scrolling within the map, then don't capture the touch events #664

Merged
merged 2 commits into from
Oct 31, 2016

Conversation

mikelambert
Copy link
Contributor

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.)

@@ -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);
Copy link
Contributor

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.

Copy link
Contributor

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?

Copy link
Contributor Author

@mikelambert mikelambert Oct 14, 2016

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.

Copy link
Contributor

@gilbox gilbox Oct 15, 2016

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.)
@felipecsl felipecsl merged commit 2499f55 into react-native-maps:master Oct 31, 2016
@felipecsl
Copy link
Contributor

thanks @mikelambert!

gilbox pushed a commit that referenced this pull request Oct 31, 2016
…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
mikelambert added a commit to mikelambert/react-native-maps that referenced this pull request Nov 7, 2016
…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
timxyz pushed a commit to 3sidedcube/react-native-maps that referenced this pull request Nov 22, 2016
* 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
  ...
Exilz pushed a commit to archriss/react-native-maps that referenced this pull request Dec 9, 2016
…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
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