Add gestureHandling map option to implement scroll zoom blocker

[avpeery]

This PR closes #10227 and is an alternative approach to PR #10962 which implements the scroll zoom blocker as a control with more customization capabilities.

@mapbox/map-design-team @mapbox/docs 👋 Hello design and docs teams! For some time, I have been working on an approach to implement a scroll zoom blocker with an alert message. There is another PR (linked above) with the other approach that allows for more customization and complexity for the scroll zoom blocker as it implements it as a map control. Please chime in if you have thoughts! Thanks!

In order to address the issue of incidentally zooming the map instead of scrolling the page, this PR introduces a new feature that allows for scroll-zoom control, by requiring pressing CTRL key while scrolling in order to zoom. There is an alert that appears when scrolling over the map without pressing the CTRL key. This feature has been implemented as a part of the ScrollZoomHandler class.

The benefit of using the approach is a minimal solution that may be more applicable for the general client implementation and does not introduce possibly unneeded customizations/complexity to the codebase. In addition, this approach currently only implements with using the CTRL key + scroll in order to zoom the map since navigator.platform to determine platform specific responses is not considered best practice, and is in the process of being deprecated.

You can opt into the scroll zoom blocker while implementing the map:

var map = window.map = new mapboxgl.Map({
    container: 'map',
    zoom: 12.5,
    center: [-77.01866, 38.888],
    style: 'mapbox://styles/mapbox/streets-v10',
    gestureHandling: true
});

Note that if scrollZoom is disabled, gestureHandling for scrollZoom will be removed and re-addded if scrollZoom is enabled again later.

Check out the debug page: scroll_zoom_blocker.html.

Here's a screen recording -->

Screen.Recording.2021-09-17.at.2.08.25.PM.mov

UPDATE This option has been chosen over the control implementation due to the simplicity, and less code needed. The opportunity for customization in the control implementation approach didn't feel necessary in most use-cases. I have also made a decision to prevent the scroll zoom blocker when the map is full screen via full screen control.

UPDATE 9/30 In order for the map option to apply to touch devices, the initial implementation scrollZoom: {requireCtrl: true} has been replaced with gestureHandling: true. This will better accommodate when touch events (e.g. TouchPan, TouchZoom, TouchRotate, TouchPitch, TapZoom, SwipeZoom) are also handled by requiring two fingers to move map (next action item).

Launch Checklist

• briefly describe the changes in this PR
• include before/after visuals or gifs if this PR includes visual changes
• write tests for all new functionality
• document any changes to public APIs
• manually test the debug page
• tagged @mapbox/map-design-team @mapbox/static-apis if this PR includes style spec API or visual changes
• apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
• add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Adds new gestureHandling map option to opt into requiring ctrl or meta key-press while scrolling to zoom map</changelog>

[mbullington]

Thanks for looping us in @avpeery !

I'm super happy to see this feature being worked on and the potential it has for our embed experience. Looks great! I left a small comment about fonts I was wondering.

[mourner]

Overall this looks great aside for a few nits, and I like this approach more for its simplicity.

[mourner]

We can try moving as much logic/styling as possible to CSS instead of hardcoding in JS — e.g. opacity: 0 in that class, and use transition-delay property instead of having a setTimeout here.

[avpeery]

@mourner I had replaced the setTimeout with an event listener for transitionend event, but unfortunately it only worked as expected in Chrome, and failed to set off in firefox and safari. I still needed to add something to either listen to or wait for the css changes, otherwise it wouldn't have time to show the change. The transitionend event listener did not work as I needed it to because the way the scroll zoom blocker is set up in the wheel event, it is called very quickly multiple times. In safari and firefox, if it the transition is the same as how it started, the transitionend event listener stops firing altogether which occurred in this scenario.... so I reverted back to using setTimeout, is this a deal breaker? And if so, is there something else I could use to give time for the css changes to appear in between adding and removing the classes? Thanks so much!

[avpeery]

@mourner addressed the changes aside from the setTimeout as mentioned in the previous comment. Let me know if this looks good to go or if there are changes to be made still. Thanks!

[rreusser]

Just a few nits. This approach is a lot less code, which is nice. 👍 The biggest thing which I think we should probably address was being unable to use it due to accessibility features.

[katydecorah]

I like this approach @avpeery! I left one suggestion below to improve readability of the message.

[ansis]

I'm not sure if I've missed any discussion about font-size, but would a larger constant size be good? the text on the debug page looks a bit small

[avpeery]

I bumped up to 110%

[ansis]

Is percentage based sizing the best approach here?

[rreusser]

I think it's percentage of the nearest parent element with a font size, which is .mapboxgl-map, which has font: 12px/20px Helvetica Neue,Arial,Helvetica,sans-serif;. Thus, I believe this font size is identically, always 13.2px. I wonder if we couldn't maybe assign this dynamically as

_container.style.fontSize = `${Math.max(10, Math.min(30, Math.floor(Math.min(mapWidth, mapHeight) * 0.04)))}px`;

so that the message would scale with the container.

[avpeery]

So I implemented setting the fontSize when adding it to the map: this._alertContainer.style.fontSize = `${Math.max(10, Math.min(24, Math.floor(this._el.clientWidth * 0.05)))}px`; which seemed to be the magic number. I was trying to see if there is some built in method to check if the map has been resized to reset the fontSize without reloading the map?

[avpeery]

@ansis @rreusser let me know what you think of the font size and if there's anything else that should change or if it's good to go! Thanks!

[avpeery]

Updates: It has been decided to use navigator.userAgent to detect Mac and iPad devices to enable meta key use due to conflict if accessibility is enabled with the ctrl key. If detection fails, the message will display ctrl key message and user can still alternatively use ctrl key on mac.

Issue! One outstanding issue is transition event listener isn't working on firefox or safari (is working on chrome), and haven't tested other browsers. I attempted adding in these CSS transitions: webkit-transition, moz-transition, ms-transition, o-transition in addition to transition, but it did not solve the problem. 'transitionend' event listener is intended to be supported by all browsers as per docs: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/transitionend_event If anyone has any ideas on what could solve this, would appreciate any and all recommendations! Thanks! @rreusser @mourner @ansis

[rreusser]

It has been decided to use navigator.userAgent to detect Mac and iPad devices to enable meta key use due to conflict if accessibility is enabled with the ctrl key. If detection fails, the message will display ctrl key message and user can still alternatively use ctrl key on mac.

Is it possible to use platform detection strictly to select which message to display, but to always allow either meta or ctrl to enable the interaction? I think it's nice to show a friendly, familiar platform-dependent message and it's low-consequence if we get it wrong, but it seems somewhat more likely to cause bugs or quirks if it's more restrictive and platform dependent about the actual behavior. I don't really see a reason not to be more permissive about which keys enable zooming.

[rreusser]

Looks great to me! A couple nitpicks, but nothing blocking on my end. I miss the philosophical cleanliness of the completely decoupled control, but this version really is nice and simple. 👏 ❤️ 🚀

[rreusser]

I think this should be fine. I've previously confirmed that HTML entities work as well, e.g. ⌘: ⌘, just in case unicode is an issue anywhere between here and the docs, though I think it should be just fine as-is.

[mourner]

This looks great! Feel free to ignore the nits — just something I noticed but may not be relevant.

[mbullington]

Congrats! This will be nice for embedded maps, will play around with in Studio once the beta is out. 🎉

cc @mapbox/map-design-studio

[strikan95]

What happens if you want to use this and drag pan with drag rotate disabled? The user would have to hold ctrl to use zoom then release ctrl key to pan then hold ctrl again to zoom in/out then release than pan etc. Its too much acrobatics for an average user.

Maybe theres a way to allow mouse panning while having ctrl pressed down that I missed?

[avpeery]

Hi @strikan95, you will still be able to use drag pan without holding down the ctrl key if it is enabled with or without drag rotate enabled.

[strikan95]

@avpeery yes but thats not what I wrote about. The issue is that if you use ctrl + wheel to zoom then you need to release ctrl to drag pan, otherwise panning wont work (or if drag rotate is enable you'll accidentally start rotating the map). It would be more logical to allow user to continue holding the ctrl and drag pan if drag rotate is disabled. Especially because rotating the map is such a rare function to use. Anyway thats exactly how google did it.

[avpeery]

@strikan95 thanks for taking the time to clarify the issue, and will take into consideration.