Adding 'cooperative gestures' as an option when creating a map #1234
Conversation
…he interactive handling
…e event pass through
|
Bundle size report: Size Change: +101 B
ℹ️ View Details
|
|
Thanks for taking the time to open this PR! Did you test this on a map that is not full screen and made sure it behaves as expected as this is probably the main use case for this feature? |
Pinch-zoom and two-finger pitch change would be unaffected, because they would both successfully pass the requirement of 2 minimum touches to bypass the instruction screen. Yep, here's a quick gif of an inline that that demonstrates how the new feature enables navigation without getting stuck on the map: |
|
Can you share a stackblitz with this version so I can try it out? |
|
I suggest to run |
|
Lint commands are returning without errors and I have a demo with the build files posted here: https://js-ewvhew.stackblitz.io |
|
Thanks at @ewagstaff. I tried the demo on mobile and instead of panning, the map pitched when I used two fingers. |
Ah @wipfli I do see what you mean. Typically I have that gesture disabled. Is there a difference between what you'd expect to do for pitch change vs panning vertically in terms of how your fingers would move in a cooperative gesture setup? (Or should enabling coop gestures disable 2-finger pitch control?) |
|
I also usually turn pitch off. The gestures for pitch and pan vertically are the same for me. |
|
I wouldn't enforce something like a mutex. But maybe add a comment in the description |
|
I would let the user decide the settings freely. We can help them by telling about the pitch stuff in the description of the feature. Would also be great to add an example to the docs website and mention pitch there. |
|
Even if we let the user decide we still have a feature that contradicts another feature. |
|
Looking at other examples deployed live, both Google and Mapbox use a three-finger pitch gesture when cooperative gestures is enabled. What are your thoughts about that adjustment? |
|
I tested here ( https://developers.google.com/maps/documentation/javascript/examples/interaction-cooperative ) and the Ctrl + 2 fingers on the touchpad zoom into the map, instead of zooming into the page like happened with the test page above |
|
@acalcutt thanks for catching this, please try the demo site again when you can: https://js-ewvhew.stackblitz.io/ |
|
This seems to work good in general I think. All controls are working in desktop and mobile (need 3 fingers to change pitch which is acceptable I guess). |
|
@HarelM what device are you seeing that on? I see the "two fingers" language both on my iPhone 12 using Safari and an Android Galaxy Tab A using Chrome. It should swap out the language based on For the changelog, I suggested this language: To add that, would I just update the CHANGELOG.md file in the repo? Should I increment the version to 2.1.9? Working on adding test cases now. |
|
No need to increment version. |
|
I've added I've updated the changelog, and I added an additional media query check to display the "2 finger" language on any screen smaller than 480px if the feature media query fails. The demo should work on your S20 as expected: https://js-ewvhew.stackblitz.io/ |
|
Yes, tests that are in the |
|
Per your suggestion @HarelM I added the |
This feature was added to maplibre few months ago. maplibre/maplibre-gl-js#1234
This feature was added to maplibre few months ago. maplibre/maplibre-gl-js#1234
Launch Checklist
This does use the same option name as Mapbox uses, but
collaborativeGesturesseems to be the standard way to describe this behavior. All of the code was written from scratch.The main additions are in
map.tswhere I added the option for collaborative gestures when the map is created. If the boolean is true, this update will create a hidden div containing gesture instructions that gets shown and hidden by adding and removing a CSS class.Small additions were made in
scroll_zoom.tsandtouch_pan.tsto add the classes to show the div when appropriate.With the option off:
With the option on:
Write tests for all new functionality.
I've asked in the #maplibre OSM channel for guidance on how to structure a test of this new option. If a maintainer could point me in the direction of a similar example, a reference would be really helpful.
Document any changes to public APIs.
Post benchmark scores.
Maybe my computer is not dialed enough to run these benchmarks, because when I run
npm run benchmarksit fires up the processes but hasn't completed after a few hours.Manually test the debug page.
Suggest a changelog category: Feature
Add an entry inside this element for inclusion in the
maplibre-gl-jschangelog:<changelog>Added collaborativeGestures option when instantiating map to prevent inadvertent scrolling/panning when navigating a page where map is embedded inline #234 </changelog>.