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

Maps cooperative gesture handling #297

Closed
zeeke opened this issue Jan 6, 2017 · 10 comments
Closed

Maps cooperative gesture handling #297

zeeke opened this issue Jan 6, 2017 · 10 comments

Comments

@zeeke
Copy link

zeeke commented Jan 6, 2017

Hi,

The cooperative gesture handling described at this link in the documentation is not working due to the iPad fix in #233

  // src/google_map.js:867

 _onTouchMove = (event) => {
    if (this.refs.google_map_dom) {
      const mapDom = ReactDOM.findDOMNode(this.refs.google_map_dom);
      if (mapDom.contains(event.target)) {
        event.preventDefault();
      }
    }
  }	

I don't know how it would be on iPad without this fix. At the moment, scolling with a single finger on the map makes a warning appear but the page does not scroll. With a big map there is a chance that the user fall in a section where he can't scroll away.

I can work on a PR if some support.

@istarkov
Copy link
Collaborator

istarkov commented Jan 6, 2017

May be just check options on gestureHandling options, and depending on it just make off code section above. I cant help much here as have near zero knowledge about web on mobile. So feel free to provide a PR, will help if any questions I know the answer

@zeeke
Copy link
Author

zeeke commented Jan 7, 2017

I tried googling about maps and iPad issues but I find nothing.

@jooj123 can you give a feedback about the documentation i linked above?

@jooj123
Copy link
Contributor

jooj123 commented Jan 9, 2017

@zeeke

The ipad fix I added here: #233
Prevents the whole page from being dragged on an ipad.
Without it the user experience is horrible on ipad.

Instead of doing this with touchmove events in react - it could possibly be done with "Scrolling and Panning" settings (greedy option) with google maps.

But this will need to be tested - I have not checked if this prevents whats mentioned in #233

@yoadsn
Copy link
Contributor

yoadsn commented Mar 9, 2017

@zeeke I can see that you have forked this repo and reverted the PR for #233.
I tried the same locally and that indeed solves the problem.
@jooj123 @istarkov I think this change should be put behind a flag to be an opt-in. The reason is that google maps has it's own (complex) way of handling the duality of swipe over a map (scroll page? pan map?) with the gestureHandling option.

The only reason I'm not 100% sure this feature can be removed completely is due to handling of the rendered markers/layers on top of the gmaps. Perhaps this has no way of working with the gestureHandling feature - but currently it's really destroying this lib for any touch enabled use.

@istarkov
Copy link
Collaborator

istarkov commented Mar 9, 2017

Lets remove, @yoadsn can you check that greedy option? Ill accept a PR with removal :-) but dont like the flag idea

@yoadsn
Copy link
Contributor

yoadsn commented Mar 9, 2017

@istarkov Sounds good. I will wait to get last input from @jooj123 on this to make sure I am missing nothing.
Basically, what we want to say is that whenever a user of this component want to control the behavior of swipes on the map he should follow the google maps guidelines and options. (I would update the docs a little as well to make sure it's not missed).

@istarkov locally - the gestureHandling option of google map works great for me in the use cases I have. The setting to use to replicate #233 is indeed to use the greedy option.

greedy
greedy

auto With a scrollable container
auto

@itsmichaeldiego
Copy link
Member

@yoadsn I am guessing this was handled in PR #334 and this issue was never closed. Let me know if otherwise and we can re-open it.

@jooj123
Copy link
Contributor

jooj123 commented Apr 6, 2018

So is this defaulting to greedy now ?

@itsmichaeldiego
Copy link
Member

@jooj123 No, you need to set gestureHandling: 'greedy as detailed in #334

@lock
Copy link

lock bot commented Dec 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants