Disable drag rotation on NavigationControl compass

[vakila]

Currently, the compass button added by default by a NavigationControl allows the user to begin a dragRotate interaction by starting a left-button/single-touch drag within the button div, and then continuing to drag across the map, outside of the button. This is achieved by the NavigationControl instantiating a new DragRotateHandler attached to the left-button - by default in addition to the map's own DragRotateHandler attached to the right-button.

This interaction is:

• Difficult to discover
• Unconventional (to my knowledge no other map does this)
• Very buggy - for example:
  • Some mobile users may have difficulty executing a click on the compass button because the short touch event is greedily interpreted as a drag (Fix Click on Compass on some mobile (add clickTolerance to DragRotateHandler) #9015)
  • If the touch/drag ends outside of the map container, the end event is not picked up by the handler and causes the drag to get "stuck"
  • Rotation reverses direction when the drag moves across the top half of the map (this seems to be a general bug in DragRotateHandler)

I think we should remove this drag-rotate functionality from the compass button, such that the compass control only:

• Rotates the compass icon in response to the user rotating the map (either with the map's default DragRotateHandler or with calls to setBearing() or similar)
• Allows the user to click to reset the bearing (and possibly pitch) to zero.

Questions for the crowd:

• Do you rely on the drag rotate functionality of the compass control?
• What problems would you/your users face if we removed it?

cc @ryanhamley @mourner

[vakila]

If we do want to remove this functionality, as I see it the next steps would be:

• Remove the additional DragRotateHandler created by the NavigationControl altogether
• Ensure that the click listener attached to the compass element now responds to click events as expected, on both desktop and mobile
• Ensure there are now no unexpected interactions with DragRotateHandler or other default map handlers on e.g. mousedown/touchstart events within the compass element added by this control

Just to be clear, users would still be able to use the map's default DragRotateHandler and TouchZoomRotateHandler to rotate the map with mouse/touch, by starting their drag elsewhere on the map canvas, outside of the control elements.

I started work on this (and adding some tests for the navigation control - we currently have none) in the WIP stop-compass-drag branch.

cc @ryanhamley @mourner @ansis

[andrewharvey]

This would also nullify #8953 which aimed to ensure that these mouse interactions could also be accessed by the keyboard for accessibility, but if we remove the mouse interaction, then it's no longer an accessibility issue.

Coincidentally I also started adding a unit test for navigation control in #8953, but I'll abandon that now if you're working on it.

[andrewharvey]

If the touch/drag ends outside of the map container, the end event is not picked up by the handler and causes the drag to get "stuck"

Mind you that also happens when interacting with the map canvas -> #4622.

[pathmapper]

Do you rely on the drag rotate functionality of the compass control?

No

What problems would you/your users face if we removed it?

Some users who are using it currently might wonder why it's not working anymore. But I don't think it would be a huge problem.

---

#8618 / #8643 are kind of related.

[Yanonix]

On mobile I personally use the compass to pitch the map. I didn't find a way to do that with fingers on the map canvas.

[ryanhamley]

I think removing this probably makes sense for the reasons outlined and because it will clean up a lot of open PRs. Obviously this feature needs some work put into it and it doesn't really seem worth the effort. However, there's a couple of points to note:

1. As @Yanonix notes, this is currently the only way to pitch the map on mobile. I know that we intend to rectify that situation when we do the major mobile gesture handling work soon, but perhaps we should hold off on removing this functionality until we add a pitch gesture.

2. I think this arguably represents a breaking change. We should consider whether or not it is. This change doesn't seem worth jumping to 2.0 (although if we have other breaking changes, we could implement a bunch all at once and make the jump). But if we decide it's not breaking then 👍

[vakila]

Hi everyone, thanks for weighing in on this!

@andrewharvey I'm not sure that removing this touch interaction is mutually exclusive with your work on keyboard compass controls in #8953 - I'll comment over there so that we can keep that discussion in one place.

Coincidentally I also started adding a unit test for navigation control in #8953, but I'll abandon that now if you're working on it.

Thanks very much for your work on those tests! Definitely do not feel you need to abandon that if you already have more tests written/in progress, it looks like you've written different tests than me so we can just merge them together (e.g. we can merge your tests and then I can resolve the conflicts with mine). If there is any reduplication I'll throw my version out.

@pathmapper 👍 Thanks for the answers! Good to know you/your users wouldn't be impacted.

@Yanonix Indeed, this (almost accidentally) provides a workaround for the lack of support for the standard two-finger-swipe up/down gesture to control pitch. But as @ryanhamley notes we plan to implement that in an upcoming release, so can I assume that once we have another way to control pitch by touch you would no longer need this functionality on the compass?

@ryanhamley re your other point

I think this arguably represents a breaking change. We should consider whether or not it is.

I'm not convinced this constitutes a semver breaking change as it doesn't break the developer-facing API, but I'm open to being convinced and I do agree we need to decide whether or not it does. Does anyone else have opinions either way?

[stevage]

Late to the party, but personally:

• I really dislike this interaction
• I really dislike the fact there is no way to simply disable rotation altogether. (I don't think most maps ever need to be rotated, and it's very confusing if it happens by accident)

At least you can disable pitch by setting maxPitch to 0. There is no equivalent for rotation.

[andrewharvey]

I really dislike the fact there is no way to simply disable rotation altogether. (I don't think most maps ever need to be rotated, and it's very confusing if it happens by accident)

map = new mapboxgl.Map({
  dragRotate: false,
  touchZoomRotate: true // true to enable pinch zoom, rotation disabled later
})
map.touchZoomRotate.disableRotation() // disable rotation, leaving pinch zoom in tact
map.keyboard.disableRotation()

map.addControl(new mapboxgl.NavigationControl({ showCompass: false }))

should work