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

MapLibre GL JS Default Scrolling Behavior Proposal #234

Closed
jxlarrea opened this issue Aug 1, 2021 · 13 comments
Closed

MapLibre GL JS Default Scrolling Behavior Proposal #234

jxlarrea opened this issue Aug 1, 2021 · 13 comments

Comments

@jxlarrea
Copy link

jxlarrea commented Aug 1, 2021

Motivation

The MapLibre default behavior when scrolling on a page is not desirable. This has been discussed extensively in the Mapbox GL JS issue #2618.

By default on both desktop and mobile devices when scrolling through a page, if the mouse cursor or touch events occurs on top of a map instance, MapLibre will hijack the scroll event, stop scrolling the page and initiates drag/pan (mobile) or scroll zoom (desktop) on the map.

Design Alternatives

In my opinion, Google Maps has the perfect page scrolling behavior and should be implemented into MapLibre either by default or as a control.

  • On desktop devices, scrollZoom should be enabled only if the user is holding down the "CTRL" key while using the scroll wheel. If the "CTRL" key is not being pressed down, an overlay is displayed with a message letting the user know that they must hold down the "CTRL" key to zoom in the map.

  • On mobile devices, dragPan should be enabled only if the user is using more than one finger to move around the map. If a touch event occurs on top of the map with only 1 finger, an overlay is displayed with a message letting the user know that they need to use two fingers to move around the map.

What do you guys think?

@HarelM
Copy link
Collaborator

HarelM commented Aug 1, 2021

Well, it a matter of teste I guess, I don't like the google map implementation of using two finger to pan. I find it not intuitive.
Also two fingers horizontal movement is currently kept for camera angle change.
I'm mainly using maplibre on full screen and not part of another page, so it makes more sense to me to use a single touch pan to move.
But I understand this is not the case for everyone, especially if the map is inside a page and you want to scroll the page and not the map.

I suggest the following as a solution:

  1. Keep the current default behaviors as is.
  2. Allow an easy setup for the alternative (google like) behavior i.e. something like map.setNonFullScreenBehavior() (or find a better name for this, but without using the word google). Calling this method will change all the default gestures according to the above requirements.
  3. If this idea is acceptable, the best solution will be to create a PR for this and discuss it further there.

@jxlarrea
Copy link
Author

jxlarrea commented Aug 1, 2021

@HarelM

But I understand this is not the case for everyone, especially if the map is inside a page and you want to scroll the page and not the map.

That's precisely the expected behavior when the map is inside a page, which is likely the most common use of MapLibre. Of course this wouldn't be the case using the map on full screen.

Allow an easy setup for the alternative (google like) behavior i.e. something like map.setNonFullScreenBehavior() (or find a better name for this, but without using the word google). Calling this method will change all the default gestures according to the above requirements.

How about creating a control that enables my proposed behavior? That would be the cleaner solution.

If this idea is acceptable, the best solution will be to create a PR for this and discuss it further there.

I'm not well versed in git so I will leave that to someone smarter than me. The proof of concept code is well annotated so it wouldn't be a problem if someone else wants to submit the PR.

@HarelM
Copy link
Collaborator

HarelM commented Aug 1, 2021

Creating a control, as far as I understand, requires user interacrion, which I don't think is what a site developer would want. Having said that, if you have a utility method that does that you can add a control to wrap that behavior.

Not sending a PR would result in lower chances this will be incorporated into this library.
If this is truly needed for your use case I suggest to take the time :-) this is a community driven project now. :-)

@jxlarrea
Copy link
Author

jxlarrea commented Aug 1, 2021

Creating a control, as far as I understand, requires user interacrion, which I don't think is what a site developer would want. Having said that, if you have a utility method that does that you can add a control to wrap that behavior.

Not at all. A control can do anything really. There are many controls out there that do not expose any UI functionality, like mapbox-omt-lang or mapbox-gl-language.

Not sending a PR would result in lower chances this will be incorporated into this library.
If this is truly needed for your use case I suggest to take the time :-) this is a community driven project now. :-)

I understand that but at least now the code is out there for the many people that want my proposed behavior on their maps embedded on their pages. :)

@HarelM
Copy link
Collaborator

HarelM commented Aug 1, 2021

P.S. There's also the possibility of writing this as a plugin...

@jxlarrea
Copy link
Author

jxlarrea commented Aug 1, 2021

P.S. There's also the possibility of writing this as a plugin...

@HarelM

Didn't know those existed. I thought extending functionality was done via controls. A plugin certainly sounds more appropriate.

EDIT: Actually just checked some plugin examples. Those are controls, maybe they were renamed at some point but they are implemented exactly the same way. Both plugins and controls are basically a class with onAdd() and onRemove() methods. Plugins are even added to a map instance using map.addControl() so yeah, both are the same thing.

@HarelM
Copy link
Collaborator

HarelM commented Nov 24, 2021

Was requested again as part of #661

@HarelM HarelM reopened this Nov 24, 2021
@ArtemFokin
Copy link

Please just implement 'cooperativeGestures' option, it's already included in mapbox-gl since v2.6
https://jsfiddle.net/api/post/library/pure/
https://www.mapbox.com/blog/mapbox-gl-js-v2-6
And also mapOptions={{cooperativeGestures: true}} doesn't work.

@HarelM
Copy link
Collaborator

HarelM commented Nov 25, 2021

We can't copy their code, it's proprietary now.

@jxlarrea
Copy link
Author

@ArtemFokin @HarelM Shortly after I opened this issue a few months ago I implemented it, even before it was a thing in Mapbox. I don't know how it works in Mapbox since I stopped using them a long time ago but the implementation behaves just like Google Maps. You can see it in action here: https://istheservicedown.com/problems/charter-communications/map

Honestly, it shouldn't be that hard to create an official implementation. I am clueless when it comes to javascript UX, DOM, UI events, etc. but still managed to implement it in 48 hours.

@HarelM
Copy link
Collaborator

HarelM commented Nov 25, 2021

@jxlarrea if you can share your code here it would be a good starting point.
I agree this shouldn't be too hard to implement.

@wimpie3
Copy link

wimpie3 commented Mar 10, 2022

@jxlarrea would be nice if you could share your solution

@ewagstaff
Copy link
Contributor

I did my best to implement with this PR: #1234

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

No branches or pull requests

5 participants